GNOME Bugzilla – Bug 791412
videoconvert: Should not forward crop meta
Last modified: 2018-04-27 14:01:35 UTC
Since 2012 videoconvert blindly pretend to support all meta found on downstream allocation query. This seems kind of error prone, and in fact it's totally wrong for the crop meta. When crop meta is added upstream, the caps width/height are reduces to the cropped width/height. As a side effect, the newly allocated buffers in videoconvert are of that size. Because it does not know, videoconvert will in fact crop as if the top/left crop was zero. The result being completly wrong.
I figure-out that if videoconvert looks at video meta when selecting the width/height it would fix this issue, because it would convert the full image. Though, it would be inefficient, because cropping reduces the surface to convert, and cropping is cheaper (it's "just" a smaller copy). So I'm not completely certain which way, the fix is trivial both ways (filter-out crop meta vs look at video meta width/height). If convertion is to be done in software anyway, and cropping is small and handle without copy at display, then this new approach would be better then dropping the meta.
Created attachment 365329 [details] [review] videoconvert: Filter-out crop meta To passthrough crop-meta, the converter would need to allocate and convert buffers of the size of the originating buffer. This is currently made difficult by GstBaseTransform since we cannot alter the caps passed though the allocation query. We would also need to wait for the first input buffer to be received in order to make the decision around that size. So the short and safe solution is just to stop pretending we can passthrought that meta.
Attachment 365329 [details] pushed as 5affe09 - videoconvert: Filter-out crop meta
Sorry to be late to this bug, but I just found this bit of code in videoconvert and I'm confused. Which piece of code restricts the caps based on GstVideoCropMeta? I thought the crop meta only specified a valid region inside the frame, completely independent of the caps. The GstVideoMeta is different and can express a physical frame width/height that's different to the logical caps, but not the Crop meta, right?
The caps in the caps event represent the cropped width/height, and videoconvert only convert the regions of this size, starting from offset 0,0. We would need to add support for this, which can be tricky, because the GstVideoCropMeta does not have to aligned with the subsampling. Some support exist in pack/unpack, but it's a bit untested. An example pipeline (with caps added, would be): source ! video/x-raw,width=320,height=240 ! videocrop left=10 ! video/x-raw,width=310,height=240 ! sink