On 9 May 2018 at 00:33, Mark Thompson <s...@jkqxz.net> wrote: > On 08/05/18 19:24, Lukas Rusak wrote: >> + >> + layer->format = avbuf->context->av_pix_fmt == AV_PIX_FMT_NV12 ? >> + DRM_FORMAT_NV12 : DRM_FORMAT_NV21; >> + layer->nb_planes = 2; >> + >> + layer->planes[1].object_index = 0; >> + layer->planes[1].offset = avbuf->plane_info[0].bytesperline * >> + avbuf->context->format.fmt.pix_mp.height; > > Is that always true? I would expect that some driver might want more > vertical alignment (especially with tiled formats) and would provide this > number somewhere else.
The V4L2 spec defines their NV12/21 as: "These are two-plane versions of the YUV 4:2:0 format. The three components are separated into two sub-images or planes. The Y plane is first. The Y plane has one byte per pixel. For V4L2_PIX_FMT_NV12, a combined CbCr plane immediately follows the Y plane in memory. " [1] Please be aware that there is now the V4L2 multi-planar API which returns the planes in independent buffers (and an independent dma_buf fd for each plane). That is not the case being handled here. If being a real pedant, then it wants to be "layer->planes[1].offset = avbuf->plane_info[0].bytesperline * avbuf->context->format.fmt.pix.height;" as this should be using the single planar API union member. width and height align between the two structures anyway. If you want the buffer to be a multiple of macroblocks or have other padding requirements, then the width and height are defined to be that size. Use the selection API [2] to get the cropping window. >> + layer->planes[1].pitch = avbuf->plane_info[0].bytesperline; >> + break; >> + >> + case AV_PIX_FMT_YUV420P: >> + >> + if (avbuf->num_planes > 1) >> + break; >> + >> + layer->format = DRM_FORMAT_YUV420; >> + layer->nb_planes = 3; >> + >> + layer->planes[1].object_index = 0; >> + layer->planes[1].offset = avbuf->plane_info[0].bytesperline * >> + avbuf->context->format.fmt.pix_mp.height; >> + layer->planes[1].pitch = avbuf->plane_info[0].bytesperline >> 1; >> + >> + layer->planes[2].object_index = 0; >> + layer->planes[2].offset = layer->planes[1].offset + >> + ((avbuf->plane_info[0].bytesperline * >> + avbuf->context->format.fmt.pix_mp.height) >> 2); >> + layer->planes[2].pitch = avbuf->plane_info[0].bytesperline >> 1; > > Similarly here, and the pitch feels dubious as well. Is > plane_info[n].bytesperline set for n > 0? Likewise the definition of YU12/YV12 is that the planes will follow immediately [3] Pass on plane_info[n] - I only know V4L2 in any depth. >> + break; >> + >> + default: > > Probably want a more explicit failure in other cases. Is there any 10-bit > handling here yet (P010, I guess)? Based on a quick search I don't believe V4L2 has added support for any 10 bit formats as yet, therefore it would be strange to get here with a 10bit format selected. Dave [1] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-nv12.html [2] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-g-selection.html#vidioc-g-selection [3] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-yuv420.html _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
