After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 791271 - v4l2videodec: padding removal only works if the driver extends both width and height
v4l2videodec: padding removal only works if the driver extends both width and...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-12-05 15:53 UTC by Philipp Zabel
Modified: 2017-12-06 19:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v4l2videodec: Handle drivers that only round up height (2.09 KB, patch)
2017-12-05 15:53 UTC, Philipp Zabel
committed Details | Review

Description Philipp Zabel 2017-12-05 15:53:37 UTC
Created attachment 365034 [details] [review]
v4l2videodec: Handle drivers that only round up height

Commit 1f31715c9861 ("v4l2videodec: use visible size, not coded size, 
for downstream negotiation filter") added support for removing the 
padding obtained as the difference between width/height from G_FMT and 
visible width/height from G_SELECTION from the probed caps obtained 
via TRY_FMT.
The attached patch fixes the padding removal for drivers that only round
up  height, but not width, to the padded frame size. This might happen 
because horizontal padding can be handled by line stride (bytesperline), 
but there is no such thing as plane stride in the V4L2 API for 
single-buffer planar formats.
Comment 1 Nicolas Dufresne (ndufresne) 2017-12-05 17:22:10 UTC
Review of attachment 365034 [details] [review]:

This seems fair, though I notice that in v4l2object.c we make up the padding from the stride instead of simply looking the at the composition data. I can't remember or find any reason why we need to calculate that. We could simply keep whatever is considering "padding" from the SELECTION separate from the padding introduce by the stride, and then the driver should be coherant, keeping this code simpler. I'm just asking for feedback, I think the code is correct though (just a bit hairy).
Comment 2 Philipp Zabel 2017-12-06 10:00:58 UTC
Not sure, I assumed this was something special for the tiled formats.
I'm not opposed to keeping a "padding" around that is the difference between format width/height and visible rectangle as opposed to the difference between format stride/height and visible rectangle to keep this code simple. But as long as there is no way to simultaneously TRY_FMT and G_SELECTION for the tried format, this is always going to be a bit iffy.
Comment 3 Nicolas Dufresne (ndufresne) 2017-12-06 12:18:02 UTC
I think I'll go with your patch then. My memory is not coming back.
Comment 4 Nicolas Dufresne (ndufresne) 2017-12-06 19:01:24 UTC
Thanks again for your work.

Attachment 365034 [details] pushed as 5e05cad - v4l2videodec: Handle drivers that only round up height