GNOME Bugzilla – Bug 724630
v4l2videodec: alignment support
Last modified: 2014-05-08 20:21:50 UTC
Created attachment 269534 [details] [review] v4l2object alignment support Some hardware requires that their buffers are aligned both in width and height. V4L2 is supporting this through G/S_FMT ioctl where: - g_fmt.fmt.pix.width/height are the unaligned width/height (the area inside the buffer where the valid pixels are) - g_fmt.fmt.pix.bytesperline is the amount of bytes to skip to switch from one line of the buffer to the next line (so it's the width aligned expressed in bytes) - g_fmt.fmt.pix.sizeimage is the total size of buffer (aligned, from which we can deduce the aligned height) In Gstreamer, alignment is managed through video meta inside "info" structure. gst_video_info_align(info, align) can be used to set the alignment constraint (padding at right and bottom) inside info. ** Explanations of attached patch proposal: * gst_v4l2_object_get_v4l2_alignment have been introduced to convert the V4L2 bytesperline & sizeimage informations into aligned width and height in pixels, based on pixel format (quite painfull but not found a better way to do it). I have not tested all the pixel formats (only NV12 on my hardware), and Samsung Exynos 64x32 tiled format must be added. * Exynos driver is using G_CROP to return the unaligned width/height while G_FMT returns aligned width/height. Functionally it's a bit different as G_CROP aims to express a window of interest inside the buffer (such as we can encounter in H264 streams for ex. and is not related to hardware alignment constraint). Nevertheless, I have tried to make the code compatible with both approaches (anyway some tests are needed with Samsung hardware to check that nothing was broken). * in gst_v4l2_buffer_pool_alloc_buffer, a change was also needed to ensure right computation of "offset" in case of non multi-planar: - else - offs += obj->bytesperline[i] * - GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT (finfo, i, height); height is wrong here, it should be height aligned. In fact it seems that this function aims to compute the right offset and stride arrays to give to gst_buffer_add_video_meta_full, but the "info" structure already contains the correct ones (at least in case of non-multiplanar and after having called gst_video_info_align with right alignment). So what have been done in this patchset is not doing the extra computation in case of non-multiplanar and give directly the stride & offset arrays from "info" to gst_buffer_add_video_meta_full.
Took me a while to understand what you are trying to do, but I finally got that the problem you are facing is in fact that you driver produces planar format stored in a single buffer, but add some extra (non-standard) padding between planes. [ Y plane ] --- padding Y [ U plane ] --- padding U [ V plane] --- padding V Something that comes to my mind is that the new v4l2_plane API has been written exactly to solve this kind of issue. Why can't your newly written driver use the multi-planar API instead of guessing the padding this way ? How can we be certain that for all drivers that the padding after Y, U and V will be uniform ? I could easily see drivers implementer that decide that their alignment is the default for the format, but add extra padding only at the end. In this case increasing the height the way you do to compute offset with uniform padding will not work. If instead you port your driver to the MPLANE API, you can set memory offset and plane size for each of the plane and remove this problem. The memory can be two seperate blocks but can also be from the same block, it does not matter (some work needed for dmabuf, but probably not that much). Let me know what you think of this, but so far I'm worried about this patch.
Hi Nicolas, We are dealing with NV12, ie semiplanar so drawing is more like this: [ Y plane ] --- padding Y [ UV plane ] --- padding UV but this doesn't change picture at all, you perfectly get the point of extra padding for height. Most of V4L2 hardware have alignment constraint in width, it's why there is an explicit bytesperline field in V4L2 different from width, to express that only width pixels have to be read on a given line, then bytesperline bytes have to be skipped to go to the next line. Most of V4L2 devices are cameras, which are based on a single line buffer (with an alignment constraint), then the line is copied in memory to build the frame, so without alignment in height. With V4L2 codec API we are dealing with slightly new video codec hardwares, planes based (Yplane/UVplane) and with alignment constraint in both width and height, and clearly we miss of an explicit V4L2 field for this alignment in height. We are using total sizeimage to express this alignment in height, and you are true that implictly we suppose that padding is done at the end of each Y and UV plane, but based on my past experience on at least 3 totally different video decoders/encoders hardware IPs, the padding in height was done that way (I precise that it was not the same HW designers team). About MPLANE, you are right, we can do it with MPLANE, but MPLANE was introduced by Samsung because of a strong hardware constraint that implies that Y address is not in the same bank than UV address, so NV12 frame is not contiguous in memory. We don't have such constraint, so we see no reason to switch all of our drivers to MPLANE. If this is really an issue on your side, we can align on Samsung way of doing and use G_CROP to get the unaligned width & height. Another way to progress on this, is that I submit problem to V4L2 mailing list so we have the opinion from V4L2 maintainers, what do you think about this ? BR.
(In reply to comment #2) > Another way to progress on this, is that I submit problem to V4L2 mailing list > so we have the opinion from V4L2 maintainers, what do you think about this ? Thanks, I'll look forward into this. Another reason I'm raising this is that on Samsung there is other IP blocks that don't have this strong constraint. Even though they use mplane API, they expose formats like NV21 fully stored in the first plane. Very small buffers are stored in slighly bigger buffer (usually aligned to 128x32 boundary) though the padding is at the end, not the way you expose it. For different padding they would use two planes.
After a discussion with v4l2 devs on IRC, adjusting the value of S_FMT and setting appropriate crop is the only way to handle height alignment. Width alignment are to be treated this way if you have multiple plane in a single plane, or if you are using a tiled format (pretty much the same conceptually). Note, I have a patch series here that I'm still working on to address some of the missing part of alignment support.
OK, thanks Nicolas, we will align on this.
Alignement is supported though proper stride and offset now.