Quoting Hendrik Leppkes (2022-03-21 08:37:43) > On Mon, Mar 21, 2022 at 12:38 AM Andreas Rheinhardt > <[email protected]> wrote: > > > > James Almer: > > > The function is not meant to clear codec parameters, and the lavf demux > > > code > > > relies on this behavior. > > > Regression since 327efa66331ebdc0087c6b656059a8df2f404019. > > > > > > Signed-off-by: James Almer <[email protected]> > > > --- > > > libavcodec/avcodec.c | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c > > > index 38bdaad4fa..253c9f56cc 100644 > > > --- a/libavcodec/avcodec.c > > > +++ b/libavcodec/avcodec.c > > > @@ -469,6 +469,7 @@ void avsubtitle_free(AVSubtitle *sub) > > > > > > av_cold int avcodec_close(AVCodecContext *avctx) > > > { > > > + AVChannelLayout ch_layout; > > > int i; > > > > > > if (!avctx) > > > @@ -524,7 +525,12 @@ av_cold int avcodec_close(AVCodecContext *avctx) > > > > > > if (avctx->priv_data && avctx->codec && avctx->codec->priv_class) > > > av_opt_free(avctx->priv_data); > > > + /* av_opt_free() will uninitialize avctx->ch_layout, but we want to > > > keep it. > > > + It will be uninitialized in avcodec_free_context() */ > > > + ch_layout = avctx->ch_layout; > > > + memset(&avctx->ch_layout, 0, sizeof(avctx->ch_layout)); > > > av_opt_free(avctx); > > > + avctx->ch_layout = ch_layout; > > > av_freep(&avctx->priv_data); > > > if (av_codec_is_encoder(avctx->codec)) { > > > av_freep(&avctx->extradata); > > > > avcodec_close() has always* called av_opt_free() and therefore > > uninitialized allocated options (the documentation of avcodec_close() > > states that it "frees all the data associated with it"); the new channel > > layout API is (potentially) based on allocations, so ch_layout belongs > > to this group of elements. > > If the demux code wants to preserve ch_layout, then the demux code > > should do it itself; it should not life in avcodec_close() (where it > > would be a hack). > > > > But is it expected that this is going to happen? It doesn't reset any > other codec properties, and if I set ch_layout without using avoptions > (you know, like 99% of all callers, if you use an codec context in > code then avoptions are rather clumsy), would I expect it to be reset? > ch_layout just happens to get reset because it potentially has > allocations, not because every codec property gets reset, its a > technical detail not a logical conclusion. I think there is such a > thing as being too strict about weird semantics here.
People should not use avcodec_close() anyway. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list [email protected] https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email [email protected] with subject "unsubscribe".
