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 748723 - dtlssrtpdec: Merges RTP and RTCP into the same stream
dtlssrtpdec: Merges RTP and RTCP into the same stream
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-04-30 19:24 UTC by Sebastian Dröge (slomo)
Modified: 2015-05-07 19:07 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastian Dröge (slomo) 2015-04-30 19:24:00 UTC
dtlssrtpdec currently has an srtpdec that splits the RTP and RTCP streams... which are then just combined again with a funnel. It would seem more useful to keep them separated, otherwise later elements (rtpbin) have to split the two streams yet another time.
Comment 1 Sebastian Dröge (slomo) 2015-04-30 19:24:33 UTC
Also it would be more symmetric with the encoder, which has separate sinkpads for RTP and RTCP.
Comment 2 Patrik Oldsberg 2015-05-01 18:54:30 UTC
If I remember correctly, the only reason for the current design was that it made integration with rtpbin a tiny bit more simple when using rtcp mux. If the incoming stream may or may not be using rtcp mux, then you'd have to set up a funnel for rtcp after the dtlssrtpdec bins anyway.

I guess it doesn't really matter that rtpbin has to split up the streams again, since the check is always done anyway? (haven't actually had a look at the code, just assuming that's how it works).

So the pro with the current design is that it makes a specific (but quite common) use case a bit more simple to implement for the application, while the con is that we are sacrificing some cycles by passing the buffers through a funnel.
Comment 3 Patrik Oldsberg 2015-05-01 18:58:02 UTC
I guess it does make a lot of sense to keep them separated if you want to do something else with the stream than passing it directly to rtpbin, is there any use case were that would be done?
Comment 4 Sebastian Dröge (slomo) 2015-05-02 07:33:29 UTC
(In reply to Patrik Oldsberg from comment #2)
> If I remember correctly, the only reason for the current design was that it
> made integration with rtpbin a tiny bit more simple when using rtcp mux. If
> the incoming stream may or may not be using rtcp mux, then you'd have to set
> up a funnel for rtcp after the dtlssrtpdec bins anyway.

You would get RTP/RTCP separated here, and then have another dtlssrtpdec that outputs RTCP. So you would then link the RTP pad to the RTP part of rtpbin, and the two RTCP pads to a funnel and then to the RTCP part of rtpbin.

This way you would only have a single funnel instead of two, and that funnel would be at a low-bandwidth stream. Due to having to send all sticky events again on stream changes, it's better to have the funnel at a low-bandwidth stream.

> I guess it doesn't really matter that rtpbin has to split up the streams
> again, since the check is always done anyway? (haven't actually had a look
> at the code, just assuming that's how it works).

The splitting in rtpbin is basically to try if it's RTP... and if not relay the packet to the RTCP handling instead. So it's additional overhead, but not much.

> So the pro with the current design is that it makes a specific (but quite
> common) use case a bit more simple to implement for the application, while
> the con is that we are sacrificing some cycles by passing the buffers
> through a funnel.

Not sure how it makes anything more simple as you need another funnel anyway if you want to handle RTCP-mux and separate RTCP. The only thing it makes easier is if you actually know that you have RTCP-mux and nothing else. If you don't, you will have two funnels and quite a bit more overhead, see above ;)



We'll go ahead then and keep them separated, I'll also provide a patch for OpenWebRTC before changing it.
Comment 5 Patrik Oldsberg 2015-05-02 07:53:15 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)
> (In reply to Patrik Oldsberg from comment #2)
> > If I remember correctly, the only reason for the current design was that it
> > made integration with rtpbin a tiny bit more simple when using rtcp mux. If
> > the incoming stream may or may not be using rtcp mux, then you'd have to set
> > up a funnel for rtcp after the dtlssrtpdec bins anyway.
> 
> You would get RTP/RTCP separated here, and then have another dtlssrtpdec
> that outputs RTCP. So you would then link the RTP pad to the RTP part of
> rtpbin, and the two RTCP pads to a funnel and then to the RTCP part of
> rtpbin.
> 
> This way you would only have a single funnel instead of two, and that funnel
> would be at a low-bandwidth stream. Due to having to send all sticky events
> again on stream changes, it's better to have the funnel at a low-bandwidth
> stream.

This is the tradeoff I'm talking about, extra funnels inside the elements vs having to set up an extra funnel after the elements. The difference is tiny either way. But I agree that's it's probably worth leaving de decision to the application.

> > I guess it doesn't really matter that rtpbin has to split up the streams
> > again, since the check is always done anyway? (haven't actually had a look
> > at the code, just assuming that's how it works).
> 
> The splitting in rtpbin is basically to try if it's RTP... and if not relay
> the packet to the RTCP handling instead. So it's additional overhead, but
> not much.
> 
> > So the pro with the current design is that it makes a specific (but quite
> > common) use case a bit more simple to implement for the application, while
> > the con is that we are sacrificing some cycles by passing the buffers
> > through a funnel.
> 
> Not sure how it makes anything more simple as you need another funnel anyway
> if you want to handle RTCP-mux and separate RTCP. The only thing it makes
> easier is if you actually know that you have RTCP-mux and nothing else. If
> you don't, you will have two funnels and quite a bit more overhead, see
> above ;)

It's more simple for the application, since you just plug each dtlssrtpdec into one rtp/rtcp pad each on rtpbin.

> We'll go ahead then and keep them separated, I'll also provide a patch for
> OpenWebRTC before changing it.

Ok, thanks :)
Comment 6 Sebastian Dröge (slomo) 2015-05-07 19:07:03 UTC
commit c534c8899cbaed9f8afb2563488a57de2965cf00
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Thu May 7 21:04:30 2015 +0200

    dtlssrtpdec: Don't merge RTP and RTCP streams that were just split by srtpdec
    
    The funnel has some overhead, and later rtpbin will have to split both streams
    again anyway.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=748723