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 432E141D13 for ; Sun, 20 Mar 2022 18:52:03 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8BA5A68B058; Sun, 20 Mar 2022 20:52:01 +0200 (EET) Received: from mail-oo1-f42.google.com (mail-oo1-f42.google.com [209.85.161.42]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6EE8968A7B8 for ; Sun, 20 Mar 2022 20:51:55 +0200 (EET) Received: by mail-oo1-f42.google.com with SMTP id o22-20020a4ad156000000b00324910b18d2so883178oor.7 for ; Sun, 20 Mar 2022 11:51:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :references:from:in-reply-to:content-transfer-encoding; bh=yH7mgMvRclpeghEGmXHVXbP7gTpnS4xJGtRwz3riN98=; b=U4lSV7H1lqUKdwW0wogfGreOfzqHtrfbf66gbzQmU61YQa7VeoBKlIKpppxJDfkzi8 ulN5+aub+ESsjjPqP0ta76YWtYTmwANvrMrcE5u9gDiTlfdAX1Bg9AgbO2AHnGEPMc9O EL2flqCwaHfncDkDpci5tAhh2JEGs3LpbEeiB5taLaYimXIIkOeP0v+Muykath3FJa+S O5SmM0PgBBIzEIk+38E5Rfa/zGBU8PRbDJKI20cvZjzdX4//Fn2gbhka36U7cJUjbnyu rY4o0VDRZNmOEicfOPvB7B2oo96aydPceFYySP/HbLntKIfQKosGIsqgkLQ8FoA7pQ8d F8Uw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=yH7mgMvRclpeghEGmXHVXbP7gTpnS4xJGtRwz3riN98=; b=pYueTTHyNN8TiY8kBhWQvLrFSXcXa54Fkrpb6q3qrGlIcwo5Db+QsWIgGF3Ig5FmTE LQYdCVq2FkN2/fduUm7kE2HLUe9VPHGqGdgUnRViBApvzCF3r0kxgiIk/ZrH0orK5gRa 2N9xi1azP/pd2n4lGU2EDM7Z+ZSB8ZAgvQ1r24HDEKBb+2vZ7TX1H1HZQDpej5f619W5 J4Mk/bEMblW1IbDJ3LiGbFYz/s+hcoVVKVyr+yKkTBa5/KKJxz+PFoEr2k+V9JtzPt2G 4RztGYiuzjE5Wlk9jeqKGLhpbluTjVPE7T4EdCdNZwBw/v74dIgISKYpzXMQ+nRUNrF0 9KMg== X-Gm-Message-State: AOAM533Vb2wqyZ6IXp4BayrpQT8pF0NZgh1/nhut4KMsJdkmF7sGfzDd r+C3mpwJrPeg3LhQtXlT6D6QiNrPf8tkpA== X-Google-Smtp-Source: ABdhPJwXbuE+GIE07B5w9BnprcgxlKSCGbVNMYVtcPjxsi28iNsbSg9z0KFTC3THR62I6GX15c0hLA== X-Received: by 2002:a05:6870:4691:b0:dd:f2ca:f5d8 with SMTP id a17-20020a056870469100b000ddf2caf5d8mr2740929oap.57.1647802313423; Sun, 20 Mar 2022 11:51:53 -0700 (PDT) Received: from [192.168.0.13] ([186.136.131.95]) by smtp.gmail.com with ESMTPSA id he5-20020a056870798500b000ddf4f625easm1568168oab.18.2022.03.20.11.51.51 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 20 Mar 2022 11:51:52 -0700 (PDT) Message-ID: <7f619d76-a701-092f-863d-664eac5cd5ba@gmail.com> Date: Sun, 20 Mar 2022 15:51:53 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220319030407.45503-1-jamrial@gmail.com> From: James Almer In-Reply-To: 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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. > > - Hendrik > _______________________________________________ > 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".