[vlc-devel] [PATCH 1/4] decoder: rearrange module load/unload

Rémi Denis-Courmont remi at remlab.net
Mon Sep 7 17:07:23 CEST 2015


Le 2015-09-07 17:53, Thomas Guillem a écrit :
> This will allow a decoder module to be loaded/unloaded more than one 
> time.
> ---
>  src/input/decoder.c | 152
> +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 91 insertions(+), 61 deletions(-)
>
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 0fad3f9..1473ac7 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -135,6 +135,89 @@ struct decoder_owner_sys_t
>  /* */
>  #define DECODER_SPU_VOUT_WAIT_DURATION ((int)(0.200*CLOCK_FREQ))
>
> +/**
> + * Load a decoder module
> + */
> +static void LoadDecoder( decoder_t *p_dec, bool b_packetizer,
> +                         const es_format_t *p_fmt )
> +{
> +    p_dec->b_frame_drop_allowed = true;
> +    p_dec->b_need_packetized = false;
> +    p_dec->i_extra_picture_buffers = 0;
> +    p_dec->p_description = NULL;
> +
> +    p_dec->pf_decode_audio = NULL;
> +    p_dec->pf_decode_video = NULL;
> +    p_dec->pf_decode_sub = NULL;
> +    p_dec->pf_get_cc = NULL;
> +    p_dec->pf_packetize = NULL;
> +
> +    es_format_Copy( &p_dec->fmt_in, p_fmt );
> +    memset( &p_dec->fmt_out, 0, sizeof(es_format_t) );
> +
> +    /* Find a suitable decoder/packetizer module */
> +    if( !b_packetizer )
> +        p_dec->p_module = module_need( p_dec, "decoder", "$codec", 
> false );
> +    else
> +        p_dec->p_module = module_need( p_dec, "packetizer",
> "$packetizer", false );
> +
> +    if( !p_dec->p_module )
> +        es_format_Clean( &p_dec->fmt_in );
> +}
> +
> +/**
> + * Unload a decoder module
> + */
> +static void UnloadDecoder( decoder_t *p_dec )
> +{
> +    module_unneed( p_dec, p_dec->p_module );
> +    p_dec->p_module = NULL;
> +
> +    if( p_dec->p_description )
> +        vlc_meta_Delete( p_dec->p_description );

Why do you clean p_module and not p_description? AFAICT, either both or 
none needs cleaning.

> +
> +    es_format_Clean( &p_dec->fmt_in );
> +    es_format_Clean( &p_dec->fmt_out );
> +}
> +
> +/**
> + * Load a packetizer module
> + *
> + * \return the fmt_out of the packetizer or p_fmt in case of 
> failure.
> + */
> +static const es_format_t *LoadPacketizer( decoder_t *p_dec,
> +                                          const es_format_t *p_fmt )
> +{
> +    decoder_owner_sys_t *p_owner = p_dec->p_owner;

This is a bit weird and ugly. I would avoid decoder specifics here and 
in the next function.

> +
> +    p_owner->p_packetizer =
> +        vlc_custom_create( p_dec, sizeof( decoder_t ), "packetizer" 
> );
> +    if( !p_owner->p_packetizer )
> +        return p_fmt;
> +
> +    LoadDecoder( p_owner->p_packetizer, true, p_fmt );
> +    if( !p_owner->p_packetizer->p_module )
> +    {
> +        vlc_object_release( p_owner->p_packetizer );
> +        p_owner->p_packetizer = NULL;
> +        return p_fmt;
> +    }
> +    else
> +        return &p_owner->p_packetizer->fmt_out;
> +}
> +
> +/**
> + * Unload a packetizer module
> + */
> +static void UnloadPacketizer( decoder_t *p_dec )
> +{
> +    decoder_owner_sys_t *p_owner = p_dec->p_owner;
> +
> +    UnloadDecoder( p_owner->p_packetizer );
> +    vlc_object_release( p_owner->p_packetizer );
> +    p_owner->p_packetizer = NULL;
> +}
> +
>  static void DecoderUpdateFormatLocked( decoder_t *p_dec )
>  {
>      decoder_owner_sys_t *p_owner = p_dec->p_owner;
> @@ -1474,28 +1557,11 @@ static decoder_t * CreateDecoder(
> vlc_object_t *p_parent,
>  {
>      decoder_t *p_dec;
>      decoder_owner_sys_t *p_owner;
> -    es_format_t null_es_format;
>
>      p_dec = vlc_custom_create( p_parent, sizeof( *p_dec ), "decoder" 
> );
>      if( p_dec == NULL )
>          return NULL;
>
> -    p_dec->b_frame_drop_allowed = true;
> -    p_dec->pf_decode_audio = NULL;
> -    p_dec->pf_decode_video = NULL;
> -    p_dec->pf_decode_sub = NULL;
> -    p_dec->pf_get_cc = NULL;
> -    p_dec->pf_packetize = NULL;
> -
> -    /* Initialize the decoder */
> -    p_dec->p_module = NULL;
> -
> -    memset( &null_es_format, 0, sizeof(es_format_t) );
> -    es_format_Copy( &p_dec->fmt_in, fmt );
> -    es_format_Copy( &p_dec->fmt_out, &null_es_format );
> -
> -    p_dec->p_description = NULL;
> -
>      /* Allocate our private structure for the decoder */
>      p_dec->p_owner = p_owner = malloc( sizeof( decoder_owner_sys_t ) 
> );
>      if( unlikely(p_owner == NULL) )
> @@ -1560,37 +1626,12 @@ static decoder_t * CreateDecoder(
> vlc_object_t *p_parent,
>      p_dec->pf_get_display_rate = DecoderGetDisplayRate;
>
>      /* Find a suitable decoder/packetizer module */
> -    if( !b_packetizer )
> -        p_dec->p_module = module_need( p_dec, "decoder", "$codec", 
> false );
> -    else
> -        p_dec->p_module = module_need( p_dec, "packetizer",
> "$packetizer", false );
> +    LoadDecoder( p_dec, b_packetizer, fmt );
>
>      /* Check if decoder requires already packetized data */
>      if( !b_packetizer &&
>          p_dec->b_need_packetized && !p_dec->fmt_in.b_packetized )
> -    {
> -        p_owner->p_packetizer =
> -            vlc_custom_create( p_parent, sizeof( decoder_t ),
> "packetizer" );
> -        if( p_owner->p_packetizer )
> -        {
> -            es_format_Copy( &p_owner->p_packetizer->fmt_in,
> -                            &p_dec->fmt_in );
> -
> -            es_format_Copy( &p_owner->p_packetizer->fmt_out,
> -                            &null_es_format );
> -
> -            p_owner->p_packetizer->p_module =
> -                module_need( p_owner->p_packetizer,
> -                             "packetizer", "$packetizer", false );
> -
> -            if( !p_owner->p_packetizer->p_module )
> -            {
> -                es_format_Clean( &p_owner->p_packetizer->fmt_in );
> -                vlc_object_release( p_owner->p_packetizer );
> -                p_owner->p_packetizer = NULL;
> -            }
> -        }
> -    }
> +        LoadPacketizer( p_dec );
>
>      /* Copy ourself the input replay gain */
>      if( fmt->i_cat == AUDIO_ES )
> @@ -1643,6 +1684,10 @@ static void DeleteDecoder( decoder_t * p_dec )
>               (char*)&p_dec->fmt_in.i_codec,
>               (unsigned)block_FifoCount( p_owner->p_fifo ) );
>
> +    const bool b_flush_spu = p_dec->fmt_out.i_cat == SPU_ES;
> +    if( p_dec->p_module )
> +        UnloadDecoder( p_dec );
> +
>      /* Free all packets still in the decoder fifo. */
>      block_FifoRelease( p_owner->p_fifo );
>
> @@ -1677,7 +1722,7 @@ static void DeleteDecoder( decoder_t * p_dec )
>  #endif
>      es_format_Clean( &p_owner->fmt );
>
> -    if( p_dec->fmt_out.i_cat == SPU_ES )
> +    if( b_flush_spu )
>      {
>          vout_thread_t *p_vout = input_resource_HoldVout(
> p_owner->p_resource );
>          if( p_vout )
> @@ -1688,23 +1733,11 @@ static void DeleteDecoder( decoder_t * p_dec 
> )
>          }
>      }
>
> -    es_format_Clean( &p_dec->fmt_in );
> -    es_format_Clean( &p_dec->fmt_out );
> -    if( p_dec->p_description )
> -        vlc_meta_Delete( p_dec->p_description );
>      if( p_owner->p_description )
>          vlc_meta_Delete( p_owner->p_description );
>
>      if( p_owner->p_packetizer )
> -    {
> -        module_unneed( p_owner->p_packetizer,
> -                       p_owner->p_packetizer->p_module );
> -        es_format_Clean( &p_owner->p_packetizer->fmt_in );
> -        es_format_Clean( &p_owner->p_packetizer->fmt_out );
> -        if( p_owner->p_packetizer->p_description )
> -            vlc_meta_Delete( p_owner->p_packetizer->p_description );
> -        vlc_object_release( p_owner->p_packetizer );
> -    }
> +        UnloadPacketizer( p_dec );
>
>      vlc_cond_destroy( &p_owner->wait_fifo );
>      vlc_cond_destroy( &p_owner->wait_acknowledge );
> @@ -1776,7 +1809,6 @@ static decoder_t *decoder_New( vlc_object_t
> *p_parent, input_thread_t *p_input,
>      if( vlc_clone( &p_dec->p_owner->thread, DecoderThread, p_dec,
> i_priority ) )
>      {
>          msg_Err( p_dec, "cannot spawn decoder thread" );
> -        module_unneed( p_dec, p_dec->p_module );
>          DeleteDecoder( p_dec );
>          return NULL;
>      }
> @@ -1833,8 +1865,6 @@ void input_DecoderDelete( decoder_t *p_dec )
>
>      vlc_join( p_owner->thread, NULL );
>
> -    module_unneed( p_dec, p_dec->p_module );
> -
>      /* */
>      if( p_dec->p_owner->cc.b_supported )
>      {

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


More information about the vlc-devel mailing list