[vlc-devel] [PATCH] UPDATED Handle GET_PARAMETER keepalive responses during PAUSE

[Jean-Baptiste Kempf]

This does not apply for me.

On 25 Jun, Paul Clark wrote :
> Fixes ticket#14939
> 
> Because Demux() is not called during PAUSE the GET_PARAMETER keepalives are
> sent by the TimeoutPrevention thread.  However this was not giving Live555
> a chance to handle the responses through doEventLoop(), and so the responses
> were queued up.  Because of a separate issue in handling pipelined
> GET_PARAMETER responses in Live555 itself (now fixed, 2015-06-24), after two
> GPs this would result in the loss of the PLAY response on resume, locking
> everything up.
> 
> Even with the fix to Live555, queuing up all GP responses until the stream
> is resumed is unpleasant and will eventually overflow Live555's response
> buffer.
> 
> This fix calls the usual Live555 response handling mechanism for GPs sent
> in the background thread during pause.  I have not added it to the normal
> GP sends in Demux() because that calls doEventLoop() anyway, and to do so
> would add a delay to normal processing.
> 
> TimeoutPrevention now requires access to p_demux itself, not just p_sys.
> 
> A mutex has been added to protect Live555 from simultaneous calls within
> Demux() or Control() and the TimeoutPrevention() thread
> ---
>  modules/access/live555.cpp | 33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/modules/access/live555.cpp b/modules/access/live555.cpp
> index fef590a..3bc7669 100644
> --- a/modules/access/live555.cpp
> +++ b/modules/access/live555.cpp
> @@ -172,7 +172,7 @@ typedef struct
> 
>  struct timeout_thread_t
>  {
> -    demux_sys_t  *p_sys;
> +    demux_t     *p_demux;
>      vlc_thread_t handle;
>      bool         b_handle_keep_alive;
>  };
> @@ -209,6 +209,7 @@ struct demux_sys_t
>      int              i_timeout;     /* session timeout value in seconds */
>      bool             b_timeout_call;/* mark to send an RTSP call to prevent
> server timeout */
>      timeout_thread_t *p_timeout;    /* the actual thread that makes sure we
> don't timeout */
> +    vlc_mutex_t      timeout_mutex; /* Serialise calls to live555 in
> timeout thread w.r.t. Demux()/Control() */
> 
>      /* */
>      bool             b_force_mcast;
> @@ -309,6 +310,7 @@ static int  Open ( vlc_object_t *p_this )
>      p_sys->psz_path = strdup( p_demux->psz_location );
>      p_sys->b_force_mcast = var_InheritBool( p_demux, "rtsp-mcast" );
>      p_sys->f_seek_request = -1;
> +    vlc_mutex_init(&p_sys->timeout_mutex);
> 
>      /* parse URL for rtsp://[user:[passwd]@]serverip:port/options */
>      vlc_UrlParse( &p_sys->url, p_sys->psz_path, 0 );
> @@ -455,6 +457,7 @@ static void Close( vlc_object_t *p_this )
>      free( p_sys->psz_path );
> 
>      vlc_UrlClean( &p_sys->url );
> +    vlc_mutex_destroy(&p_sys->timeout_mutex);
> 
>      free( p_sys );
>  }
> @@ -1235,7 +1238,7 @@ static int Play( demux_t *p_demux )
>              if( p_sys->p_timeout )
>              {
>                  memset( p_sys->p_timeout, 0, sizeof(timeout_thread_t) );
> -                p_sys->p_timeout->p_sys = p_demux->p_sys; /* lol, object
> recursion :D */
> +                p_sys->p_timeout->p_demux = p_demux;
>                  if( vlc_clone( &p_sys->p_timeout->handle,
> TimeoutPrevention,
>                                 p_sys->p_timeout, VLC_THREAD_PRIORITY_LOW )
> )
>                  {
> @@ -1273,6 +1276,10 @@ static int Demux( demux_t *p_demux )
>      bool            b_send_pcr = true;
>      int             i;
> 
> +    /* Protect Live555 from simultaneous calls in TimeoutPrevention()
> +       during pause */
> +    vlc_mutex_locker locker(&p_sys->timeout_mutex);
> +
>      /* Check if we need to send the server a Keep-A-Live signal */
>      if( p_sys->b_timeout_call && p_sys->rtsp && p_sys->ms )
>      {
> @@ -1428,6 +1435,8 @@ static int Control( demux_t *p_demux, int i_query,
> va_list args )
>      bool *pb, *pb2;
>      int *pi_int;
> 
> +    vlc_mutex_locker locker(&p_sys->timeout_mutex); /* (see same in Demux)
> */
> +
>      switch( i_query )
>      {
>          case DEMUX_GET_TIME:
> @@ -2108,21 +2117,35 @@ VLC_NORETURN
>  static void* TimeoutPrevention( void *p_data )
>  {
>      timeout_thread_t *p_timeout = (timeout_thread_t *)p_data;
> +    demux_t *p_demux = p_timeout->p_demux;
> +    demux_sys_t *p_sys = p_demux->p_sys;
> 
>      for( ;; )
>      {
>          /* Voodoo (= no) thread safety here! *Ahem* */
>          if( p_timeout->b_handle_keep_alive )
>          {
> +            /* Protect Live555 from us calling their functions
> simultaneously
> +               with Demux() or Control() */
> +            vlc_mutex_locker locker(&p_sys->timeout_mutex);
> +
>              char *psz_bye = NULL;
>              int canc = vlc_savecancel ();
> 
> - p_timeout->p_sys->rtsp->sendGetParameterCommand( *p_timeout->p_sys->ms,
> NULL, psz_bye );
> +            p_sys->rtsp->sendGetParameterCommand( *p_sys->ms,
> default_live555_callback, psz_bye );
> +
> +            if( !wait_Live555_response( p_demux ) )
> +            {
> +              msg_Err( p_demux, "GET_PARAMETER keepalive failed: %s",
> +                       p_sys->env->getResultMsg() );
> +              /* Just continue, worst case is we get timed out later */
> +            }
> +
>              vlc_restorecancel (canc);
>          }
> -        p_timeout->p_sys->b_timeout_call = !p_timeout->b_handle_keep_alive;
> +        p_sys->b_timeout_call = !p_timeout->b_handle_keep_alive;
> 
> -        msleep (((int64_t)p_timeout->p_sys->i_timeout - 2) * CLOCK_FREQ);
> +        msleep (((int64_t)p_sys->i_timeout - 2) * CLOCK_FREQ);
>      }
>      vlc_assert_unreachable(); /* dead code */
>  }
> -- 
> 2.1.4
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

-- 
With my kindest regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device