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 730217 - rtpsession: do not mark internal senders as expired
rtpsession: do not mark internal senders as expired
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.3.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-05-15 21:09 UTC by Aleix Conchillo Flaqué
Modified: 2014-05-19 08:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
don't check inactivity on internal senders (3.12 KB, patch)
2014-05-15 21:14 UTC, Aleix Conchillo Flaqué
none Details | Review
don't check inactivity on internal senders fixup (3.59 KB, patch)
2014-05-15 21:18 UTC, Aleix Conchillo Flaqué
rejected Details | Review

Description Aleix Conchillo Flaqué 2014-05-15 21:09:05 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.
Comment 1 Aleix Conchillo Flaqué 2014-05-15 21:14:42 UTC
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.
Comment 2 Aleix Conchillo Flaqué 2014-05-15 21:18:23 UTC
Created attachment 276628 [details] [review]
don't check inactivity on internal senders fixup

Just fixed a couple of typos.
Comment 3 Wim Taymans 2014-05-16 14:22:31 UTC
wouldn't it be better to update last_activity when a new packet is sent?
Comment 4 Wim Taymans 2014-05-16 14:57:36 UTC
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
Comment 5 Aleix Conchillo Flaqué 2014-05-16 15:05:50 UTC
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.
Comment 6 Aleix Conchillo Flaqué 2014-05-16 17:52:17 UTC
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!
Comment 7 Wim Taymans 2014-05-19 08:10:24 UTC
(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.