[vlc-devel] [RFC PATCH 1/2] packetizer: h264: also update dec extra from pf_packetize

[Denis Charmet]

Hi,

Le lundi 20 avril 2015 à 08:18:06, Thomas Guillem a écrit :
> See #14429
> ---
>  modules/packetizer/h264.c | 103 ++++++++++++++++++++++++++++------------------
>  1 file changed, 64 insertions(+), 39 deletions(-)
> 
> diff --git a/modules/packetizer/h264.c b/modules/packetizer/h264.c
> index d89748f..189f5c5 100644
> --- a/modules/packetizer/h264.c
> +++ b/modules/packetizer/h264.c
> @@ -186,6 +186,7 @@ static block_t *CreateAnnexbNAL( decoder_t *, const uint8_t *p, int );
>  static block_t *OutputPicture( decoder_t *p_dec );
>  static void PutSPS( decoder_t *p_dec, block_t *p_frag );
>  static void PutPPS( decoder_t *p_dec, block_t *p_frag );
> +static int FillDecExtra( decoder_t *p_dec );
>  static void ParseSlice( decoder_t *p_dec, bool *pb_new_picture, slice_t *p_slice,
>                          int i_nal_ref_idc, int i_nal_type, const block_t *p_frag );
>  static void ParseSei( decoder_t *, block_t * );
> @@ -320,44 +321,7 @@ static int Open( vlc_object_t *p_this )
>          p_dec->fmt_out.i_extra = 0;
>          p_dec->fmt_out.p_extra = NULL;
>  
> -        /* Set the new extradata */
> -        for( i = 0; i < SPS_MAX; i++ )
> -        {
> -            if( p_sys->pp_sps[i] )
> -                p_dec->fmt_out.i_extra += p_sys->pp_sps[i]->i_buffer;
> -        }
> -        for( i = 0; i < PPS_MAX; i++ )
> -        {
> -            if( p_sys->pp_pps[i] )
> -                p_dec->fmt_out.i_extra += p_sys->pp_pps[i]->i_buffer;
> -        }
> -        p_dec->fmt_out.p_extra = malloc( p_dec->fmt_out.i_extra );
> -        if( p_dec->fmt_out.p_extra )
> -        {
> -            uint8_t *p_dst = p_dec->fmt_out.p_extra;
> -
> -            for( i = 0; i < SPS_MAX; i++ )
> -            {
> -                if( p_sys->pp_sps[i] )
> -                {
> -                    memcpy( p_dst, p_sys->pp_sps[i]->p_buffer, p_sys->pp_sps[i]->i_buffer );
> -                    p_dst += p_sys->pp_sps[i]->i_buffer;
> -                }
> -            }
> -            for( i = 0; i < PPS_MAX; i++ )
> -            {
> -                if( p_sys->pp_pps[i] )
> -                {
> -                    memcpy( p_dst, p_sys->pp_pps[i]->p_buffer, p_sys->pp_pps[i]->i_buffer );
> -                    p_dst += p_sys->pp_pps[i]->i_buffer;
> -                }
> -            }
> -            p_sys->b_header = true;
> -        }
> -        else
> -        {
> -            p_dec->fmt_out.i_extra = 0;
> -        }
> +        FillDecExtra( p_dec );
>  
>          /* Set callback */
>          p_dec->pf_packetize = PacketizeAVC1;
> @@ -490,6 +454,7 @@ static block_t *PacketizeAVC1( decoder_t *p_dec, block_t **pp_block )
>          p += i_size;
>      }
>      block_Release( p_block );
> +    FillDecExtra( p_dec );
>  
>      return p_ret;
>  }
> @@ -552,7 +517,9 @@ static block_t *PacketizeParse( void *p_private, bool *pb_ts_used, block_t *p_bl
>      while( p_block->i_buffer > 5 && p_block->p_buffer[p_block->i_buffer-1] == 0x00 )
>          p_block->i_buffer--;
>  
> -    return ParseNALBlock( p_dec, pb_ts_used, p_block );
> +    p_block = ParseNALBlock( p_dec, pb_ts_used, p_block );
> +    FillDecExtra( p_dec );
Why doing that for each packet and not only when you get a parameter set
ie inside ParseNALBlock or even in PutSPS/PutPPS?
> +    return p_block;
>  }
>  static int PacketizeValidate( void *p_private, block_t *p_au )
>  {
> @@ -1142,6 +1109,64 @@ static void PutPPS( decoder_t *p_dec, block_t *p_frag )
>      p_sys->pp_pps[i_pps_id] = p_frag;
>  }
>  
> +static int FillDecExtra( decoder_t *p_dec )
<nitpicking> there is few point to return anything  if you don't check
the result </nitpicking>
> +{
> +    decoder_sys_t *p_sys = p_dec->p_sys;
> +    int i, i_new_extra = 0;
> +
> +    if( !p_sys->b_sps && !p_sys->b_pps )
> +        return 0;
> +
> +    /* Set the new extradata */
> +    for( i = 0; i < SPS_MAX; i++ )
> +    {
> +        if( p_sys->pp_sps[i] )
> +            i_new_extra += p_sys->pp_sps[i]->i_buffer;
> +    }
> +    for( i = 0; i < PPS_MAX; i++ )
> +    {
> +        if( p_sys->pp_pps[i] )
> +            i_new_extra += p_sys->pp_pps[i]->i_buffer;
> +    }
> +
> +    /* Change extra only if size changes */
This is not enough it may work out of luck but it will fail to detect
all the changes.
> +    if( i_new_extra == p_dec->fmt_out.i_extra )
> +        return 0;
> +
> +    p_dec->fmt_out.i_extra = i_new_extra;
> +    free( p_dec->fmt_out.p_extra );
> +
> +    p_dec->fmt_out.p_extra = malloc( p_dec->fmt_out.i_extra );
> +    if( p_dec->fmt_out.p_extra )
> +    {
> +        uint8_t *p_dst = p_dec->fmt_out.p_extra;
> +
> +        for( i = 0; i < SPS_MAX; i++ )
> +        {
> +            if( p_sys->pp_sps[i] )
> +            {
> +                memcpy( p_dst, p_sys->pp_sps[i]->p_buffer, p_sys->pp_sps[i]->i_buffer );
> +                p_dst += p_sys->pp_sps[i]->i_buffer;
> +            }
> +        }
> +        for( i = 0; i < PPS_MAX; i++ )
> +        {
> +            if( p_sys->pp_pps[i] )
> +            {
> +                memcpy( p_dst, p_sys->pp_pps[i]->p_buffer, p_sys->pp_pps[i]->i_buffer );
> +                p_dst += p_sys->pp_pps[i]->i_buffer;
> +            }
> +        }
> +        p_sys->b_header = true;
> +        return 0;
> +    }
> +    else
> +    {
> +        p_dec->fmt_out.i_extra = 0;
> +        return -1;
> +    }
> +}
> +
>  static void ParseSlice( decoder_t *p_dec, bool *pb_new_picture, slice_t *p_slice,
>                          int i_nal_ref_idc, int i_nal_type, const block_t *p_frag )
>  {
> -- 
> 2.1.3
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel

Regards,

-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre