| Message ID | 20240421151851.1165-1-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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@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. - Andreas
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@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.
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@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
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);
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@gmail.com> --- libavformat/demux.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)