[Bug 701587] rtsp-client: send new-session signal at the right time.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=701587
  GStreamer | gst-rtsp-server | 1.0.6

--- Comment #4 from Wim Taymans <wim.taymans at gmail.com> 2013-06-20 08:30:58 UTC ---
(In reply to comment #3)
> @@ -1252,6 +1277,18 @@ handle_setup_request (GstRTSPClient * client,
> GstRTSPClientState * state)
> 
>      /* we need a new media configuration in this session */
>      sessmedia = NULL;
> +
> +    /* find the session in the session pool */
> +    sessid = gst_rtsp_session_get_sessionid (session);
> +    if (!(session = gst_rtsp_session_pool_find (priv->session_pool, sessid)))
> +      goto no_session;
> 
> I realize, that these code line are not really need. However, I do not
> understand how the code would result in a leaked (previous?) session. Please
> explain.
> 
In the lines above, there is a call to gst_rtsp_session_pool_create(), which
gives you a ref to a session that you need to unref. Your code then does
gst_rtsp_session_pool_find() and overwrites the variable you were supposed to
unref.

> 
> static void
>  handle_request (GstRTSPClient * client, GstRTSPMessage * request)
> @@ -1672,10 +1693,6 @@ handle_request (GstRTSPClient * client, GstRTSPMessage *
> request)
>      if (!(session = gst_rtsp_session_pool_find (priv->session_pool, sessid)))
>        goto session_not_found;
> 
> -    /* we add the session to the client list of watched sessions. When a
> session
> -     * disappears because it times out, we will be notified. If all sessions
> are
> -     * gone, we will close the connection */
> -    client_watch_session (client, session);
>    }
> 
> I don't understand your comment.
> What's done here is that client_watch_session is called as soon as the SETUP
> request is received, just before the SETUP response is generated and sent to
> the client. Doing that, the new-session signal is sent at the right time and
> it's possible e.g, to configure session timeout (<=> we make sure that correct
> timeout parameter in the session response header is included). Without this
> patch, the new-signal is sent as soon as the received request includes the
> session header (<=> the SETUP request has been previously handled).

client_watch_session() makes sure that when a session is closed that we also
close the client connection and remove any refs to the session that we have.

In RTSP you can connect and do a setup (and make a session). Then you can close
the connection (and finalize the client) and reopen the connection (this makes
a new client) object. Then you can access your previous session by sessionid.
What we need to do is watch this old previous session in the new client so that
when it disappears we can close the client. The key is that there is no 1-to-1
relation between client and session.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.