[vlc-devel] [PATCH v2] demux: avformat: handle AV_PKT_DATA_NEW_EXTRADATA

[Rémi Denis-Courmont]

Le maanantaina 4. tammikuuta 2021, 12.39.00 EET Zhao Zhili a écrit :
> This is a best-effort solution. Seek may not work as expected.
> ---
> v2:
> 1. check on p_track->p_es before es_format_Copy
> 2. check ES_OUT_SET_ES_FMT return value
> 3. ignore new extradata which is the same as previous version
> 
>  modules/demux/avformat/demux.c | 38 +++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/modules/demux/avformat/demux.c b/modules/demux/avformat/demux.c
> index 4a765b7352..b0d231d160 100644
> --- a/modules/demux/avformat/demux.c
> +++ b/modules/demux/avformat/demux.c
> @@ -55,6 +55,7 @@ struct avformat_track_s
>  {
>      es_out_id_t *p_es;
>      vlc_tick_t i_pcr;
> +    es_format_t es_format;
>  };
> 
>  /**************************************************************************
> *** @@ -711,6 +712,8 @@ int avformat_OpenDemux( vlc_object_t *p_this )
> 
>              es_fmt.i_id = i;
>              p_track->p_es = es_out_Add( p_demux->out, &es_fmt );
> +            if( p_track->p_es != NULL )
> +                es_format_Copy( &p_track->es_format, &es_fmt );
>              if( p_track->p_es && (s->disposition & AV_DISPOSITION_DEFAULT)
> ) es_out_Control( p_demux->out, ES_OUT_SET_ES_DEFAULT, p_track->p_es );
> 
> @@ -766,7 +769,12 @@ void avformat_CloseDemux( vlc_object_t *p_this )
>      demux_t     *p_demux = (demux_t*)p_this;
>      demux_sys_t *p_sys = p_demux->p_sys;
> 
> -    free( p_sys->tracks );
> +    if( p_sys->tracks != NULL )

Should be redundant, as i_tracks ought to be 0 if tracks is NULL.

> +    {
> +        for( unsigned i = 0; i < p_sys->i_tracks; i++ )
> +            es_format_Clean( &p_sys->tracks[i].es_format );
> +        free( p_sys->tracks );
> +    }
> 
>      if( p_sys->ic )
>      {
> @@ -821,6 +829,34 @@ static int Demux( demux_t *p_demux )
>          av_packet_unref( &pkt );
>          return 1;
>      }
> +
> +    // handle extra data change, this can happen for FLV
> +    int side_data_size;
> +    uint8_t *side_data = av_packet_get_side_data( &pkt,
> AV_PKT_DATA_NEW_EXTRADATA, &side_data_size );
> +    if( side_data != NULL &&
> side_data_size > 0 ) {

Is side_data_size set to 0 on failure or is it undefined? It looks like the 
NULL check should be redundant here.

> +        // ignore new extradata which is the same as previous version
> +        if( side_data_size != p_track->es_format.i_extra ||
> +            memcmp( side_data, p_track->es_format.p_extra, side_data_size )
> != 0 ) +        {
> +            msg_Warn( p_demux, "New extra data found, seek may not work as
> expected" );
> +            void *p_extra = realloc(
> p_track->es_format.p_extra, side_data_size );
> +            if( p_extra == NULL )

unlikely()

> +            {
> +                av_packet_unref( &pkt );
> +                return 0;
> +            }

Could invert the predicate to factor code.

But anyway, it is probably better to clear p_extra and i_extra on error, than 
to leave mismatched parameters.

> +            p_track->es_format.p_extra = p_extra;
> +            memcpy( p_track->es_format.p_extra, side_data, side_data_size
> ); +            p_track->es_format.i_extra = side_data_size;
> +            if( es_out_Control( p_demux->out, ES_OUT_SET_ES_FMT,
> +                        p_track->p_es, &p_track->es_format ) != VLC_SUCCESS
> ) +            {
> +                av_packet_unref( &pkt );
> +                return 0;
> +            }
> +        }
> +    }
> +
>      if( p_stream->codecpar->codec_id == AV_CODEC_ID_SSA )
>      {
>          p_frame = BuildSsaFrame( &pkt, p_sys->i_ssa_order++ );


-- 
Реми Дёни-Курмон
http://www.remlab.net/