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 8E06A4262B for ; Mon, 21 Mar 2022 12:10:37 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E3FF168B15A; Mon, 21 Mar 2022 14:10:33 +0200 (EET) Received: from mail-oo1-f41.google.com (mail-oo1-f41.google.com [209.85.161.41]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6F08E68008C for ; Mon, 21 Mar 2022 14:10:27 +0200 (EET) Received: by mail-oo1-f41.google.com with SMTP id v19-20020a056820101300b0032488bb70f5so4725513oor.5 for ; Mon, 21 Mar 2022 05:10:27 -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=1LDbnLSDtpFtGGpwSMHwATb5SlZhk0vFv/0R7rM62nA=; b=hVDB9TG9ObEk48Yhq+vFil7NRSz4iSyyCNuVXbXaCOZ49qsrY1qFDmk+mnqV6+bbYI WE0O4IkmSk2XbbOBquzxmDmBW5nhXRxGvxCr7WS8flwqA8bm/4v7KP20h2MRHJRtSxjv HDhvwrZmTXNBF+jotgipcPxVtmchdZq4/NmABVen4RHCPgq1zR6JLzybE9kW6BBnNLYN 5+cb+VBiFZxr+nLFD5gWw3z7GI6EfYDJ/Tg/YWqC+bUvHGH+O2hDNp+iLblm1Oo1GCiI AEDTG4LbcQL5Q2/udXgrBBxdk+YtaoyQCuv9d/oo8TSmY6hnfi1G7894RkdkGrgC285U YY1g== 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=1LDbnLSDtpFtGGpwSMHwATb5SlZhk0vFv/0R7rM62nA=; b=zH0BsUf3CdY4/GCN/2mwigyUpuLSAtbWJKftPnnhbeW7yaUAmHt9qLFIbLaF19sc0b 7Tp/L5IGt2f57JUXBgqcbfHZBfebddziIWHaf+UHFXSz00ctaepeQGGY/Ep7bgcu9YgA 5k10qF3Qd1dXwXgvfK4Wer95f3qBrEx/ojOePQjqn0CIEIqqVirJhR1s3NWkfxcMnWBW Q0lpIx8sN/P8lQOi2VC87QDmoeCXfFs9Tnta3cq6hFI3rzqlAYx/xoS/ujYX0DhpYRNr eMuu4TYDj4kThf9G1uA6Pw86yOkUwYSZI96OZULeq+JnnSQvZLviErsccyxqHjnnjohs gjVQ== X-Gm-Message-State: AOAM531/dRwFqpEpPfdwoH6Nx6u5653iVqHGPK9uOpAONGsbHJ0nnDn8 egHwwjNvWS8MQ1QCi3Suv4o53GAJxYFsog== X-Google-Smtp-Source: ABdhPJyi18cvH2IGiEMhWk0YyWne6aZs9obzJgWovkU4j99Q+OulWivlbmy4uiiuVogrHcAlUnGMDg== X-Received: by 2002:a05:6870:1496:b0:dd:fe5c:4bbc with SMTP id k22-20020a056870149600b000ddfe5c4bbcmr2998261oab.166.1647864625149; Mon, 21 Mar 2022 05:10:25 -0700 (PDT) Received: from [192.168.0.13] ([186.136.131.95]) by smtp.gmail.com with ESMTPSA id 21-20020a056870121500b000ddb064e097sm5523230oan.31.2022.03.21.05.10.23 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 21 Mar 2022 05:10:24 -0700 (PDT) Message-ID: <4ab643ba-e45d-f7dc-9a38-14c8a6e9ffb3@gmail.com> Date: Mon, 21 Mar 2022 09:10:22 -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> <164784909651.6085.6378539386386313722@lain.red.khirnov.net> From: James Almer In-Reply-To: <164784909651.6085.6378539386386313722@lain.red.khirnov.net> 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/21/2022 4:51 AM, Anton Khirnov wrote: > 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. What's your opinion on the "don't free" AVOption flag Hendrik suggested? _______________________________________________ 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".