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 728041 - rtph264depay: marks all output buffers as delta units when outputting avc format
rtph264depay: marks all output buffers as delta units when outputting avc format
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 1.2.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-11 13:50 UTC by Josep Torra Valles
Modified: 2014-04-12 10:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtph264depay: only guess AU boundaries when aren't indicated by marker (3.46 KB, patch)
2014-04-11 16:31 UTC, Josep Torra Valles
committed Details | Review

Description Josep Torra Valles 2014-04-11 13:50:04 UTC
Some video decoders expect at least one buffer with the delta-unit flag unset to start decoding and rtph264depay seems to push only buffers with the delta-unit flag set and stream isn't played.

To reproduce the issue download [1] and play [2][3] pipelines.

[1] https://dl.dropboxusercontent.com/u/4577541/sample.gdp
[2] gst-launch-1.0 -v filesrc location=sample.gdp ! gdpdepay ! rtph264depay ! fakesink silent=false
[3] gst-launch-1.0 filesrc location=sample.gdp ! gdpdepay ! rtph264depay ! avdec_h264 ! queue ! xvimagesink
Comment 1 Tim-Philipp Müller 2014-04-11 14:05:08 UTC
Note: works fine when forcing output to video/x-h264,stream-format=byte-stream
Comment 2 Josep Torra Valles 2014-04-11 16:31:01 UTC
Created attachment 274115 [details] [review]
rtph264depay: only guess AU boundaries when aren't indicated  by marker

This patch fixes the issue and adds some comments for better understanding of the code intention.
Comment 3 Tim-Philipp Müller 2014-04-11 16:56:56 UTC
Makes sense, I think, but I wonder if we should keep track of whether we have ever seen the marker bit set, and then decide based on that and not whether the marker bit is set in this specific packet or not, since even if the sender does set the market bit, it won't be set on every payload. (Assuming we can assume that the sender doesn't change)
Comment 4 Wim Taymans 2014-04-12 02:43:59 UTC
commit eaee14aff4489823b6540568ba6c57c559dd9edd
Author: Josep Torra <n770galaxy@gmail.com>
Date:   Fri Apr 11 18:19:49 2014 +0200

    rtph264depay: only guess AU boundaries when aren't indicated by marker
    
    The marker bit isn't mandatory and we had in place code to guess AU
    boundaries by detecting a new picture start. This guessing code
    didn't work with interlaced content that has proper marker bits
    to indicate the AU boundaries. It was leaking the first field buffer
    and producing a corrupted output.
    
    fixes: https://bugzilla.gnome.org/show_bug.cgi?id=728041
Comment 5 Wim Taymans 2014-04-12 02:48:42 UTC
(In reply to comment #3)
> Makes sense, I think, but I wonder if we should keep track of whether we have
> ever seen the marker bit set, and then decide based on that and not whether the

You say we should keep track of marker bits and then assume that when they are set, they are set on all AU-end packets? That would make it possible to skip the AU-start detection heuristic and maybe get more cases right.

> marker bit is set in this specific packet or not, since even if the sender does
> set the market bit, it won't be set on every payload. (Assuming we can assume
> that the sender doesn't change)

Or are you saying that we can't assume the marker bit is set on all AU-end packets and we need to do the heuristic in all cases anyway?
Comment 6 Tim-Philipp Müller 2014-04-12 10:04:46 UTC
Sorry if that wasn't very clear.

I think I was asking if it is reasonable to assume that if the marker bit is set once the marker bit will always be set correctly to mark the end of all AUs. If the answer to that question is yes, then it might be a good idea to change the code to do if (!depay->marker_bit_in_use) instead of if (!marker) instead so that we don't do heuristics ever in those cases. But then, arguably, our heuristics should never hurt I guess, unless they're broken, so might just as well leave it as is instead of making assumptions about the input.