[Bug 762543] xvimagesink: wrong pool selection when rendering buffers having GstVideoCropMeta attached

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=762543

--- Comment #14 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
(In reply to sreerenj from comment #11)
> Wondering what is the use case of GstVideoCropMeta then?? :)

It was introduced I believe to try and support theora specification. In theora,
encoder can decide a x,y,width,height crop, regardless of the uv alignment. The
spec says the encoder SHOULD respect the alignment though. Our encoder always
set x/y to 0, which is always aligned with the sub-sampling. The problem is
that the crop meta is badly defined. Currently, it is only usable if the caps
are the GstBuffer original size. Otherwise you don't know what VideoMeta to
pass when mapping the frame. In the original VideoFrame implementation, it
would simply ignore the width/height from the caps, and use the one from
VideoMeta, though this would lead to certain issues with x/y is not zero. Some
assertions preventing that were added since, as this would lead to buffer
oveflow. Maybe this was the wrong fix, it remains that using VideoMeta +
VideoAlignment is the most tested, and most natural thing to do. This kind of
padding is meant to be stored as stride and offset array. Though, with
sub-sampling, this impose a certain alignment, which you have here.

(In reply to Sebastian Dröge (slomo) from comment #12)
> GstVideoAlignment and GstVideoCropMeta are completely different things that
> only overlap slightly in some use cases. And if you use the crop meta
> correctly, things should work correctly nonetheless. Correctly means: caps
> contain display size, allocation query contains memory size, crop meta
> contains cropping that has to be done to get from memory size to display
> size.

Not in this case. The proposed pool from xvimagesink is not used, the sink does
not save the caps from the allocation query. It only works if you use the pool
atm. The caps in the query where never meant to be a second set_format() imho.
This case is fixable using the video alignment, which means keeping the cropped
width/height in caps and setting the VideoMeta properly. Meanwhile,
VideoCropMeta is not usable here, also nothing in the design or doc seems to
help with that. There is a bug about it, but until it's fixed, it's better to
avoid it when not required.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.