GNOME Bugzilla – Bug 740899
video-info: Add plane size information
Last modified: 2014-12-16 03:19:44 UTC
In certain cases one could want to implement a video buffer pool that allocate each plane separately. The problem is that figuring-out correct plane sizes from a GstVideoInfo is not really possible. Though, it's really easy to calculate it when filling up plane information inside video info implementation. A known use case is the gl buffer pool and memory. For the gl upload uses case, it uses one malloced buffer per plane to download/upload from/to the textures. Keeping these on sync. When comes time to allocate the plane, it current uses video info offset differences to figure-out the size. The problems starts when we try to implement GstVideoAlignment. After running gst_video_info_align(), the offset technique to deduce the plane sizes does not work any-more.
Created attachment 291801 [details] [review] [PATCH] video-info: Add plane_size array The plane size is needed when allocating 1 memory per plane. This size is actually hard to deduce in elements that support large amount of formats and wants to handle GstVideoAlignement. https://bugzilla.gnome.org/show_bug.cgi?id=740899 --- gst-libs/gst/video/video-info.c | 141 +++++++++++++++++----------------------- gst-libs/gst/video/video-info.h | 5 +- 2 files changed, 64 insertions(+), 82 deletions(-)
Comment on attachment 291801 [details] [review] [PATCH] video-info: Add plane_size array Looks good to me but I don't like that we get rid of all the padding ;) Isn't the same also needed for GstVideoMeta?
(In reply to comment #2) > (From update of attachment 291801 [details] [review]) > Looks good to me but I don't like that we get rid of all the padding ;) Do you have a way around that ? > > Isn't the same also needed for GstVideoMeta? Size and plane_size[] are only useful for allocation. When VideoMeta (default_map) maps a memory, it currently uses find_memory with the offset and a size of 1. Passing plane_size for validating that plane is large enough would not work, as there is no indication how much data has been added for padding before the offset. Using GStreamer default plane sizes for this width and height may not work either as our default includes some padding at the end of planes to enhance alignement for system memory operations when allocating multiple planes in a single buffer. Though, adding plane_size[] (along with missing size) to VideoMeta could be nice. As right now, the updated VideoInfo endup with invalid size, hence plane_size[] values. But this is a different issue that also require a slight API juggling.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 291801 [details] [review] [details]) > > Looks good to me but I don't like that we get rid of all the padding ;) > > Do you have a way around that ? I don't, at least nothing even more ugly. But we should get some more opinions if something more important than the plane size might be missing from GstVideoInfo before merging this.
Ok, I'll give it some time.
This won't be needed any-more. Required plane size can always we computed using stride * height. The extra padding make little sense when dealing with split allocation. The only exception is for tiled format, but it is also very straightforward to compute is needed.