[vlc-devel] [PATCH 2/2] Wait-free snapshot handling in the vout thread

[basos g]

2009/3/10 Rémi Denis-Courmont <remi at remlab.net>:
> Using vlc_mutex_trylock(), we make sure that the video output thread
> will not yield because of an incomplete snapshot request.
>
> ---
>  src/video_output/video_output.c |   16 +++++++++-------
>  1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> index 27aad54..e12e033 100644
> --- a/src/video_output/video_output.c
> +++ b/src/video_output/video_output.c
> @@ -1191,11 +1191,13 @@ static void* RunThread( void *p_this )
>             p_filtered_picture = filter_chain_VideoFilter( p_vout->p->p_vf2_chain,
>                                                            p_picture );
>
> -        /* FIXME it is ugly that b_snapshot is not locked but I do not
> -         * know which lock to use (here and in the snapshot callback) */
> -        vlc_mutex_lock( &p_vout->p->snapshot.lock );
> -        const bool b_snapshot = p_vout->p->snapshot.i_request > 0 && p_picture != NULL;
> -        vlc_mutex_unlock( &p_vout->p->snapshot.lock );
> +        bool b_snapshot = false;
> +        if( vlc_mutex_trylock( &p_vout->p->snapshot.lock ) )
> +        {
> +             b_snapshot = p_vout->p->snapshot.i_request > 0
> +                       && p_picture != NULL;
> +             vlc_mutex_unlock( &p_vout->p->snapshot.lock );
> +        }
>
>         /*
>          * Check for subpictures to display
> @@ -1215,9 +1217,9 @@ static void* RunThread( void *p_this )
>         /*
>          * Take a snapshot if requested
>          */
> -        if( p_directbuffer && b_snapshot )
> +        if( p_directbuffer && b_snapshot
> +         && vlc_mutex_trylock( &p_vout->p->snapshot.lock ) )
>         {
> -            vlc_mutex_lock( &p_vout->p->snapshot.lock );
>             assert( p_vout->p->snapshot.i_request > 0 );
>             while( p_vout->p->snapshot.i_request > 0 )
>             {
> --
> 1.6.2
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
>

Nice though.
Why lock twice , though ?
Couldn't we write something like :

       /*
         * Take a snapshot if requested
         */

        if( p_directbuffer &&
            vlc_mutex_trylock( &p_vout->p->snapshot.lock ) )
        {
           if ( p_vout->p->snapshot.i_request > 0 )
          {
               while( p_vout->p->snapshot.i_request > 0 )
               {
                   picture_t *p_pic = picture_New( p_vout->fmt_out.i_chroma,
                                                   p_vout->fmt_out.i_width,
                                                   p_vout->fmt_out.i_height,
                                                   p_vout->fmt_out.i_aspect  );
                   if( !p_pic )
                       break;

                   picture_Copy( p_pic, p_directbuffer );

                   p_pic->p_next = p_vout->p->snapshot.p_picture;
                   p_vout->p->snapshot.p_picture = p_pic;
                   p_vout->p->snapshot.i_request--;
               } //end while
               vlc_cond_broadcast( &p_vout->p->snapshot.wait );
         } //end if i_request>0
          vlc_mutex_unlock( &p_vout->p->snapshot.lock );
      } //end if mutext try lock