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 736647 - Tunneled RTSP sessions do not always timeout as expected
Tunneled RTSP sessions do not always timeout as expected
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
unspecified
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-14 19:21 UTC by Branko Subasic
Modified: 2014-09-24 07:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
A possible fix for the described issue (4.38 KB, patch)
2014-09-14 19:25 UTC, Branko Subasic
needs-work Details | Review
Proposed patch 1/3. (1.58 KB, patch)
2014-09-19 16:51 UTC, Branko Subasic
committed Details | Review
Proposed patch 2/3. (4.41 KB, patch)
2014-09-19 16:51 UTC, Branko Subasic
committed Details | Review
Proposed patch 3/3. (891 bytes, patch)
2014-09-19 16:52 UTC, Branko Subasic
committed Details | Review

Description Branko Subasic 2014-09-14 19:21:40 UTC
gst-rtsp-server monitors a clients liveness by requiring it to periodically send either an RTCP or an RTSP packet. If it does not receive a packet within a certain period of time (by default 60 seconds) the server destroys the session and releases resources allocated for that session.
 
However, currently this mechanics is disabled when RTP is tunneled over RTSP. The server simply relies on the TCP connection. If it's up the client is considered being alive. Unfortunately, this way a client with network problem may drop of without the server noticing.

This problem is easily fixed by using the same liveness monitoring as when UDP transport is used, i.e. requiring the client to periodically send RTCP or RTSP packets. I think that would be in line with the RTSP RFC. It's discussed in RFC 2326, section A.2, and in more detail in the RTSP v2 draft, section 10.5.

I will attach a patch which allows the application to choose if RTCP/RTSP monitoring should be used or not for tunneled sessions. The default behavior is not to use it, i.e. same as currently. Would something like that be a accepted?
Comment 1 Branko Subasic 2014-09-14 19:25:35 UTC
Created attachment 286176 [details] [review]
A possible fix for the described issue
Comment 2 Sebastian Dröge (slomo) 2014-09-15 08:15:08 UTC
Comment on attachment 286176 [details] [review]
A possible fix for the described issue

Makes sense to me, but maybe this should become the new default if it's in line with the RFC?
Comment 3 Sebastian Dröge (slomo) 2014-09-15 08:15:34 UTC
Wim, what was the original reason to special case TCP connections here?
Comment 4 Wim Taymans 2014-09-15 09:27:11 UTC
IIRC, my thinking was that the server should try to be as tolerant as possible and I seem to remember there was I client I tested that didn't send lively messages (or when I used rtspsrc without lively messages on a server in TCP mode, it would not timeout, though I don't know for sure anymore). It was a conscious choice, somehow.

I'm fine with making this the default. As long as there are RTP packets flowing through the TCP connection, it should be OK to omit the liveness messages but the problem is when you pause the TCP stream, there is no other way for the server to detect liveness than to rely on the RTCP or RTSP wellness messages.
Comment 5 Branko Subasic 2014-09-16 06:52:50 UTC
Making the server as tolerant as possible sounds great to me, but without risking e.g. network issues keeping a session around for ever (well...). An alternative solution would be to not require any RTCP/RTSP packets from the client wile it's actually reading data that the server is sending. But if we can not send any more data for some reason (client closed it's window, network problems, ...)  then we need require RTCP/RTSP packets.

This could be achieved by restarting the timeout period not only when we receive RTCP/RTSP data from the client, but also when we send data to the client. If we can send then it's reading, right?

What do you think of this approach?
Comment 6 Wim Taymans 2014-09-16 07:10:46 UTC
I think that is the best, remove the code to prevent timeout, like your patch does. Then reset the timer everytime an RTP packet is sent (and RTCP/RTSP is receiver as would already be the case). I'm just a little concerned about the overhead of reseting the timer for each packet but maybe it's not so bad.
Comment 7 Sebastian Dröge (slomo) 2014-09-16 07:12:11 UTC
Comment on attachment 286176 [details] [review]
A possible fix for the described issue

Sounds like a good plan, yes
Comment 8 Branko Subasic 2014-09-19 16:51:21 UTC
Created attachment 286647 [details] [review]
Proposed patch 1/3.
Comment 9 Branko Subasic 2014-09-19 16:51:42 UTC
Created attachment 286648 [details] [review]
Proposed patch 2/3.
Comment 10 Branko Subasic 2014-09-19 16:52:00 UTC
Created attachment 286649 [details] [review]
Proposed patch 3/3.
Comment 11 Sebastian Dröge (slomo) 2014-09-24 07:39:01 UTC
commit 2745e6f65416de4b70e07875f00e94059c030821
Author: Branko Subasic <branko@axis.com>
Date:   Fri Sep 19 18:29:00 2014 +0200

    tests: Extend unit test timeout to accomodate for valgrind
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=736647

commit 2218510cae291dca6f27920583729f8cfaf276ff
Author: Branko Subasic <branko@axis.com>
Date:   Fri Sep 19 18:28:50 2014 +0200

    rtsp-*: Treat sending packets to clients as keepalive
    
    As long as gst-rtsp-server can successfully send RTP/RTCP data to
    clients then the client must be reading. This change makes the server
    timeout the connection if the client stops reading.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=736647

commit 733ef1162b78809751ccb327cbd0b07f46e6a42e
Author: Branko Subasic <branko@axis.com>
Date:   Fri Sep 19 18:28:30 2014 +0200

    rtsp-client: Allow backlog to grow while expiring session
    
    Allow the send backlog in the RTSP watch to grow to unlimited size while
    attempting to bring the media pipeline to NULL due to a session
    expiring.  Without this change the appsink element cannot change state
    because it is blocked while rendering data in the new_sample callback.
    This callback will block until it has successfully put the data into the
    send backlog. There is a chance that the send backlog is full at this
    point which means that the callback may block for a long time, possibly
    forever. Therefore the media pipeline may also be prevented from
    changing state for a long time.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=736647