From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 9AFDE42618 for ; Mon, 21 Mar 2022 07:51:46 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9BDD768B15F; Mon, 21 Mar 2022 09:51:43 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id E0FD36881C8 for ; Mon, 21 Mar 2022 09:51:37 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id E5371240179 for ; Mon, 21 Mar 2022 08:51:36 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id ZYRO7xY9xwYi for ; Mon, 21 Mar 2022 08:51:36 +0100 (CET) Received: from lain.red.khirnov.net (lain.red.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.red.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 478962400F5 for ; Mon, 21 Mar 2022 08:51:36 +0100 (CET) Received: by lain.red.khirnov.net (Postfix, from userid 1000) id 844D91601AD; Mon, 21 Mar 2022 08:51:36 +0100 (CET) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: References: <20220319030407.45503-1-jamrial@gmail.com> =?utf-8?q?=3CAS1PR01M?= =?utf-8?q?B95649020ABFF3A60C4FDF2A98F159=40AS1PR01MB9564=2Eeurprd01=2Eprod?= =?utf-8?q?=2Eexchangelabs=2Ecom=3E?= Mail-Followup-To: FFmpeg development discussions and patches Date: Mon, 21 Mar 2022 08:51:36 +0100 Message-ID: <164784909651.6085.6378539386386313722@lain.red.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/avcodec: don't uninitialize ch_layout in avcodec_close() X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Quoting Hendrik Leppkes (2022-03-21 08:37:43) > On Mon, Mar 21, 2022 at 12:38 AM Andreas Rheinhardt > 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 > > > --- > > > 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 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".