GNOME Bugzilla – Bug 683142
h264parse: should proxy demuxer width+height (incorrectly reports Canon MOV resolution as 1920x1088 instead of 1920x1080)
Last modified: 2013-07-24 08:07:53 UTC
If you only have gstreamer1.0-libav installed, you get the correct 1920x1080 resolution. However, if you also have gstreamer1.0-plugins-bad installed, the h264parse element gets used, which is broken and results in the incorrect 1920x1088 resolution. Small test file (21 MB, md5 dd6c7f44d666962058343f0732aae509): http://cdn.novacut.com/MVI_5751.MOV There was a similar bug in libav that would give height=1088 even when the bad plugins weren't installed. But this has since been fixed in Quantal: https://bugs.launchpad.net/ubuntu/+source/libav/+bug/937561
Created attachment 223112 [details] pipeline without -bad
Created attachment 223113 [details] pipeline with -bad
Still an issue with 1.0.x git. The parser needs to proxy the width/height received from the demuxer, which indicates the resolution to crop to. GstH264Parse:h264parse0.GstPad:sink: caps = video/x-h264, ... width=(int)1920, height=(int)1080, ... avdec_h264:avdec_h264-0.GstPad:sink: caps = video/x-h264, ... width=(int)1920, height=(int)1088, ...
commit 0e0dd05fd645b32a10b5d5da74f2feb67010ec87 Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Thu Dec 13 14:12:52 2012 +0100 h264parse: use upstream width/height when given The upstream width and height should override the dimension detected in the file. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=683142
I don't think this is the right approach for this, especially not without sanity-checking the upstream width/height (also do decoders take the upstream width/height at all?). Would probably be better to let the demuxer put crop metadata on the buffers or something like that.
IMHO, - sanity checking upstream width/height is "nice", but not really super-important - decoders *should* take the provided upstream size and crop (there's a bug about that I think) - the commit is needed either way, even if we add crop metadata as well, because: - the width/height in the caps should always reflect the effective width/height after cropping. This is the case with videocrop even when using crop meta (hopefully!), and should be the case here as well. I'm not sure what the crop meta adds in this context, or how it would work exactly.
(In reply to comment #6) > IMHO, > > - sanity checking upstream width/height is "nice", but not really > super-important I wouldn't be surprised if we can produce crashes downstream if a larger width/height than actually present is given in the caps > - decoders *should* take the provided upstream size and crop (there's a bug > about that I think) Yes, where I gave the same argument ;) > - the commit is needed either way, even if we add crop metadata as well, > because: > > - the width/height in the caps should always reflect the effective > width/height after cropping. This is the case with videocrop even when using > crop meta (hopefully!), and should be the case here as well. Correct, but the reason why I don't think that just changing the width/height is a good idea is that it's ambigious. With the crop metadata you know which x/y offset and which width/height should be used, with only a different width/height in the caps you don't know anything about that. I guess something similar to the libav change could be allowed though. Just allowing a width/height that is smaller and that would result in the same number of macro blocks.
Ok, so basically we agree that the commit is fine, we just need: a) more sanity checking, and b) optional crop meta in the (unlikely?) case cropping does not take place at bottom+right ? Do you have an example codec/case where the usual crop off at bottom/right does not apply?
MXF has support for this, in theory JPEG2000 but decoders are already cropping (and don't allow zerocopy).
Ok, so to me it seems this is primarily a demuxer/decoder issue then, and we should close this bug, since the original issue has been fixed. The non-0/0 offset cropping is a new feature to be implemented all over the place.
Yes, closing bug.