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

[Rémi Denis-Courmont]

Le mercredi 24 juin 2015, 18:18:03 Paul Clark a écrit :
> 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.
> ---
>   modules/access/live555.cpp | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/modules/access/live555.cpp b/modules/access/live555.cpp
> index fef590a..517ce58 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;
>   };
> @@ -1235,7 +1235,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 ) )
>                   {
> @@ -2108,6 +2108,8 @@ 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( ;; )
>       {
> @@ -2117,12 +2119,20 @@ static void* TimeoutPrevention( void *p_data )
>               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 ) )

Are you sure this function is re-entrant? Seems like nothing good will happen 
if the timeout thread and the input thread end up waiting at the same time.

> +            {
> +              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 */
>   }

-- 
Rémi Denis-Courmont
http://www.remlab.net/