GNOME Bugzilla – Bug 730217
rtpsession: do not mark internal senders as expired
Last modified: 2014-05-19 08:10:24 UTC
Currently, if internal sender sources' SSRC is different than the suggested one, they will be marked as timed out, if inactivity is detected. This is because inactivity is computed with source->last_activity. This field is not update for internal sender sources. So it will always expire. There's a final check to mark the sources as expired: if (source->ssrc != sess->suggested_ssrc) { This will be true if we have multiple internal sender sources in the same session.
Created attachment 276626 [details] [review] don't check inactivity on internal senders We don't check for inactivity on internal senders as last_activity is not updated on those.
Created attachment 276628 [details] [review] don't check inactivity on internal senders fixup Just fixed a couple of typos.
wouldn't it be better to update last_activity when a new packet is sent?
Like so? commit d004eda79dd2dc32b1b970c45bd93da7a842edc9 Author: Wim Taymans <wtaymans@redhat.com> Date: Fri May 16 16:52:25 2014 +0200 rtpsession: update last_activity when sending RTP Also update last_activity when doing something with the internal source to make sure don't timeout early. See https://bugzilla.gnome.org/show_bug.cgi?id=730217
That's what I did initially but then I thought that this check if (source->ssrc != sess->suggested_ssrc) { didn't make sense when the session has multiple internal senders with different ssrc. And also that last_activity was not updated for some reason that I still didn't knew. But yes, updating last_activity would solve this problem too as we wouldn't reach the check above. If we the check above doesn't really make sense for the use case I mentioned then I would skip doing it with my patch. And if updating last_activity also makes sense when sending RTP packets, then I would also include that.
Just tried your fix. It works great. Feel free to close the bug and ignore my patch if you think it doesn't make sense. And thanks!
(In reply to comment #5) > That's what I did initially but then I thought that this check > > if (source->ssrc != sess->suggested_ssrc) { > > didn't make sense when the session has multiple internal senders with different > ssrc. And also that last_activity was not updated for some reason that I still > didn't knew. The idea is that one of those internal sources has an SSRC equal to the suggested SSRC and we would like to keep that in the session. If the (internal) suggested SSRC times out (and was sending SR reports), we turn it into receiver (sending RR reports) instead of removing the SSRC and choosing a new one the next time we need to make an RR.