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 607353 - rtph264pay & base: Don't crash if the other side specifies the profile-level-id
rtph264pay & base: Don't crash if the other side specifies the profile-level-id
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal blocker
: 0.10.18
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-01-18 20:22 UTC by Olivier Crête
Modified: 2010-01-19 12:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
basertppayload: Reject empty caps (954 bytes, patch)
2010-01-18 20:24 UTC, Olivier Crête
rejected Details | Review
basertppayload: Reject empty caps (954 bytes, patch)
2010-01-18 20:25 UTC, Olivier Crête
committed Details | Review
rtph264pay: Don't ignore the return value from set_outcaps (3.59 KB, patch)
2010-01-18 20:25 UTC, Olivier Crête
committed Details | Review
rtph264pay: Don't set profile-level-id in out caps (4.30 KB, patch)
2010-01-18 20:25 UTC, Olivier Crête
committed Details | Review

Description Olivier Crête 2010-01-18 20:22:57 UTC
gst-launch-0.10  videotestsrc ! x264enc  ! rtph264pay ! application/x-rtp, payload=98, profile-level-id=asdasdsa ! fakesink

-> FAIL

First, the basepayload class does not check if the intersection between the desired caps and the result from getcaps is not empty.

Second, rtph264pay, does not check the return value from set_outcaps

Third, profile-level-id shouldn't even be there. It is a restriction placed on the sender. So it should be in the caps received by the payloader, not the ones it produces (the same information is in the sprop-parameter-sets).
Comment 1 Olivier Crête 2010-01-18 20:24:55 UTC
Created attachment 151707 [details] [review]
basertppayload: Reject empty caps
Comment 2 Olivier Crête 2010-01-18 20:25:14 UTC
Created attachment 151708 [details] [review]
basertppayload: Reject empty caps
Comment 3 Olivier Crête 2010-01-18 20:25:19 UTC
Created attachment 151709 [details] [review]
rtph264pay: Don't ignore the return value from set_outcaps
Comment 4 Olivier Crête 2010-01-18 20:25:22 UTC
Created attachment 151710 [details] [review]
rtph264pay: Don't set profile-level-id in out caps

The profile-level-id represents restrictions on what can be sent, it does not
describe the stream. So it should be reflected in the sink caps of the
payloader, not the src caps.
Comment 5 Olivier Crête 2010-01-18 20:29:51 UTC
Also, we should look into how the caps negotiation works for RTP caps. I see at least 3 different types of parameters

1. Parameters that must be the same on the receiver and the sender (like ilbc mode), this more or less matches how Gst works
2. Parameters that describe the stream (ie sprop-* for H.264), these should only be used in setcaps (not getcaps) and show only flow pay->depay
3. Parameters that restrict the stream (like profile-level-id or ptime or maxptime), should should flow from depay to pay.. So they should never be set in setcaps, only used in getcaps.

I'll try to whip out some patches to the baseclass and the various payloaders to implement that if it makes sense.
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2010-01-19 09:39:04 UTC
Olivier, the first two patches are the same. Did you indent to upload a different patch too? Otherwise this looks good.
Comment 7 Wim Taymans 2010-01-19 12:30:41 UTC
commit ad399c806922a419713d50a97731ae3bc5fb8b6d
Author: Olivier Crête <olivier.crete@collabora.co.uk>
Date:   Mon Jan 18 14:33:30 2010 -0500

    basertppayload: Reject empty caps
    
    https://bugzilla.gnome.org/show_bug.cgi?id=607353
Comment 8 Wim Taymans 2010-01-19 12:37:12 UTC
commit 7a0590b1f181853162a02a3082a4c3a097978fc5
Author: Olivier Crête <olivier.crete@collabora.co.uk>
Date:   Mon Jan 18 14:41:10 2010 -0500

    rtph264pay: Don't ignore the return value from set_outcaps
    
    https://bugzilla.gnome.org/show_bug.cgi?id=607353
Comment 9 Wim Taymans 2010-01-19 12:48:52 UTC
commit c4fa559f1594ab4f8505da78196c5a871b833c0a
Author: Olivier Crête <olivier.crete@collabora.co.uk>
Date:   Mon Jan 18 14:49:26 2010 -0500

    rtph264pay: Don't set profile-level-id in out caps
    
    The profile-level-id represents restrictions on what can be sent, it does not
    describe the stream. So it should be reflected in the sink caps of the
    payloader, not the src caps.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=607353