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 670202 - rtph264depay: depends on sprop-parameter-sets in caps if avc output is negotiated
rtph264depay: depends on sprop-parameter-sets in caps if avc output is negoti...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 0.10.32
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-16 11:26 UTC by Marc Leeman
Modified: 2012-03-15 20:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
workaround patch (2.76 KB, patch)
2012-02-17 01:25 UTC, Olivier Crête
none Details | Review

Description Marc Leeman 2012-02-16 11:26:55 UTC
rtph264depay now seems to depend on the presence of sprop-parameter-sets in the caps; failing on streams that have NAL 7/8 multiplexed in the stream.

  /* ERRORS */
incomplete_caps:
  {
    GST_DEBUG_OBJECT (depayload, "we have incomplete caps");
    gst_caps_unref (srccaps);
    return FALSE;
  }


This breaks quite some streams that were working back with the previous stable release of gstreamer0.10-plugins-good (0.10.30).
Comment 1 Marc Leeman 2012-02-16 11:55:08 UTC
http://crichton.homelinux.org/~marc/downloads/vbcp-10-3-0-152-493000.gdp

The following is fine in the releases (not pre-releases), broken in git

gst-launch filesrc location=vbcp-10-3-0-152-493000.gdp ! gdpdepay ! rtph264depay ! ffdec_h264 ! xvimagesink
Comment 2 Marc Leeman 2012-02-16 11:55:40 UTC
sorry, compressed it

http://crichton.homelinux.org/~marc/downloads/vbcp-10-3-0-152-493000.gdp.xz
Comment 3 Tim-Philipp Müller 2012-02-16 12:09:09 UTC
Seems quite important and probably exists in pre-release branch too, so marking as blocker.

At first glance it looks like the depayloader decides whether it's ok to get sps/pps inline and not via the sprop-parameter-sets depending on the negotiated output format. That doesn't really seem right though. If it negotiates AVC and there is no sprop-parameter-sets, then it just needs to wait until it gets them inline before outputting anything IMHO.
Comment 4 Marc Leeman 2012-02-16 13:23:38 UTC
[marc@eee1215n gst-plugins-good]$ git bisect good
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[dca42d4767adff3578e5d5990604766735ec1f9b] tests: clean up rtp-payloading test a little
Comment 5 Marc Leeman 2012-02-16 13:26:25 UTC
[marc@eee1215n gst-plugins-good]$ git bisect good
6872b408730fd4eaacbb26fca79ebfb3f204e761 is the first bad commit
commit 6872b408730fd4eaacbb26fca79ebfb3f204e761
Author: Tim-Philipp M��ller <tim.muller@collabora.co.uk>
Date:   Wed Feb 8 20:58:04 2012 +0000

    rtph264depay: add stream-format and alignment fields to src template caps
    
    Because we can. And so we get a warning if we try to output avc with
    nal alignment or somesuch.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=606662

:040000 040000 28f2d24219528003be24eb34f0e8147f39c550ea d341015f7de231883985d4f8b21caa21231af039 M	gst
Comment 6 Tim-Philipp Müller 2012-02-16 13:36:16 UTC
I think this commit just exposes the issue, because it will now negotiate to avc format with a decoder that can accept either bytestream format or avc format, whereas before it probably just defaulted to bytestream.

We can of course change the order of the stream-format in the caps, but that just hides the bug again, it's still there and will pop up in other contexts, I think.
Comment 7 Olivier Crête 2012-02-16 15:59:07 UTC
This is not a regression. That said, attachment #192627 [details] on bug #654850 is a patch that makes rtph264depay parse the SPS/PPS out of the stream and put it in the caps to output AVC mode. But I would wait after the release to push this patch as it's non-trivial and may cause regressions.
Comment 8 Olivier Crête 2012-02-17 01:25:42 UTC
Created attachment 207830 [details] [review]
workaround patch

I see 3 options:

1. Revert Tim's patch and get back to the previously half-working state
2. Apply my patches from bug #654850 which implements parsing the SPS/PPS out of the stream and putting htem in the caps, which I think is the right long term solution (as it saves us one frame of latency when using AVC mode on a well formed RTP stream), but it may be a bit risky just before a release.
3. Apply the attached patch which is supposed to disable AVC output mode if there is no sprop-parameters-set in the input caps.
Comment 9 Tim-Philipp Müller 2012-02-17 10:35:09 UTC
Note that this patch of mine that exposes all this brokeness is only in git master, not in the 0.10 release branch, so it's all not a problem for the release, as you said earlier. It's not actually a regression, but should really be fixed for the next release.

Let's just try to fix this in master soon-ish, so there's plenty of time for people to test.

Another option is to reverse the avc/bytestream order in the template, so that it prefers bytestream if downstream accepts both, as it did before (my assumption was that parsing in the depayloader is cheaper/easier, so avc as output is preferable, but maybe that's wrong).
Comment 10 Olivier Crête 2012-03-15 20:26:46 UTC
I committed the patch from bug #654850 that makes rtph264depay just read the SPS/PPS from the stream if AVC out is negotiated.

commit 053f33adc8bd1325c01ef373469dc116ec624346
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Thu Jul 14 16:23:49 2011 -0400

    rtph264depay: Make output in AVC stream format work even without complete sp
    
    This allows outputting streams in AVC format even if the SPS/PPS are sent in
    the RTP stream.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=654850