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

[Andreas Cadhalpun]

On 02.11.2016 13:07, Michael Niedermayer wrote:
> On Sat, Oct 22, 2016 at 01:16:00AM +0200, Andreas Cadhalpun wrote:
>>  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

I think the parser should be re-initialized in that case, too.
For example the ac3 parser stores the codec_id in the parser context with:
    if(hdr.bitstream_id>10)
        hdr_info->codec_id = AV_CODEC_ID_EAC3;
    else if (hdr_info->codec_id == AV_CODEC_ID_NONE)
        hdr_info->codec_id = AV_CODEC_ID_AC3;

So if it is first AV_CODEC_ID_EAC3 and then AV_CODEC_ID_AC3, this wouldn't
be updated and the parser would change the avctx codec_id back, which
seems wrong.

> the patch seems fine otherwise

OK, pushed.

Best regards,
Andreas