GNOME Bugzilla – Bug 741501
videopool: should update video alignment after change it
Last modified: 2014-12-22 14:44:35 UTC
Video buffer pool will update video alignment to respect stride alignment requirement. But haven't update it to video alignment in configure. Which will cause user get wrong video alignment. Our use case need get the actual video alignment. Will upload the patch later.
Created attachment 292726 [details] [review] patch to fix the issue.
Review of attachment 292726 [details] [review]: It seem rather harmless, but it is slightly against gst_buffer_pool_set_config() documentation that says that if your pool changes a parameter in the config it should return false. It's not really changing the alignment, this patch in fact replace the user selected alignment with the applied alignment. Would it be possible to explain in what situation it is required to read back the alignment from the buffer pool configuration ? This is important that we understand and document this behaviour. If we decide that the config shall be update with applied alignment, this patch should come with a V4l2 buffer pool update to be consitent (along with any pool I may have forgotten).
Our hardware use physical address and has alignment requirement. The video buffer is allocator by one hardware and will used by another hardware. We can get physical address as we implement our allocator. But our hardware also need know the video padding to interpret the input video buffer. So we need know the actual video padding.
Comment on attachment 292726 [details] [review] patch to fix the issue. If we can update the size, we can definitely update the alignment.
Created attachment 292866 [details] [review] updated patch for all place need update video alignment.
Comment on attachment 292866 [details] [review] updated patch for all place need update video alignment. The patch does not apply with current master, would it be possible to resubmit ?
Created attachment 293144 [details] [review] updata the patch to apply to latest master branch.
Comment on attachment 293144 [details] [review] updata the patch to apply to latest master branch. commit e9c6c833c958c8fa28bd9e6a779f16556d92dabe Author: Song Bing <b06498@freescale.com> Date: Mon Dec 22 09:25:04 2014 -0500 videopool: update video alignment after applying Video buffer pool will update video alignment to respect stride alignment requirement. But haven't updated it to video alignment in configure. Which will cause user get wrong video alignment. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=741501
Thanks for your patch. Still, be careful when using this in your code. The only trustful alignment check is the one you do an allocated buffer stride and size. Any other check using this information should only be used as a hint to make smarter decisions. Ignoring this comment will make your code unsafe.