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 E3FAB49040 for ; Wed, 1 May 2024 20:53:37 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3CB1E68D733; Wed, 1 May 2024 23:53:35 +0300 (EEST) Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 723C368D6F3 for ; Wed, 1 May 2024 23:53:28 +0300 (EEST) Received: by mail-pf1-f176.google.com with SMTP id d2e1a72fcca58-6ed112c64beso6664567b3a.1 for ; Wed, 01 May 2024 13:53:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1714596806; x=1715201606; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=8DyU5YsvBVBlmn4J7rglfnGJU1fFKt78uUNF9IgD1EQ=; b=TycXls3ujbaGFCxHAMI9qkWtR7NfpXnj9fGLDuQU2zjohmjck1shuZo2Q9jdSkh+4K ivq/57+d5zMIcT1U/6vZ8KIuRSl95hIB8KGRoIKj0zHJlTAEqVHd8mOD8XvqPVE+iXiP hVSgUzETZ5RorTNcBUbxBssISVeRlVpGYe1vp5eWyrrUXC85bI9+r0JL4SQfCfPXDZTC SHMJioxSbHotIx27lGen2rbNo7S/mvVg/jDVZ0M2f1NvV9jgRSobUhm9rVTJXIH0UGMg lw2WdUvj5DJbOq4bLT6lgTal5JqKKscqbipiSiDzgCzAR7Jmrypvm7OQyipanF1bID3j q8Ww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714596806; x=1715201606; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=8DyU5YsvBVBlmn4J7rglfnGJU1fFKt78uUNF9IgD1EQ=; b=cWOxLRyRayPoe21GTE+/QKAoY4MDS6HIOSUV1/VdS5NEaJRm94zYlRsZK99/WT95go ocJfKEUPVzR7rw89LwzfNTlVpd63z06bnUjG+WgNhzDiAE1DIwsoN/uotdrBixjDc10X IfVG5v4kNxETbuhZinLa7wo0VYxfs1zItTcRy7UMfYPVSGulGQ6RuFl+GdEICj1ZdCzp wTY6ols3BuTePdhcUl5aqapwU2TTeHnEBIwHq7ieGl45lg+B9zoRW9MBkGx/YRcShQcy 1gmJKpGXrRJ+6zZZ/6hL1n/yCbpKHR3avIwCQPthy+rjxXXadnfb/TxZ7MrfCsV9cCuv 366Q== X-Gm-Message-State: AOJu0YxaevvUsuq/23m7Io8ygGrFHfX1MdV0AoFYkphMf4n2mFRseOzI g5Jj9TspHppdyjlf394pTz+z/lInPbZSxRKjJxKw1bDj9oNVtG+5hgjz+g== X-Google-Smtp-Source: AGHT+IF+OR25sLgAw5BRydT7rl8scr97p+BIz5bnh9Xh0mbmVii4bVk1i1p6RKYLkDbj1wmj1OgbyQ== X-Received: by 2002:a05:6a20:970c:b0:1ae:42ad:b571 with SMTP id hr12-20020a056a20970c00b001ae42adb571mr4169997pzc.24.1714596806273; Wed, 01 May 2024 13:53:26 -0700 (PDT) Received: from [192.168.0.10] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id fv7-20020a056a00618700b006f3f5d3595fsm7079731pfb.80.2024.05.01.13.53.24 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 May 2024 13:53:25 -0700 (PDT) Message-ID: <189ada78-d624-43fd-9cf3-1d51efe1c439@gmail.com> Date: Wed, 1 May 2024 17:53:24 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240501190156.36095-1-jamrial@gmail.com> Content-Language: en-US From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 1/3] avcodec/avcodec: prevent ch_layout from being uninitialized in ff_codec_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 5/1/2024 5:26 PM, Andreas Rheinhardt wrote: > James Almer: >> It's a user-set parameter shared with AVCodecParameters, so it should only >> be freed by avcodec_free_context(). >> >> Signed-off-by: James Almer >> --- >> libavcodec/avcodec.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c >> index 888dd76228..fc8a40e4db 100644 >> --- a/libavcodec/avcodec.c >> +++ b/libavcodec/avcodec.c >> @@ -414,6 +414,7 @@ void avsubtitle_free(AVSubtitle *sub) >> >> av_cold void ff_codec_close(AVCodecContext *avctx) >> { >> + AVChannelLayout ch_layout; >> int i; >> >> if (!avctx) >> @@ -468,7 +469,13 @@ av_cold void ff_codec_close(AVCodecContext *avctx) >> >> if (avctx->priv_data && avctx->codec && avctx->codec->priv_class) >> av_opt_free(avctx->priv_data); >> + >> + // Work around av_opt_free() unsetting ch_layout >> + 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 and the other patches will cause memleaks for users that use > allocated channel layouts and avcodec_close()+av_free() (this is > deprecated, not forbidden). That's awful, but guess it needs to be supported until avcodec_close() is gone, so I'm withdrawing this patch. > > Furthermore, where does the rule "user-set parameters shared with > AVCodecParameters should only be freed by avcodec_free_context()" come > from? It is news to me. It's not a rule, it's the ideal/expected behavior seeing the crash Michael found, where the only shared field cleared during avcodec_close() was ch_layout because it may contain allocated data and can be set through an AVOption. If you're copying params between codecpar and avctx, the latter should not have only one of the relevant fields nuked on an internal failure. _______________________________________________ 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".