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 C213149039 for ; Wed, 1 May 2024 20:51:37 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F387D68D734; Wed, 1 May 2024 23:51:34 +0300 (EEST) Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 928AD68D6B9 for ; Wed, 1 May 2024 23:51:28 +0300 (EEST) Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1e3ff14f249so9241455ad.1 for ; Wed, 01 May 2024 13:51:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1714596686; x=1715201486; 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=3t673iTiCyrCjqu83jLGLlmvV7DGJUt48cQIKQDYnxA=; b=QkHxmgr1lTIuO7NqhqM9yMMdKBvk5BWiSrNezz361JmG1bsWb0tMcT7j1VuoOSJjvT XL3wX8GqcZv7YLyMzZKvWbQjilt/J7+7oypg2OVFCw1Y8t53NZ8U/Ma6Tiei5KpTYuzC tIoTxZn+hQAtNQ9RKd/g0IKtLEMImDAZkhFFASlOOMifyh6vUcl/O6oG/YVoj5sIbl0b corfzbCofwMPqVnhEcnuJv6D6Z2vg24ykFNAioZFxjLWl9M8APPqA9+x6ROwxwrsJeQc k5PbJhCMl5iWGLsCN+7DrdKwRemp7eBJ2+GcrkJzRFSexZaxlUGf99oAgOK8wRbPLgtR dt9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714596686; x=1715201486; 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=3t673iTiCyrCjqu83jLGLlmvV7DGJUt48cQIKQDYnxA=; b=TRKKb3dIapxPNiq43mR9y1o75s0MZtzCrHcFGG+zTVpKiD5jEN5l/LQ6d/REDt+XC6 LiYvKmArnEn6Y78PL1PJ2/SEx7YKaQgtiJKB0GMBb+o8g5+DTtoRQ7f21jrLn07biqtn Y9QryAFYsaGNjlfd0enGSLMPVQZ7Qzdml9huU07P0HFVUN9oXGGEZQZw+I5/gcn5WTaf h/Yc/tNxg/LPfLfaNGD99Ct2ttBgNTo1WYWep+SsJ4XWQvn8SYgXY+MlqQAT79QXmEKf kgK4dmdE5nv8hFVGpRoXWS5Ne4ptny/vvlC0taIk61Ck0r6EPjEQyiHW1xb9q89RqLOa EvuA== X-Gm-Message-State: AOJu0YygDthkQNulfI8d2G5hHk6CTR2Rr1hIx9Asa2snmWRL+7tcry1Y 121/yYS6GxWi2hq4WhNa0sZNTaLyk5VdvzXftO4GlenH0EMOp98aWcrtGA== X-Google-Smtp-Source: AGHT+IFGUl9ToANpaqWdejw3ELK0+SEtXhJcTkAgFgYErxe5NrhJqKv0PI8bB101yikVnTpjgHMRzw== X-Received: by 2002:a17:903:1cc:b0:1e2:45f3:2d57 with SMTP id e12-20020a17090301cc00b001e245f32d57mr1245325plh.6.1714596685673; Wed, 01 May 2024 13:51:25 -0700 (PDT) Received: from [192.168.0.10] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id w8-20020a170902a70800b001dd82855d47sm24530883plq.265.2024.05.01.13.51.23 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 01 May 2024 13:51:24 -0700 (PDT) Message-ID: <01946ed3-ef94-4bb3-91e9-cbcc81c25fcc@gmail.com> Date: Wed, 1 May 2024 17:51:22 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240501190156.36095-1-jamrial@gmail.com> <20240501190156.36095-2-jamrial@gmail.com> Content-Language: en-US From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 2/3] avcodec/avcodec: don't free coded_side_data in ff_codec_close() when decoding 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:40 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 | 11 ++++++----- >> libavcodec/options.c | 4 ++++ >> 2 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c >> index fc8a40e4db..e560efff6a 100644 >> --- a/libavcodec/avcodec.c >> +++ b/libavcodec/avcodec.c >> @@ -458,11 +458,12 @@ av_cold void ff_codec_close(AVCodecContext *avctx) >> >> av_freep(&avctx->internal); >> } >> - >> - for (i = 0; i < avctx->nb_coded_side_data; i++) >> - av_freep(&avctx->coded_side_data[i].data); >> - av_freep(&avctx->coded_side_data); >> - avctx->nb_coded_side_data = 0; >> + if (av_codec_is_encoder(avctx->codec)) { >> + for (i = 0; i < avctx->nb_coded_side_data; i++) >> + av_freep(&avctx->coded_side_data[i].data); >> + av_freep(&avctx->coded_side_data); >> + avctx->nb_coded_side_data = 0; >> + } >> >> av_buffer_unref(&avctx->hw_frames_ctx); >> av_buffer_unref(&avctx->hw_device_ctx); >> diff --git a/libavcodec/options.c b/libavcodec/options.c >> index 0c3b40a186..7c32a71275 100644 >> --- a/libavcodec/options.c >> +++ b/libavcodec/options.c >> @@ -177,6 +177,10 @@ void avcodec_free_context(AVCodecContext **pavctx) >> av_freep(&avctx->inter_matrix); >> av_freep(&avctx->rc_override); >> av_channel_layout_uninit(&avctx->ch_layout); >> + for (int i = 0; i < avctx->nb_coded_side_data; i++) >> + av_freep(&avctx->coded_side_data[i].data); >> + av_freep(&avctx->coded_side_data); >> + avctx->nb_coded_side_data = 0; >> av_frame_side_data_free( >> &avctx->decoded_side_data, &avctx->nb_decoded_side_data); >> > > 1. ff_codec_close() already has an "if is_encoder" branch, it does not > need another one. > 2. The code in ff_codec_close() will be redundant as soon as > FF_API_AVCODEC_CLOSE is no more, so it should be inside > FF_API_AVCODEC_CLOSE. > 3. The documentation of this field does not mention ownership, but given > that this field existed for a long time lavc's previous behaviour > established the implicit contract that lavc will free this in > avcodec_close(). You are breaking this implicit contract for no good Yes, I wrote this set forgetting avcodec_close() was still a thing. > reason apart from this new principle that user-set stuff shared with > AVCodecParameters should not be freed in avcodec_close(). Which is crazy > given that the relevant AVCodecParameters field has only been added > recently. _______________________________________________ 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".