GNOME Bugzilla – Bug 797304
rtpsession: The RTP timestamp in the RTCP Sender Report may be completely wrong
Last modified: 2018-11-03 15:33:32 UTC
Created attachment 373962 [details] [review] Reset the RTPSession object in PAUSED->READY The RTP timestamp used for RTCP Sender Report is generated from a cached extended RTP timestamp, which in turn is updated for every pushed RTP buffer. The timestamp generation does not work with a gst-rtsp-server that uses GST_RTSP_SUSPEND_MODE_RESET. If the reset suspend mode is used, the server will configure a media and prepare it in RTSP DESCRIBE/SETUP, which will trigger rtpsource to cache an extended RTP timestamp. The pipeline will now be set to GST_STATE_NULL. In RTSP PLAY, the pipeline will be set to GST_STATE_PLAYING again. A new random offset will be used for the RTP timestamp, but the previously cached extended RTP timestamp in all RTPSource objects remains, which leads to a situation where gst_rtp_buffer_ext_timestamp() sometimes return 0. When this happens, the RTP timestamp in the RTCP Sender Report is completely broken. I have implemented rtp_session_reset() that will iterate over all it's RTPSource's and reset them. The important part is to clear RTPSource.last_rtptime. Does it make sense to reset the whole RTPSession object?
Review of attachment 373962 [details] [review]: Makes sense. Did you check if there are more things to reset similarly?
I checked in RTPSource if there where anything more obvious, but couldn't find anything. rtp_source_reset() already reset a lot of things, but I'm not familiar with all the details here. The retained_feedback list in RTPSource might also make sense to reset, but I don't know what the impact would be for rtspsrc/rtpbin. I have not looked at RTPSession.
(In reply to Linus Svensson from comment #2) > The retained_feedback list in RTPSource might also make sense to reset Yes :) Can you add that?
(In reply to Sebastian Dröge (slomo) from comment #3) > Yes :) Can you add that? Yes. I'll add it and run our RTSP tests. Will probably have something ready at afternoon today.
Created attachment 373976 [details] [review] Reset RTP Source
Review of attachment 373976 [details] [review]: ::: gst/rtpmanager/rtpsession.c @@ +1079,3 @@ + + g_hash_table_foreach (sess->ssrcs[sess->mask_idx], (GHFunc) reset_source, + NULL); I think we should actually remove all the sources here. They're not going to make any sense the next time the element is started up again. But there are a couple of other things that should also be reset in rtpsession: last_rtcp_check_time and the other timeout related fields, first_rtcp, and various other fields. ::: gst/rtpmanager/rtpsource.c @@ +258,3 @@ + g_queue_foreach (src->retained_feedback, (GFunc) gst_buffer_unref, NULL); + g_queue_clear (src->retained_feedback); + src->last_rtptime = -1; This looks correct
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/510.