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 8D9FA41D39 for ; Sun, 20 Mar 2022 22:01:54 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id C255368B0F9; Mon, 21 Mar 2022 00:01:51 +0200 (EET) Received: from mail-yw1-f177.google.com (mail-yw1-f177.google.com [209.85.128.177]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id A433268A126 for ; Mon, 21 Mar 2022 00:01:44 +0200 (EET) Received: by mail-yw1-f177.google.com with SMTP id 00721157ae682-2e5a8a8c1cdso129595537b3.3 for ; Sun, 20 Mar 2022 15:01:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=38RqWzb/AUaZ4zMYtcmSAmKgkDQxDlPvdx+XR+RJeOw=; b=E8P9ODWG4kwZtbTXhQcGHX45lxDW9epCkY5jDMcsM/5y2+aZazEWHfqd9ZU1CLOLsJ bBCdDGj7GmbFy6JtI7JDGlSOLxGpaklfr8u5L/Ngv2XJRSGEQ4YYfzxkkeZG1gW0/OsE RQh9tMpzkK8yKdby3OR3hyp3CCM4WPfxi+w6SKK0Ajj8BVII3nuAtErfbpK7fAotTFQP VEdX4XPTF6OG6wyL03d8Y4QixVyIAhPoPnH0HYbLSe+1lOWqdi/DelWDjto2TQQo1uyA 0tp18qr0znKec7LAITFcshpTuG0/QXiPAW+Mf6KEUPlGhEHK+YTQ+LXoIaBHz4Qla80B 3Qvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=38RqWzb/AUaZ4zMYtcmSAmKgkDQxDlPvdx+XR+RJeOw=; b=402Gs3tVr1jAWU2dzQ7qaZ3WZv1bXNpTZS1/9S6i2h5ORotRrNMBBHersNeVh27zCc lwbvbvHZU7/Yrs8b+JjOjX27f242MaK10hb02DIskWUXBFA+2fInUqRaKvVC7ZeKA7jT U9Y5up/HBL4yuVWMGuTSj4FnqiXH3frK85w5Qq1Axxu0StR47DLR9hEA80Gqa0p3qa9T Ua57zImsrDYj6fytYs1QxDaf621HjVW6kkLySKfK4nPsVjSvZod8OzpQkWXEppNj1oxX aoKzoZlcoA2GmVNTZq8ZvbKrbkKDGyXdSeJvweIyrOsWks3BgKt4wJzks/3Exs4qwEXB I5DQ== X-Gm-Message-State: AOAM533xFo/b4HEaKCYMZHNiQPkEfNAFsZm03rmXX8SdFl5k0d5zK5PR 94UDee/X74uk72mSJlewY2eiNc7WTHsAZjSWJIeskqdX X-Google-Smtp-Source: ABdhPJzDyCG1DjSpwT+XcxiygfFNBOnM1W1XeAIA5iw93L7ix0Om49M555LUU6t4lNO55p8uRtto8AOhMn3JVTTLKzU= X-Received: by 2002:a05:690c:9:b0:2dd:1de0:7b13 with SMTP id bc9-20020a05690c000900b002dd1de07b13mr22724465ywb.450.1647813702717; Sun, 20 Mar 2022 15:01:42 -0700 (PDT) MIME-Version: 1.0 References: <20220319030407.45503-1-jamrial@gmail.com> <7f619d76-a701-092f-863d-664eac5cd5ba@gmail.com> In-Reply-To: <7f619d76-a701-092f-863d-664eac5cd5ba@gmail.com> From: Hendrik Leppkes Date: Sun, 20 Mar 2022 23:01:29 +0100 Message-ID: To: FFmpeg development discussions and patches 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: On Sun, Mar 20, 2022 at 7:52 PM James Almer wrote: > > On 3/19/2022 4:50 AM, Hendrik Leppkes wrote: > > On Sat, Mar 19, 2022 at 4:04 AM James Almer wrote: > >> > >> 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); > > > > This feels pretty ugly and still a bit risky that any call to > > av_opt_free could invalidate data its not supposed to. Maybe we should > > have a flag for AVOptions instead where av_opt_free won't touch an > > entry, because its only there to set/get it, not manage its memory? > > Where would that flag be set? av_opt_free() takes none. And that > function exists purely to free strings, dictionaries, and now > uninitialize AVChannelLayout elements in a struct. If you don't want to > free what av_opt_set() allocated, you shouldn't call av_opt_free() at all. > On the AVOption element in the table, with the other AV_OPT_FLAG_* _______________________________________________ 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".