[vlc-devel] [RFC PATCH] decoder: separate vout initialization from buffer allocation

[Rémi Denis-Courmont]

Le mercredi 29 octobre 2014, 19:07:54 Thomas Guillem a écrit :
> ---
>  include/vlc_codec.h | 14 ++++++++++
>  src/input/decoder.c | 75
> +++++++++++++++++++++++++++++------------------------ 2 files changed, 55
> insertions(+), 34 deletions(-)
> 
> diff --git a/include/vlc_codec.h b/include/vlc_codec.h
> index 1e7c8a6..429cfa5 100644
> --- a/include/vlc_codec.h
> +++ b/include/vlc_codec.h
> @@ -97,6 +97,7 @@ struct decoder_t
>      /* Video output callbacks
>       * XXX use decoder_NewPicture/decoder_DeletePicture
>       * and decoder_LinkPicture/decoder_UnlinkPicture */
> +    int             (*pf_vout_format_update)( decoder_t * );
>      picture_t      *(*pf_vout_buffer_new)( decoder_t * );
>      void            (*pf_vout_buffer_del)( decoder_t *, picture_t * );
>      void            (*pf_picture_link)   ( decoder_t *, picture_t * );
> @@ -180,6 +181,19 @@ struct encoder_t
> 
> 
>  /**
> + * This function notifies the video output pipeline of a new video output
> + * format (fmt_out.video). If there is currently no video output or if the
> + * video output format has changed, a new audio video will be set up.
> + * @return 0 if the video output is working, -1 if not. */
> +static inline int decoder_UpdateVideoFormat( decoder_t *dec )
> +{
> +    if( dec->pf_vout_format_update != NULL )
> +        return dec->pf_vout_format_update( dec );
> +    else
> +        return -1;
> +}
> +
> +/**
>   * This function will return a new picture usable by a decoder as an output
> * buffer. You have to release it using decoder_DeletePicture or by
> returning * it to the caller as a pf_decode_video return value.
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 972522a..ca2ad88 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -53,6 +53,9 @@
> 
>  #include "../video_output/vout_control.h"
> 
> +static bool DecoderIsExitRequested( decoder_t *p_dec );
> +static bool DecoderIsFlushing( decoder_t *p_dec );
> +

This change should not be needed.

>  static decoder_t *CreateDecoder( vlc_object_t *, input_thread_t *,
>                                   es_format_t *, bool, input_resource_t *,
>                                   sout_instance_t *p_sout );
> @@ -66,7 +69,7 @@ static void       DecoderSignalWait( decoder_t *, bool );
>  static void       DecoderUnsupportedCodec( decoder_t *, vlc_fourcc_t );
> 
>  /* Buffers allocation callbacks for the decoders */
> -static picture_t *vout_new_buffer( decoder_t * );

You need to keep that.

> +static int vout_update_format( decoder_t * );
>  static void vout_del_buffer( decoder_t *, picture_t * );
>  static void vout_link_picture( decoder_t *, picture_t * );
>  static void vout_unlink_picture( decoder_t *, picture_t * );
> @@ -158,12 +161,38 @@ struct decoder_owner_sys_t
>  /**************************************************************************
> *** * Public functions
>  
> ***************************************************************************
> **/ -picture_t *decoder_NewPicture( decoder_t *p_decoder )
> +picture_t *decoder_NewPicture( decoder_t *p_dec )
>  {
> -    picture_t *p_picture = p_decoder->pf_vout_buffer_new( p_decoder );
> -    if( !p_picture )
> -        msg_Warn( p_decoder, "can't get output picture" );
> -    return p_picture;
> +    if( decoder_UpdateVideoFormat( p_dec ) ) {
> +        msg_Warn( p_dec, "can't get output picture" );
> +        return NULL;
> +    }

This seems correct up to this point. But down below you still need to call the 
pf_vout_buffer_new.

...

> @@ -778,7 +807,8 @@ static decoder_t * CreateDecoder( vlc_object_t
> *p_parent,
> 
>      /* Set buffers allocation callbacks for the decoders */
>      p_dec->pf_aout_format_update = aout_update_format;
> -    p_dec->pf_vout_buffer_new = vout_new_buffer;
> +    p_dec->pf_vout_format_update = vout_update_format;

That´s good but you also need to update transcode.

> +    p_dec->pf_vout_buffer_new = decoder_NewPicture;

No. decoder_NewPicture() is the wrapper around the callback, not the other way 
that´s used by codecs. This change probably breaks the video transcoder.

>      p_dec->pf_vout_buffer_del = vout_del_buffer;
>      p_dec->pf_picture_link    = vout_link_picture;
>      p_dec->pf_picture_unlink  = vout_unlink_picture;
> @@ -2027,7 +2057,7 @@ static int aout_update_format( decoder_t *p_dec )
>      return 0;
>  }
> 
> -static picture_t *vout_new_buffer( decoder_t *p_dec )
> +static int vout_update_format( decoder_t *p_dec )
>  {
>      decoder_owner_sys_t *p_owner = p_dec->p_owner;
> 
> @@ -2049,7 +2079,7 @@ static picture_t *vout_new_buffer( decoder_t *p_dec )
>              !p_dec->fmt_out.video.i_height )
>          {
>              /* Can't create a new vout without display size */
> -            return NULL;
> +            return -1;
>          }
> 
>          video_format_t fmt = p_dec->fmt_out.video;
> @@ -2153,33 +2183,10 @@ static picture_t *vout_new_buffer( decoder_t *p_dec
> ) {
>              msg_Err( p_dec, "failed to create video output" );
>              p_dec->b_error = true;
> -            return NULL;
> +            return -1;
>          }
>      }
> -
> -    /* Get a new picture
> -     */
> -    for( ;; )
> -    {
> -        if( DecoderIsExitRequested( p_dec ) || p_dec->b_error )
> -            return NULL;
> -
> -        picture_t *p_picture = vout_GetPicture( p_owner->p_vout );
> -        if( p_picture )
> -            return p_picture;
> -
> -        if( DecoderIsFlushing( p_dec ) )
> -            return NULL;
> -
> -        /* */
> -        DecoderSignalWait( p_dec, true );
> -
> -        /* Check the decoder doesn't leak pictures */
> -        vout_FixLeaks( p_owner->p_vout );
> -
> -        /* FIXME add a vout_WaitPictureAvailable (timedwait) */
> -        msleep( VOUT_OUTMEM_SLEEP );
> -    }

This needs to stay as vout_new_buffer().

> +    return 0;
>  }
> 
>  static void vout_del_buffer( decoder_t *p_dec, picture_t *p_pic )

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