| Message ID | 20210310120332.27225-4-anton@khirnov.net |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/7] lavc: factor out encoder init/validation from avcodec_open2() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure | warning | Failed to apply patch |
On 3/10/2021 9:03 AM, Anton Khirnov wrote: > --- > libavcodec/decode.c | 8 +++++++- > libavcodec/decode.h | 6 ------ > libavcodec/utils.c | 6 ------ > 3 files changed, 7 insertions(+), 13 deletions(-) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index e5a301ec58..d25b15e95a 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt) > return 0; > } > > -int ff_decode_bsfs_init(AVCodecContext *avctx) > +static int decode_bsfs_init(AVCodecContext *avctx) > { > AVCodecInternal *avci = avctx->internal; > int ret; > @@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags) > > int ff_decode_preinit(AVCodecContext *avctx) > { > + int ret = 0; > + > /* if the decoder init function was already called previously, > * free the already allocated subtitle_header before overwriting it */ > av_freep(&avctx->subtitle_header); > @@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS > avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS; > } > > + ret = decode_bsfs_init(avctx); Did you try to run fate with THREADS set to something other than 1? This moves the bsfs initialization after ff_thread_init(), so all the copies of avctx in frame threading scenarios will be missing the bsfs requested by the decoder. fate-vp9 should be enough to confirm this, since afaik it's the only frame threaded decoder that uses a bsf. > + if (ret < 0) > + return ret; > + > return 0; > } > diff --git a/libavcodec/decode.h b/libavcodec/decode.h > index a865fe954f..1467b1eb33 100644 > --- a/libavcodec/decode.h > +++ b/libavcodec/decode.h > @@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt); > */ > int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame); > > -/** > - * Called during avcodec_open2() to initialize avctx->internal->bsf. > - * The bsf should be freed with av_bsf_free(). > - */ > -int ff_decode_bsfs_init(AVCodecContext *avctx); > - > /** > * Make sure avctx.hw_frames_ctx is set. If it's not set, the function will > * try to allocate it from hw_device_ctx. If that is not possible, an error > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 6d5aeb4eaf..918cb1b4d1 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -730,12 +730,6 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code > goto free_and_end; > } > > - if (av_codec_is_decoder(avctx->codec)) { > - ret = ff_decode_bsfs_init(avctx); > - if (ret < 0) > - goto free_and_end; > - } > - > if (HAVE_THREADS > && !(avci->frame_thread_encoder && (avctx->active_thread_type&FF_THREAD_FRAME))) { > ret = ff_thread_init(avctx); >
James Almer: > On 3/10/2021 9:03 AM, Anton Khirnov wrote: >> --- >> libavcodec/decode.c | 8 +++++++- >> libavcodec/decode.h | 6 ------ >> libavcodec/utils.c | 6 ------ >> 3 files changed, 7 insertions(+), 13 deletions(-) >> >> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >> index e5a301ec58..d25b15e95a 100644 >> --- a/libavcodec/decode.c >> +++ b/libavcodec/decode.c >> @@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal >> *avci, const AVPacket *pkt) >> return 0; >> } >> -int ff_decode_bsfs_init(AVCodecContext *avctx) >> +static int decode_bsfs_init(AVCodecContext *avctx) >> { >> AVCodecInternal *avci = avctx->internal; >> int ret; >> @@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, >> AVFrame *frame, int flags) >> int ff_decode_preinit(AVCodecContext *avctx) >> { >> + int ret = 0; >> + >> /* if the decoder init function was already called previously, >> * free the already allocated subtitle_header before overwriting >> it */ >> av_freep(&avctx->subtitle_header); >> @@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS >> avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS; >> } >> + ret = decode_bsfs_init(avctx); > > Did you try to run fate with THREADS set to something other than 1? This > moves the bsfs initialization after ff_thread_init(), so all the copies > of avctx in frame threading scenarios will be missing the bsfs requested > by the decoder. > For frame threaded decoders the codec's decode function is directly run with the child AVCodecContext; they don't need the bsfs: The parent's bsf has already been run on the packets before the frame threaded code is reached. > fate-vp9 should be enough to confirm this, since afaik it's the only > frame threaded decoder that uses a bsf. > >> + if (ret < 0) >> + return ret; >> + >> return 0; >> } >> diff --git a/libavcodec/decode.h b/libavcodec/decode.h >> index a865fe954f..1467b1eb33 100644 >> --- a/libavcodec/decode.h >> +++ b/libavcodec/decode.h >> @@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, >> AVPacket *pkt); >> */ >> int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame); >> -/** >> - * Called during avcodec_open2() to initialize avctx->internal->bsf. >> - * The bsf should be freed with av_bsf_free(). >> - */ >> -int ff_decode_bsfs_init(AVCodecContext *avctx); >> - >> /** >> * Make sure avctx.hw_frames_ctx is set. If it's not set, the >> function will >> * try to allocate it from hw_device_ctx. If that is not possible, >> an error >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >> index 6d5aeb4eaf..918cb1b4d1 100644 >> --- a/libavcodec/utils.c >> +++ b/libavcodec/utils.c >> @@ -730,12 +730,6 @@ int attribute_align_arg >> avcodec_open2(AVCodecContext *avctx, const AVCodec *code >> goto free_and_end; >> } >> - if (av_codec_is_decoder(avctx->codec)) { >> - ret = ff_decode_bsfs_init(avctx); >> - if (ret < 0) >> - goto free_and_end; >> - } >> - >> if (HAVE_THREADS >> && !(avci->frame_thread_encoder && >> (avctx->active_thread_type&FF_THREAD_FRAME))) { >> ret = ff_thread_init(avctx); >> > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 3/10/2021 10:52 AM, Andreas Rheinhardt wrote: > James Almer: >> On 3/10/2021 9:03 AM, Anton Khirnov wrote: >>> --- >>> libavcodec/decode.c | 8 +++++++- >>> libavcodec/decode.h | 6 ------ >>> libavcodec/utils.c | 6 ------ >>> 3 files changed, 7 insertions(+), 13 deletions(-) >>> >>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >>> index e5a301ec58..d25b15e95a 100644 >>> --- a/libavcodec/decode.c >>> +++ b/libavcodec/decode.c >>> @@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal >>> *avci, const AVPacket *pkt) >>> return 0; >>> } >>> -int ff_decode_bsfs_init(AVCodecContext *avctx) >>> +static int decode_bsfs_init(AVCodecContext *avctx) >>> { >>> AVCodecInternal *avci = avctx->internal; >>> int ret; >>> @@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, >>> AVFrame *frame, int flags) >>> int ff_decode_preinit(AVCodecContext *avctx) >>> { >>> + int ret = 0; >>> + >>> /* if the decoder init function was already called previously, >>> * free the already allocated subtitle_header before overwriting >>> it */ >>> av_freep(&avctx->subtitle_header); >>> @@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS >>> avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS; >>> } >>> + ret = decode_bsfs_init(avctx); >> >> Did you try to run fate with THREADS set to something other than 1? This >> moves the bsfs initialization after ff_thread_init(), so all the copies >> of avctx in frame threading scenarios will be missing the bsfs requested >> by the decoder. >> > > For frame threaded decoders the codec's decode function is directly run > with the child AVCodecContext; they don't need the bsfs: The parent's > bsf has already been run on the packets before the frame threaded code > is reached. Alright, should be ok, then. > >> fate-vp9 should be enough to confirm this, since afaik it's the only >> frame threaded decoder that uses a bsf. >> >>> + if (ret < 0) >>> + return ret; >>> + >>> return 0; >>> } >>> diff --git a/libavcodec/decode.h b/libavcodec/decode.h >>> index a865fe954f..1467b1eb33 100644 >>> --- a/libavcodec/decode.h >>> +++ b/libavcodec/decode.h >>> @@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, >>> AVPacket *pkt); >>> */ >>> int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame); >>> -/** >>> - * Called during avcodec_open2() to initialize avctx->internal->bsf. >>> - * The bsf should be freed with av_bsf_free(). >>> - */ >>> -int ff_decode_bsfs_init(AVCodecContext *avctx); >>> - >>> /** >>> * Make sure avctx.hw_frames_ctx is set. If it's not set, the >>> function will >>> * try to allocate it from hw_device_ctx. If that is not possible, >>> an error >>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c >>> index 6d5aeb4eaf..918cb1b4d1 100644 >>> --- a/libavcodec/utils.c >>> +++ b/libavcodec/utils.c >>> @@ -730,12 +730,6 @@ int attribute_align_arg >>> avcodec_open2(AVCodecContext *avctx, const AVCodec *code >>> goto free_and_end; >>> } >>> - if (av_codec_is_decoder(avctx->codec)) { >>> - ret = ff_decode_bsfs_init(avctx); >>> - if (ret < 0) >>> - goto free_and_end; >>> - } >>> - >>> if (HAVE_THREADS >>> && !(avci->frame_thread_encoder && >>> (avctx->active_thread_type&FF_THREAD_FRAME))) { >>> ret = ff_thread_init(avctx); >>> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index e5a301ec58..d25b15e95a 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -184,7 +184,7 @@ static int extract_packet_props(AVCodecInternal *avci, const AVPacket *pkt) return 0; } -int ff_decode_bsfs_init(AVCodecContext *avctx) +static int decode_bsfs_init(AVCodecContext *avctx) { AVCodecInternal *avci = avctx->internal; int ret; @@ -2010,6 +2010,8 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame, int flags) int ff_decode_preinit(AVCodecContext *avctx) { + int ret = 0; + /* if the decoder init function was already called previously, * free the already allocated subtitle_header before overwriting it */ av_freep(&avctx->subtitle_header); @@ -2046,5 +2048,9 @@ FF_ENABLE_DEPRECATION_WARNINGS avctx->export_side_data |= AV_CODEC_EXPORT_DATA_MVS; } + ret = decode_bsfs_init(avctx); + if (ret < 0) + return ret; + return 0; } diff --git a/libavcodec/decode.h b/libavcodec/decode.h index a865fe954f..1467b1eb33 100644 --- a/libavcodec/decode.h +++ b/libavcodec/decode.h @@ -69,12 +69,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt); */ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame); -/** - * Called during avcodec_open2() to initialize avctx->internal->bsf. - * The bsf should be freed with av_bsf_free(). - */ -int ff_decode_bsfs_init(AVCodecContext *avctx); - /** * Make sure avctx.hw_frames_ctx is set. If it's not set, the function will * try to allocate it from hw_device_ctx. If that is not possible, an error diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 6d5aeb4eaf..918cb1b4d1 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -730,12 +730,6 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code goto free_and_end; } - if (av_codec_is_decoder(avctx->codec)) { - ret = ff_decode_bsfs_init(avctx); - if (ret < 0) - goto free_and_end; - } - if (HAVE_THREADS && !(avci->frame_thread_encoder && (avctx->active_thread_type&FF_THREAD_FRAME))) { ret = ff_thread_init(avctx);