[FFmpeg-devel] [PATCH 2/2 v2] avformat/demux: extract extradata from packets when context update is requested

[Andreas Rheinhardt]

James Almer:
> On 4/21/2024 12:32 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> If the demuxer doesn't set extradata in the stream's codecpar, a
>>> need_context_update request will delete the previously extracted
>>> extradata in
>>> the stream's internal AVCodecContext.
>>> As we can't ensure the old extradata is valid for the stream in its post
>>> context update request state, try to get extradata from the new
>>> packet instead
>>> of attempting to preserve the old in some form.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>   libavformat/demux.c | 12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/demux.c b/libavformat/demux.c
>>> index abfd5fee7d..39aa1cd4e3 100644
>>> --- a/libavformat/demux.c
>>> +++ b/libavformat/demux.c
>>> @@ -1319,6 +1319,8 @@ fail:
>>>       return ret;
>>>   }
>>>   +static int extract_extradata(FFFormatContext *si, AVStream *st,
>>> const AVPacket *pkt);
>>> +
>>>   static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>>>   {
>>>       FFFormatContext *const si = ffformatcontext(s);
>>> @@ -1373,6 +1375,11 @@ static int read_frame_internal(AVFormatContext
>>> *s, AVPacket *pkt)
>>>                   return ret;
>>>               }
>>>   +            if (!sti->avctx->extradata &&
>>> +                (ret = extract_extradata(si, st, pkt)) < 0) {
>>> +                av_packet_unref(pkt);
>>> +                return ret;
>>> +            }
>>>               sti->codec_desc =
>>> avcodec_descriptor_get(sti->avctx->codec_id);
>>>                 sti->need_context_update = 0;
>>> @@ -2470,6 +2477,8 @@ static int extract_extradata(FFFormatContext
>>> *si, AVStream *st, const AVPacket *
>>>       if (ret < 0)
>>>           return ret;
>>>   +    av_bsf_flush(sti->extract_extradata.bsf);
>>> +
>>>       ret = av_bsf_send_packet(sti->extract_extradata.bsf, pkt_ref);
>>>       if (ret < 0) {
>>>           av_packet_unref(pkt_ref);
>>> @@ -3090,7 +3099,8 @@ find_stream_info_err:
>>>           err = codec_close(sti);
>>>           if (err < 0 && ret >= 0)
>>>               ret = err;
>>> -        av_bsf_free(&sti->extract_extradata.bsf);
>>> +        if (sti->extract_extradata.bsf)
>>> +            av_bsf_flush(sti->extract_extradata.bsf);
>>>       }
>>>       if (ic->pb) {
>>>           FFIOContext *const ctx = ffiocontext(ic->pb);
>>
>> This will keep the BSF around for longer than necessary, although only a
>> tiny minority of demuxers ever set need_context_update at all.
> 
> It's not exactly a massive context, so i don't think it's a problem. But
> if you prefer, i could add an FF_INFMT_FLAG_ for this.

Actually, it is worse: It is legal for the codec id to change upon
need_context_update and then the BSF would still be configured for the
old BSF.
Anyway, an FF_INFMT_FLAG flag is overkill for this: Just reset the
extract_extradata.inited flag.

- Andreas