GNOME Bugzilla – Bug 749655
vtdec: Require width and height field for H264
Last modified: 2015-06-01 15:27:33 UTC
This decoder does not work if width and height field are not set in the sinkpad caps. Let's make this explicit by adding them to the template caps.
Created attachment 303694 [details] [review] vtdec: Require width and height field for H264
Note, this patch is not tested, just wrote it while debugging with samskiter on IRC. Review attentively.
Comment on attachment 303694 [details] [review] vtdec: Require width and height field for H264 Question is also why h264parse did not add the width/height to the caps for him...
Normally you don't need h264parse when using RTP depayloader. Though, I think I remember now that width and height parsing was only added in master recently (that bug was reported against 1.4). What was my thinking around proposing this patch, is that if we strictly require width and height, it's probably better to expose it in the template caps, making it fail earlier. Does that make sense ? (Adding h264parse after the depayloader indeed fixed the issue)
Of course, the patch is good and should be merged :) I'm just wondering if there is also another bug hiding here in h264parse (or rtph264depay): not exposing width/height sometimes. We fixed some things related to that since 1.4, but maybe more is left?
Ok, I've looked in master, and interestingly Wim reverted that work. As an example, for the hypothetical pipeline: someh264src ! rtph264pay ! rtph264depay ! vtdec ! ... The caps received by vtdec will not contain width and height, forcing a parser. Note that adding a parser works. After a discussion, Olivier started adding width and height parsing (from sps/pps), so they would be present in the depayloaded caps. But might have been mistakenly added to the payloaded caps too, that would explain the revert. Having width and height in the depayloaded caps (read from SPS/PPS), was the only way we found not to have to parse twice the H264 stream. From Olivier, it seemed that it was wanted that (de)payloaders should not require a parser (for performance reason).
Yes the depayloader should put it on the caps if it can. Note that decodebin will still autoplug parsers in any case.
Attachment 303694 [details] pushed as 4cca6ef - vtdec: Require width and height field for H264
There is a separate bug that suggest adding PPS/SPS parsing to the depayloader in order to better avoid the need for an extra parser. This would simply be an optimization. I think Farstream may not work with vtdec at the moment.