[FFmpeg-devel] [PATCH] avformat: close parser if codec changed

[Michael Niedermayer]

On Sat, Oct 22, 2016 at 01:16:00AM +0200, Andreas Cadhalpun wrote:
> On 22.10.2016 00:18, Michael Niedermayer wrote:
> > On Mon, Oct 17, 2016 at 08:49:23PM +0200, Andreas Cadhalpun wrote:
> >> The parser depends on the codec and thus must not be used with a different one.
> >> If it is, the 'avctx->codec_id == s->parser->codec_ids[0] ...' assert in
> >> av_parser_parse2 gets triggered.
> >>
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavformat/utils.c | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> > 
> > This changes the audio output from
> > http://samples.ffmpeg.org/MPEG2/vid_0x80.ts
> > 
> > is that intended ?
> 
> Nice catch: it shouldn't change, of course.
> 
> While processing that sample only the codec_tag gets changed, which is no reason to
> close the parser. That is only necessary, when the codec_id changes.
> 
> Fixed patch is attached.
> 
> Best regards,
> Andreas
> 

>  utils.c |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
> ffefc22756b774cb7652587207ae66cfbf681be3  0001-avformat-close-parser-if-codec-changed.patch
> From 9de87a4fb2c6c6311a11a2da5de8554a71adfa66 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Mon, 17 Oct 2016 20:26:51 +0200
> Subject: [PATCH] avformat: close parser if codec changed
> 
> The parser depends on the codec and thus must not be used with a different one.
> If it is, the 'avctx->codec_id == s->parser->codec_ids[0] ...' assert in
> av_parser_parse2 gets triggered.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavformat/utils.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 70dbfa1..a8a78ed 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -480,6 +480,12 @@ static int update_stream_avctx(AVFormatContext *s)
>          if (!st->internal->need_context_update)
>              continue;
>  
> +        /* close parser, because it depends on the codec */
> +        if (st->parser && st->internal->avctx->codec_id != st->codecpar->codec_id) {
> +            av_parser_close(st->parser);
> +            st->parser = NULL;
> +        }
> +
>          /* update internal codec context, for the parser */
>          ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
>          if (ret < 0)
> @@ -1515,6 +1521,12 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>                  st->info->found_decoder = 0;
>              }
>  
> +            /* close parser, because it depends on the codec */
> +            if (st->parser && st->internal->avctx->codec_id != st->codecpar->codec_id) {
> +                av_parser_close(st->parser);
> +                st->parser = NULL;
> +            }
> +

what if the codec id differs but both are supported by the parser ?
AVCodecParser has a list of 5 codec ids ?

I didnt find a testcase where this makes a difference, just wondering

the patch seems fine otherwise

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161102/1329eee9/attachment.sig>