[FFmpeg-devel] [PATCH] flvdec: set need_context_update when changing codec id

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Fri Nov 25 01:44:40 EET 2016


On 24.11.2016 16:31, Michael Niedermayer wrote:
> On Thu, Nov 24, 2016 at 01:37:17AM +0100, Andreas Cadhalpun wrote:
>> On 23.11.2016 03:26, Michael Niedermayer wrote:
>>> On Fri, Nov 04, 2016 at 10:28:20PM +0100, Andreas Cadhalpun wrote:
>>>> Otherwise the codec context and codecpar might disagree on the codec id,
>>>> triggering asserts in av_parser_parse2.
>>>>
>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>> ---
>>>>  libavformat/flvdec.c | 16 ++++++++++++----
>>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
>>>> index e53c345..4ba7fc8 100644
>>>> --- a/libavformat/flvdec.c
>>>> +++ b/libavformat/flvdec.c
>>>> @@ -289,7 +289,9 @@ static int flv_same_video_codec(AVCodecParameters *vpar, int flags)
>>>>  static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
>>>>                                 int flv_codecid, int read)
>>>>  {
>>>> +    int ret = 0;
>>>>      AVCodecParameters *par = vstream->codecpar;
>>>> +    enum AVCodecID old_codec_id = vstream->codecpar->codec_id;
>>>>      switch (flv_codecid) {
>>>>      case FLV_CODECID_H263:
>>>>          par->codec_id = AV_CODEC_ID_FLV1;
>>>> @@ -317,20 +319,26 @@ static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
>>>>              else
>>>>                  avio_skip(s->pb, 1);
>>>>          }
>>>> -        return 1;     // 1 byte body size adjustment for flv_read_packet()
>>>> +        ret = 1;     // 1 byte body size adjustment for flv_read_packet()
>>>> +        break;
>>>>      case FLV_CODECID_H264:
>>>>          par->codec_id = AV_CODEC_ID_H264;
>>>>          vstream->need_parsing = AVSTREAM_PARSE_HEADERS;
>>>> -        return 3;     // not 4, reading packet type will consume one byte
>>>> +        ret = 3;     // not 4, reading packet type will consume one byte
>>>> +        break;
>>>>      case FLV_CODECID_MPEG4:
>>>>          par->codec_id = AV_CODEC_ID_MPEG4;
>>>> -        return 3;
>>>> +        ret = 3;
>>>> +        break;
>>>>      default:
>>>>          avpriv_request_sample(s, "Video codec (%x)", flv_codecid);
>>>>          par->codec_tag = flv_codecid;
>>>>      }
>>>>  
>>>> -    return 0;
>>>> +    if (par->codec_id != old_codec_id)
>>>> +        vstream->internal->need_context_update = 1;
>>>
>>> If this occurs only for fuzzed samples then rejecting the change
>>> with a request for a sample seems better
>>>
>>> changing teh codec_id midstream like this could, seems problematic
>>> changing at at header time might be ok if that works better than not
>>> changing it for some real sample
>>
>> This happens for almost every file, because at the beginning the codec_id
>> is AV_CODEC_ID_NONE. However, usually need_context_update is already
>> set from avformat_new_stream, so a change can be rejected, if this
>> is not the case. Patch doing that attached.
> 
> the code i tested prviously checked for AV_CODEC_ID_NONE

Only checking for AV_CODEC_ID_NONE is not sufficient, though, because it
does not guarantee that a newly set codec_id will actually be propagated
to the internal context. This only happens if need_context_update is set.

> but this is probably ok too

Pushed.

Do you think the ogg parsers should be handled similarly?
Because I'm similarly not sure if changing the codec parameters midstream
works for these.

Best regards,
Andreas


More information about the ffmpeg-devel mailing list