GNOME Bugzilla – Bug 707361
video: Add support for 64x32 tiled NV12 color format
Last modified: 2014-01-13 15:54:08 UTC
Created attachment 253941 [details] [review] androidmedia: add support for COLOR_QCOM_FormatYUV420PackedSemiPlanar64x32Tile2m8ka colorspace There is a weird tiled colorspace in some Qualcomm devices. The attached patch is for 0.10 and copied from vlc. I don't really know how to tiled formats should be represented in 1.0
I think this has been done already now. Mostly a case for libgstvideo now?
Yes, I ported that to 1.0 too already. What's missing is generic support in libgstvideo for this now. The same format (I think it is the same) is also used by Samsung Exynos btw: V4L2_PIX_FMT_NV12MT
Ok, adding this to libgstvideo was not that easy. The problem was that we need the video height to be able to figure-out where are the pixels in the buffer. As I thought breaking this API again was a bad idea, I managed to implement it in a backward compatible way by introducing pack2_func/unpack2_func. Not the cutest thing ever, but backward compatible. The only known "external" user of this is videmixer, since it's copying videoconvert.c. Note that pack/unpack are not very optimal. It would be made better by playing with the pack_lines parameters and do pack/unpack 32 lines per call. Though no one ever tried using that. In genenral this should not be a problem as we expect the format to be passed as-is to the display. For the conversion code, I have found the source of what the videolan guy has done, that's what I'm based on and it came with very nice explanation. Note that I have kept his reduction, as it made sense. I'm very open to comment, suggestion, better ideas. Patches will follow...
Created attachment 259760 [details] [review] [PATCH] Define NV12 64x32 Tile format This is used by certain HW like the Exynos and Qualcomm CODECs. The format is describe at http://linuxtv.org/downloads/v4l-dvb-apis/re30.html . --- gst-libs/gst/video/video-format.c | 3 +++ gst-libs/gst/video/video-format.h | 2 ++ gst-libs/gst/video/video-info.c | 8 ++++++++ 3 files changed, 13 insertions(+)
Created attachment 259761 [details] [review] [PATCH] video-format: Add height paramter to Pack/Unpack method This parameter is needed to implemement pack/unpack for format where pixel are layout using pattern. It is the case for hardware supported NV12 tile formats, where tiles a layout in Z flip Z order. As adding this parameter would be an API break, we needed to dupplicate pack/unpack and GST_VIDEO_FORMAT_ALL. The original pack unpack don't use the extra parameter, thus can be cased to the old signature format. --- docs/libs/gst-plugins-base-libs-sections.txt | 3 + gst-libs/gst/video/video-blend.c | 13 +- gst-libs/gst/video/video-format.c | 350 +++++++++++++++++---------- gst-libs/gst/video/video-format.h | 85 ++++++- gst/videoconvert/gstvideoconvert.c | 4 +- gst/videoconvert/videoconvert.c | 56 ++--- gst/videotestsrc/gstvideotestsrc.c | 8 +- gst/videotestsrc/videotestsrc.c | 4 +- tests/check/elements/playbin-complex.c | 12 +- tests/check/libs/video.c | 19 +- 10 files changed, 362 insertions(+), 192 deletions(-)
Created attachment 259762 [details] [review] [PATCH] Implement pack/unpack method for NV12_64x32 gst-libs/gst/video/video-format.c | 187 +++++++++++++++++++++++++++++++++++++- gst-libs/gst/video/video-format.h | 2 +- 2 files changed, 186 insertions(+), 3 deletions(-)
Created attachment 259765 [details] [review] [PATCH] Add round up 128 macr gst/gstutils.h | 9 +++++++++ 1 file changed, 9 insertions(+)
Created attachment 259769 [details] [review] [PATCH] videoscale-test: Omit unsupported format NV12_64x3 tests/check/elements/videoscale.c | 1 + 1 file changed, 1 insertion(+)
I'm not a fan of the _ALL2 addition, I think we should just leave the pack/unpack pointers NULL for formats that can't be transformed with the old API. It's a slight API break, but if a tree falls in the middle of the forest and no one listens, does it make a sound ?
Yeah, the _ALL2 thing looks not nice. Otherwise this looks ok :)
I don't like it how the layout is now determined by the width of the video. this breaks cropping or simple adjustments to the width to implement padding. What I think would be better is if you code the extra parameters to determine the layout of the planes inside a free plane of data. This would also make it possible to reuse the existing functions and existing things will just keep on working.
After a long discussion on IRC, it was revealed that GstVideoInfo width and height are cropped width and height, while the layout in this format depends on the image size (more precisely, original number or rows and columns). This information does not exist in GStreamer and cannot be deduced (the crop meta consist of x, y, width, height where width and height will likely have to match the VideoInfo width and height). Note that this cropping API matches V4L2 API, but not Android API (which uses crop-left, crop-right, crop-top and crop-bottom). Take note though that videoconvert does not support incoming buffer with crop meta at the moment, and destination buffer is assumed to have no cropping (no x parameter to pack). But on the other hand, "standard" formats can be cropped on right and bottom without having to do anything, thus a buffer could be somehow cropped by simply reducing the width and the height. This is also true for this format, as long as the image width and height is preserved somewhere. Why does it matter for NV12, it's because the pattern used to layout the tiles has a special case on the last row. If it is an odd row, the tiles are placed linearly instead of the Z pattern. Obviously, they do so since otherwise they'd have to wast a row of tiles, which is a lot bigger then a row of pixels. In the end, if height is cropped more then a tile height, we need to know what was the original height otherwise we will miss-interpret the last line. This being said, we need not to confused bugs with problems, there is three known bugs in the code atm: Converter Code: 1. Number of columns shall be determine using stride (rounded down to multiple of 128), not width rounded up. Obviously, ROUND_DOWN_128 (sride) >= ROUND_UP_128 (width). This is defined by the format, and most of the time this will be the same. 2. The tile_position() function assumed stride being a multiple of 128, this is always true in general, but we can improve this to support any random stride without affecting much the performance. 3. The unpack() function ignores the x. This is a bit tricky to handle, but right now we assume it's always 0, which will probably remain true until we start supporting incoming buffers with crop meta. (I can fix that, it's just about adding more special cases, i.e. if x is odd, I simply did not notice the signature difference). Now for the missing information, which is not defined in GStreamer, it was suggested to add an extra plane with the extra parameters. I first thought that adding the full index would be nice, but adding memory offset in a plane seems a bit dangerous in the end. Also, I felt that the most generic and correct way to do this would be to double the planes, ensuring that each planes has it's map, and that the map has it's own stride. Though this is not possible since it would require more then the maximum of 4 planes we are allowed. (8 would be a number that fits all format). In the end, I will encode the image height in the place (in big endian). It is very generic, not dedicated to tiles, and could be reused for wide variety of format with special layout, and where this layout can be expressed by a function. The second question was where to setup that plane, the answer is where the data is created. As this information is not stored in GStreamer, the last time we have this information is when we assemble the buffer. This is very similar to how palettes are handled, so this is how the extra parameter will be handled. The only problem with that approach is that we will not be able to support the same thing on top of A420, as there would be not enough planes left in Gstreamer API. We might need a fast path for Android use case, though I think that the sink should be able to support that format directly if the decoder produces it. If Andoni could clarify this, adding a de-tiling fast path should be rather easy but require some time and I'd be incline not to do that now. For me that converter is simply for enable videotestsrc to produce this format.
Created attachment 260181 [details] [review] [PATCH] Define NV12 64x32 Tile format This is used by certain HW like the Exynos and Qualcomm CODECs. The format is describe at http://linuxtv.org/downloads/v4l-dvb-apis/re30.html . https://bugzilla.gnome.org/show_bug.cgi?id=707361 --- gst-libs/gst/video/video-format.c | 3 +++ gst-libs/gst/video/video-format.h | 2 ++ gst-libs/gst/video/video-info.c | 8 ++++++++ 3 files changed, 13 insertions(+)
Created attachment 260182 [details] [review] [PATCH] Implement pack/unpack method for NV12_64x32 https://bugzilla.gnome.org/show_bug.cgi?id=707361 --- docs/libs/gst-plugins-base-libs-sections.txt | 1 + gst-libs/gst/video/gstvideopool.c | 8 ++ gst-libs/gst/video/video-format.c | 202 ++++++++++++++++++++++++++- gst-libs/gst/video/video-format.h | 11 +- gst-libs/gst/video/video-info.c | 6 +- 5 files changed, 220 insertions(+), 8 deletions(-)
Created attachment 260183 [details] [review] [PATCH] videoscale-test: Omit unsupported format NV12_64x32 https://bugzilla.gnome.org/show_bug.cgi?id=707361 --- tests/check/elements/videoscale.c | 1 + 1 file changed, 1 insertion(+)
Created attachment 260184 [details] [review] [PATCH] video-frame: Tile formats needs compatible height For tile format we need to check that the un-cropped height stored in the last plane is the same in the src and destination. Layout could otherwise be affected. https://bugzilla.gnome.org/show_bug.cgi?id=707361 --- gst-libs/gst/video/video-frame.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
Let me know what you think of this version. Thanks !
Created attachment 264022 [details] [review] [PATCH] video: Add definitions for tiled video format docs/libs/gst-plugins-base-libs-sections.txt | 1 + gst-libs/gst/video/video-format.h | 84 +++++++++++++++++++++++++++- gst-libs/gst/video/video-frame.h | 8 +++ gst-libs/gst/video/video-info.h | 23 +++++++- 4 files changed, 111 insertions(+), 5 deletions(-)
Created attachment 264023 [details] [review] [PATCH] video: Implement frame copy for tiled video forma gst-libs/gst/video/video-frame.c | 64 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 5 deletions(-)
Created attachment 264024 [details] [review] [PATCH] video: Implement format convertion for tiled video format This is done by introducing a new helpder function, gst_frame_pack() and gst_frame_unpack() --- docs/libs/gst-plugins-base-libs-sections.txt | 2 + gst-libs/gst/video/video-format.c | 101 ++++++++++++++++++++ gst-libs/gst/video/video-frame.c | 137 +++++++++++++++++++++++++++ gst-libs/gst/video/video-frame.h | 6 ++ gst/videoconvert/videoconvert.c | 4 +- gst/videotestsrc/videotestsrc.c | 4 +- 6 files changed, 249 insertions(+), 5 deletions(-)
Created attachment 264025 [details] [review] [PATCH] video: Define NV12_64x32 tiled format This video format is known to be used by Exynos 4 and certain Qualcomm hardware. Along with the format definition, this format comes with GstVideoFormatInfoTileOffset and GstVideoFormatInfoGetTile function implementation, allowing generic colorconvertion to be achieved --- gst-libs/gst/video/video-format.h | 4 +++- gst-libs/gst/video/video-info.c | 10 ++++++++++ tests/check/elements/videoscale.c | 1 + 3 files changed, 14 insertions(+), 1 deletion(-)
Ok, to address all comments I had and issues I've found (including issues I've had with the extra plane), I've rewritten it a third time. This time I basically integrate the tile format into the infrastructure adding tile_width/tile_height to VideoFormatInfo, adding n_tiles_x, n_tiles_y to VideoInfo (that get ajusted when gst_video_info_align() is called with appropriate alignment). With this, I managed to implement generic frame copying and generic color conversion for tiles. It should be really easy now to implement a detiling fast-path (e.g. NV12_64x32 -> NV12, I420).
Created attachment 264031 [details] [review] [PATCH] video: Implement format convertion for tiled video format This is done by introducing a new helpder function, gst_frame_pack() and gst_frame_unpack(). --- docs/libs/gst-plugins-base-libs-sections.txt | 2 + gst-libs/gst/video/video-frame.c | 137 +++++++++++++++++++++++++++ gst-libs/gst/video/video-frame.h | 6 ++ gst/videoconvert/videoconvert.c | 4 +- gst/videotestsrc/videotestsrc.c | 4 +- 5 files changed, 148 insertions(+), 5 deletions(-)
Created attachment 264032 [details] [review] [PATCH] video: Define NV12_64x32 tiled format This video format is known to be used by Exynos 4 and certain Qualcomm hardware. Along with the format definition, this format comes with GstVideoFormatInfoTileOffset and GstVideoFormatInfoGetTile function implementation, allowing generic colorconvertion to be achieved. --- gst-libs/gst/video/video-format.c | 101 ++++++++++++++++++++++++++++++++++++++ gst-libs/gst/video/video-format.h | 4 +- gst-libs/gst/video/video-info.c | 10 ++++ tests/check/elements/videoscale.c | 1 + 4 files changed, 115 insertions(+), 1 deletion(-)
A lot of code is missing here like macros to access tiles in frame, extra abidata in the info etc.
Some remarks: - You now moved the unpacking of the tiled formats to a higher layer, the frame layer. You seem to want to do this so that you can use extra fields (from the GstVideoInfo, the x/y tiles) to do the unpacking. Why not place those extra fields in the last free plane and avoid this extra layer of unpacking? - You have implemented a default function to fill the x/y tiles in the GstVideoInfo. When you use GstVideoMeta, how is this updated? where does the x/y tiles come from? are you going to use the width/height of the GstVideoMeta for this? You could also store this in the 4th plane.. - frame_pack and frame_unpack seem to only be able to unpack one line of a frame starting from the leftmost pixel. This seems weird as a general unpack function.
(In reply to comment #26) > Some remarks: > > - You now moved the unpacking of the tiled formats to a higher layer, the frame > layer. You seem to want to do this so that you can use extra fields (from the > GstVideoInfo, the x/y tiles) to do the unpacking. Why not place those extra > fields in the last free plane and avoid this extra layer of unpacking? Adding an extra plane is more difficult then it looks likes. With the extra plane, we need to add code all over the stack to be able to ignore it, parse it, produce it. It is also very error prone, developer would constantly forget to add it, which result is crash or worst, undefined behaviour. Also, using a free plane has the problem that it prevent using tiles for certain formats like A420 (no plane left). Adding extra field is much cleaner and keep the code well centralized with the added addition that it is easy to reuse for any tiled formats. The extra layer is really thin, I've also took the time to profile it, and it is not a visible overhead. > > - You have implemented a default function to fill the x/y tiles in the > GstVideoInfo. When you use GstVideoMeta, how is this updated? where does the > x/y tiles come from? are you going to use the width/height of the GstVideoMeta > for this? You could also store this in the 4th plane.. There is a call to gst_video_info_align() that will update it. My finding after more research, is that the un-cropped size can be deduced from the video alignment. In the case there is not video alignment, a default will be assumed. This not much different than any other formats. It would have been put into GstVideoMeta, but it would be an ABI break as there is no padding. I sill think we should use more the video alignment, as otherwise we have duplicate information, with risk of error. As an example, current we could put a different width and height in the VideoMeta then what is in the caps, it's being copied, and that would have side effect as it's not expected. > > - frame_pack and frame_unpack seem to only be able to unpack one line of a > frame starting from the leftmost pixel. This seems weird as a general unpack > function. You can review the existing implementation, unpack/pack multiple lines or unpack/pack from a specific offset is not implemented. But I can add it to the API, if it's there because there is an intention to implement that.
(In reply to comment #25) > A lot of code is missing here like macros to access tiles in frame, extra > abidata in the info etc. I'm not sure I understand what you mean by "a lot". I've added: GST_VIDEO_FRAME_N_TILES_X GST_VIDEO_FRAME_N_TILES_Y GST_VIDEO_FRAME_TILE_WIDTH GST_VIDEO_FRAME_TILE_WIDTH GST_VIDEO_FRAME_COMP_N_TILES_X GST_VIDEO_FRAME_COMP_N_TILES_X Accessing the tile data is done through a function pointer, those haven't been made as macro in the past, is it what you would like ? Is there any code you see that you would replace with call to macros ? Thanks
(In reply to comment #28) > > I'm not sure I understand what you mean by "a lot". I've added: I mean that the code is not in the patches that you attached, the patches are incomplete.
(In reply to comment #29) > (In reply to comment #28) > > > > I'm not sure I understand what you mean by "a lot". I've added: > > I mean that the code is not in the patches that you attached, the patches are > incomplete. I've copy pasted the macros I just mention from the attached patch: [PATCH] video: Add definitions for tiled video format https://bugzilla.gnome.org/attachment.cgi?id=264022 It's not impossible I am missing something, there is my branch over there: http://cgit.collabora.com/git/user/nicolas/gst-plugins-base.git/log/?h=707361-nv12-tiled-rework It's four patches, I don't see what is missing yet. Do you mean it won't compile for you ?
(In reply to comment #29) > I mean that the code is not in the patches that you attached, the patches are > incomplete. Thinking of it, maybe you are looking it up from some emails. I have fixed some errors in the patches after comment 22 because two patches where wrong. Are you sure you where not looking at one of the obsoleted patch ?
Created attachment 264813 [details] [review] Add tiled support flags
Created attachment 264814 [details] [review] add tiled pack and unpack for NV12T Also add docs describing the T component
Previous 2 patches are what I think should happen. It adds a new Tiled component that does not add a plane, it contains the number of tiles in the stride, this along with the other stride allows calculation of the x and y dimension of the tiled region. This adds support for NV12T to both videoconvert and videotestsrc.
Created attachment 264815 [details] [review] update of second patch
Review of attachment 264815 [details] [review]: I can give this variant a try, though gst_video_frame_copy() is broken with that code. Where you on that already ? ::: gst-libs/gst/video/video-format.c @@ +2088,3 @@ + * @GST_VIDEO_TILE_MODE_ZFLIPZ_2X2: Every four adjacent buffers - two + * horizontally and two vertically are grouped together and are located + * in memory in Z or flipped Z order. We should probably mention the exception on the last row if there is an odd number of row. Other then that, I like the idea of exposing tile mode inside an enumration. @@ +2220,3 @@ + tile_size = tile_width * tile_height; + + x_tiles = stride[0] / 64; x_tiles = stride[0] / tile_width; ::: gst-libs/gst/video/video-format.h @@ +143,3 @@ GST_VIDEO_FORMAT_NV16, GST_VIDEO_FORMAT_NV24, + GST_VIDEO_FORMAT_NV12T, What name will we give to the next variant I have kept on hold ? It's NV12, with tile mode "LINEAR", and tile sizes is 16x16. Stride alignment should be 16. (was the reason I have put 64x32 in the name, even though not very cute API whise). ::: gst-libs/gst/video/video-info.c @@ +574,3 @@ + info->stride[0] = GST_ROUND_UP_128 (width); + info->stride[1] = info->stride[0]; + info->stride[2] = GST_ROUND_UP_32 (height) / 32; Ok, fair, this is perfectly fine for my needs, and does not require change to the video pool.
Created attachment 264874 [details] [review] update patch Made the format definition 3 planes, as it should have been.
Created attachment 264875 [details] [review] add video-tile helpers These functions can help with dealing with tiled formats. It is set up in such a way that we can avoid breaking current code and add new tile modes later.
Created attachment 264876 [details] [review] Fix video copy Patch to fix the plane copy code
Created attachment 264877 [details] [review] some cleanups
Created attachment 264879 [details] [review] fixes fix some off-by-ones
branch here: http://cgit.freedesktop.org/~wtay/gst-plugins-base/log/?h=tiled
> > What name will we give to the next variant I have kept on hold ? > > It's NV12, with tile mode "LINEAR", and tile sizes is 16x16. Stride alignment > should be 16. (was the reason I have put 64x32 in the name, even though not > very cute API whise). V4L2 has these variants: TM12 -> NV12MT VM12 -> NV12MT_16X16 HM12 -> HM12 is like NV12MT_16X16 but contiguous planes AFAIU No idea if these are 'official' fourccs. These fourccs don't look very descriptive making them very hard to remember, which is not a good property for things you put on caps. Lets go for the more scriptive NV12_16L16 and NV12_64Z32 then, as you suggested? I put the Linear and Zigzag marker in there.. > > ::: gst-libs/gst/video/video-info.c > @@ +574,3 @@ > + info->stride[0] = GST_ROUND_UP_128 (width); > + info->stride[1] = info->stride[0]; > + info->stride[2] = GST_ROUND_UP_32 (height) / 32; > > Ok, fair, this is perfectly fine for my needs, and does not require change to > the video pool. What we really would need is the size of the plane in the GstVideoMeta (either as size or as number of strides). It is needed in some places (mapping, copying, ...) and we always end up having to guess it based on the cropped height, which turns out to not work so well for tiled formats. For the next ABI break, I think this would be best. We could derive the number of vertical tiles from it, like we do now for the stride. It would also allow us to store multiple memory blocks per plane (interesting for RTP raw video)
(In reply to comment #43) > > > > What name will we give to the next variant I have kept on hold ? > > > > It's NV12, with tile mode "LINEAR", and tile sizes is 16x16. Stride alignment > > should be 16. (was the reason I have put 64x32 in the name, even though not > > very cute API whise). > > V4L2 has these variants: > > TM12 -> NV12MT > VM12 -> NV12MT_16X16 > HM12 -> HM12 is like NV12MT_16X16 but contiguous planes AFAIU > > No idea if these are 'official' fourccs. These fourccs don't look very > descriptive making them very hard to remember, which is not a good property for > things you put on caps. > > Lets go for the more scriptive NV12_16L16 and NV12_64Z32 then, as you > suggested? I put the Linear and Zigzag marker in there.. I have no ideas either, though this naming seems just right to me, and should scale quite well. > What we really would need is the size of the plane in the GstVideoMeta (either > as size or as number of strides). It is needed in some places (mapping, > copying, ...) and we always end up having to guess it based on the cropped > height, which turns out to not work so well for tiled formats. For the next ABI > break, I think this would be best. We could derive the number of vertical tiles > from it, like we do now for the stride. It would also allow us to store > multiple memory blocks per plane (interesting for RTP raw video) Indeed, next ABI break though. Streaming tiled video through RTP (1 tile per packet) is an interesing idea. Thanks for all this work, it is very appreciated. I'll go through the code and do some testing when I after new year.
Commit 1df82fc14fe79ed24301e078425d3e3e30aabb0c is relevant to this bug: Author: Jan Schmidt <jan@centricular.com> Date: Tue Dec 31 23:18:54 2013 +1100 androidmedia: Add new color format, and enhance debug output Add a new color format seen on my Galaxy S3 (OMX_SEC_COLOR_FormatNV12Tiled = 0x7fc00002) to the table, but don't actually implement it - the decoder doesn't choose it. Remove an assert that makes the plugin fail noisily and take the app down if it sees a color format it doesn't recognise (just skip the codec instead) Modify the debug output when plugin scanning to print color format info to make this sort of thing easier in the future.
Relevant as in OMX_SEC_COLOR_FormatNV12Tiled is NV12_16L16
I tried this version today, but so far it fails for me when trying to map the third plane (which does not really exist). default video-frame.c:137:gst_video_frame_map_id: failed to map video frame plane 2 Running: gst-launch-1.0 videotestsrc ! video/x-raw,width=1280,height=720,format=NV12T ! v4l2sink device=/dev/video12 The difference is that the buffers are mmap() allocated, and there is multiple memories. I need to dig more, but it's odd to me to map a plane that does not exist, though I'm guessing to get the metadata to be copied, we need to declare that third plane. To be continued ...
Review of attachment 264876 [details] [review]: I've also tested this code and found that UV of the last row is not copied correctly. ::: gst-libs/gst/video/video-frame.c @@ +272,3 @@ + tile_size = 1 << ts; + + mode = finfo->pixel_stride[tidx]; Pixel stride is per component, not per plane. Should be GST_VIDEO_COMP_TILEINFO instead of tidx.
Review of attachment 264876 [details] [review]: After fixing these two, it seems to work correctly for me. ::: gst-libs/gst/video/video-frame.c @@ +275,3 @@ + + sx_tiles = sinfo->stride[plane] >> ws; + sy_tiles = sinfo->stride[tidx]; This is not right, you need to scale the height. sy_tiles = GST_VIDEO_FORMAT_INFO_SCALE_HEIGHT (finfo, plane, sinfo->stride[tidx]); @@ +278,3 @@ + + dx_tiles = dinfo->stride[plane] >> ws; + dy_tiles = dinfo->stride[tidx]; Same
updated my branch with suggestions http://cgit.freedesktop.org/~wtay/gst-plugins-base/log/?h=tiled
(In reply to comment #50) > updated my branch with suggestions > http://cgit.freedesktop.org/~wtay/gst-plugins-base/log/?h=tiled I confirm it works on Exynos / v4l2. The fact that number of plane is 3 rather then 2 was a small annoyance, but nothing compare to having an actual third plane.
Created attachment 265808 [details] [review] Don't use extra plane and component As discussed on IRC, a) using an extra plane without actually having associated plane data was not very nice and b) using the forth component would prevent support for tile ARGB or A420 formats. Proposed solution for a) is to encode the size in tiles in the stride: (y_tile << 16) | x_tiles For b) I simply added tile_mode (the tiling mode, currently ZFLIPZ_2X2), tile_ws (the tile width shift), tile_hs (the tile height shift). It was discuss that we probably don't need to do the union with the reserved data since this structure is never allocated on the stack or not by GStreamer. A fourth opinion is welcomed.
updated my branch http://cgit.freedesktop.org/~wtay/gst-plugins-base/log/?h=tiled looking good to me, can't find anything to change.
Created attachment 265848 [details] [review] Generate enumartion type from video-tile.h too
Created attachment 265849 [details] [review] Complete the description for documentation generator This should complete the patch set I think, anything else ?
commit 016e5285c7354601dc67e8a1c94ac280e2c9e4cb Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Wed Jan 8 19:43:01 2014 -0500 doc: Add new sections introduce for tile format https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 595bcfb4d7026a8d2000f8f00a7e075e5d1bea46 Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Wed Jan 8 19:42:35 2014 -0500 video: Generate types for tile enumeration https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit f52fd7a68b8b6e5814aa076fcade6f0d5a7ef066 Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Wed Jan 8 19:41:56 2014 -0500 video: Don't use extra plane and componenent for tile format Instead of using extra plane, we encode the number of tiles in x and y in the stride of each planes (i.e. y_tiles << 16 | x_tiles) and introduce tile_mode, tile_width and tile_height into GstVideoFormatInfo structure. https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit d899e6df5ac162709f8d2cf53c23d0fa8ce3f09f Author: Wim Taymans <wtaymans@redhat.com> Date: Fri Jan 3 22:36:13 2014 +0100 video: rename NV12T -> NV12_64Z32 Is a bit more descriptive and allows us to add more tiled types later. https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit c8597330a95b35822ac54575fd2df7a669645612 Author: Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> Date: Fri Jan 3 22:29:09 2014 +0100 video-frame: scale vertical tiles based on subsampling https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 53605e35f4d5748c5a433a36b8f0286a10143577 Author: Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> Date: Fri Jan 3 22:18:08 2014 +0100 video-frame: fix tiled pixel stride Pixel stride is per component, not per plane. We get the tile mode from the pixelstride of the TILE component. https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 61cbdf379bcafdcf1aac1af793046fcd6a8d47c8 Author: Wim Taymans <wtaymans@redhat.com> Date: Thu Dec 26 17:40:05 2013 +0100 format: improve docs https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 12eedf86e880935d8f93e6327b73396baed5f00b Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Dec 25 16:22:32 2013 +0100 tests: fix videoscale test for NV12T https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit f3e989179bd32eb3615c9d0a8a124113fa09afa0 Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Dec 25 16:06:43 2013 +0100 video-format: fix off-by-one for tiled coordinates https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit fb0fecbf48b1a43821484991ec26f6d0d7b9f7ca Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Dec 25 15:22:24 2013 +0100 video-tile: improve docs https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 33c486e5c1d9472e0844de6fae08272c36a6dbfe Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Dec 25 14:57:30 2013 +0100 video-format: use shifts when possible https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 980811c1201c1ca9c8d10bbf2f0d3ef4d02d9a9f Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Dec 25 14:23:04 2013 +0100 video-frame: fix copy of tiled formats Add code to copy tiled planes. https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 3ceb7dfe22c0f4247fd4e4949223cfa1e3ace33c Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Dec 25 14:11:57 2013 +0100 video-tile: add tile mode and helper functions Move the tile helper functions to their own file. Make it possible to make other tiling modes later. https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit f8d3b9b4fcc5e08b771314fa95e9ed8f750b54e6 Author: Wim Taymans <wtaymans@redhat.com> Date: Fri Dec 20 21:27:46 2013 +0100 video: add NV12T support https://bugzilla.gnome.org/show_bug.cgi?id=707361 commit 67a271723562eb259f67bece7f08a1af090d166e Author: Wim Taymans <wtaymans@redhat.com> Date: Thu Dec 19 16:11:50 2013 +0100 Add tiled color format support https://bugzilla.gnome.org/show_bug.cgi?id=707361