GNOME Bugzilla – Bug 747394
rtpsession: Track RTX ssrc caps
Last modified: 2015-04-16 15:34:31 UTC
Created attachment 301000 [details] [review] rtpsession: Track RTX ssrc caps This avoids warnings such as: 0:00:09.739081034 24515 0x21cd4f0 WARN rtpsource rtpsource.c:1412:rtp_source_get_new_sr: no clock-rate, cannot interpolate rtp time
Review of attachment 301000 [details] [review]: ::: gst/rtpmanager/gstrtprtxsend.c @@ +594,2 @@ gst_event_parse_caps (event, &caps); + gst_event_unref (event); After this the caps are not valid anymore @@ +605,3 @@ + /* The session might need to know the RTX ssrc */ + caps = gst_caps_make_writable (caps); The event usually owns the one and only reference to the caps, so you need to explicitly copy them above before unreffing the event
*** Bug 747845 has been marked as a duplicate of this bug. ***
Bug 747845 contains patches for the same thing, but don't add RTX knowledge to rtpsession (which is IMHO breaking abstractions too much). Ideally we would get "mixed caps" merged, and then can have rtxsend send the caps for both streams with both ssrcs
Created attachment 301535 [details] [review] rtpsource: Try to keep clock-rate and payload type updated for sender RTCP too For RTX we will get packets with different payload types, and only for the main stream we have the clock-rate and everything inside the caps. For the RTX stream we will have to ask. The clock-rate is later needed for properly creating sender reports.
Created attachment 301536 [details] [review] rtprtxsend: Copy over timestamps from the orignal buffers to the RTX buffers
Created attachment 301537 [details] [review] examples/rtp: Implement request-pt-map handler for server-rtpaux example
Mixed caps initial implementation can be found here: http://cgit.freedesktop.org/~wtay/gstreamer/log/?h=mixed-caps
Created attachment 301541 [details] [review] rtpsession: Track RTX ssrc caps This is needed so that we can generate SR for RTX stream correctly (the clock rate is required).
Just updated the older patch to remove the refcounting bug so if we do decide to go with this approach, it's valid. As Sebastian points out, adding RTX awareness to rtpsession is a bit annoying, but involves a little less overhead.
Main problem with my patch is that we previously never emitted request-pt-map for senders... and now we would emit it for every single packet unless something answers the signal (then we only do it once per pt).
We're going with adding RTX knowledge to rtpbin now, it's needed in too many places.
Review of attachment 301541 [details] [review]: ::: gst/rtpmanager/gstrtprtxsend.c @@ +604,3 @@ + /* The session might need to know the RTX ssrc */ + caps = gst_caps_make_writable (caps); Still wrong, you don't own the reference to caps... so would have to copy in any case. I fixed that locally :) ::: gst/rtpmanager/rtpsession.c @@ +2707,3 @@ + obtain_internal_source (sess, ssrc, &created, GST_CLOCK_TIME_NONE); + if (source) { + rtp_source_update_caps (source, caps); Problem here now is that the caps contain the ssrc and the payload type, but those are going to be invalid for the newly created source. Looking into this now
Created attachment 301729 [details] [review] rtpsession: Track RTX ssrc caps This is needed so that we can generate SR for RTX stream correctly (the clock rate is required).
Created attachment 301730 [details] [review] rtpsource/rtprtxsend: Also pass correct seqnum-offset and payload to the RTX rtpsource
commit 80268e7d370ade8f3b4a7aed3e7268ca4d22c52e Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Apr 16 15:31:25 2015 +0200 rtpsource/rtprtxsend: Also pass correct seqnum-offset and payload to the RTX rtpsource https://bugzilla.gnome.org/show_bug.cgi?id=747394 commit 26bec72e520522af0751abe738fb169ea52d6c04 Author: Arun Raghavan <arun@centricular.com> Date: Mon Apr 6 12:56:50 2015 +0530 rtpsession: Track RTX ssrc caps This is needed so that we can generate SR for RTX stream correctly (the clock rate is required). https://bugzilla.gnome.org/show_bug.cgi?id=747394 commit 17c6532b7578506c5bed82c1c5b5d51820430b76 Author: Sebastian Dröge <sebastian@centricular.com> Date: Tue Apr 14 13:56:38 2015 +0200 rtprtxsend: Copy over timestamps from the orignal buffers to the RTX buffers https://bugzilla.gnome.org/show_bug.cgi?id=747394