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 C1B7C42614 for ; Mon, 21 Mar 2022 07:38:05 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9E38668AB17; Mon, 21 Mar 2022 09:38:02 +0200 (EET) Received: from mail-yw1-f171.google.com (mail-yw1-f171.google.com [209.85.128.171]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 769A168AB17 for ; Mon, 21 Mar 2022 09:37:56 +0200 (EET) Received: by mail-yw1-f171.google.com with SMTP id 00721157ae682-2e5e176e1b6so68160377b3.13 for ; Mon, 21 Mar 2022 00:37:56 -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=DkXRFW/YdMTnJ/oAk+Iv0EdZIcNlAE5FB6Mvvii6VLs=; b=ltPdiQTDoKiyVhKWmGja2BBkLuWgtecsP7QGuptO2DsqEG8j3E/ac99Qqcr/FdxFc8 o85+36zHbVtSd7qc27NlUul1gQnON2tJhxTGOGQlvs8//0/rrat0KF+1x12HsiZ66OGF nT9OzfwCPJxiKxsjhxzIv9dvBSQGLekTjLrkrGY/AMOMwNqf65851YlZ/QSKvsHZq29c mumKZls97cWuPPafqyrOzSodvNEr0g4UdH/FAOXg53ld3UzUN6JK0AGD07zSJcpAaDOF R+hpsljB83AzzhqE2ogIX3a9Z0iTWuB3j/+WjyUljBjWamQPHyc/LSaGiVhfrqoqgc3y v00A== 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=DkXRFW/YdMTnJ/oAk+Iv0EdZIcNlAE5FB6Mvvii6VLs=; b=5ABsmXqQXtWLgogixDwOcCft6POJIR/Rq2aN9hGYO2ifGXt55M7WhuBoatS8bo4igJ Ur6WRsnMI4skJcHc3UZLbTRpOukjPiht8z8MHnNnl2iPbM2jmJei2fkn0sa51cof5yzy /piL7tEFo4Rl1xesFapIXO33skMJFDjyn6i5OH6rZcXT4zFiUVEZwTZTnksE+5RU1Yqu +aUti/iph1JFld2Sak6G83GBUCesYoLROADFFm0mdyGy3OCTg8R9YLTgFSzA41uEEQzB zlV2zKjDDDADxWfl7ipK0MkQOpJFBU3U3DhHOW2S6Z+VY97uP1qLpOjOFHzg7tiHSBYt azCA== X-Gm-Message-State: AOAM5303Tb+ejr6Mv2xmcSr0pJKVpqOqbY5zY6Fvjl7P2Np2oDzi0RKT +1oBPVUKhO2DzflQJvNzd2r0ib+WFhc7E8VMkuZOu3vT X-Google-Smtp-Source: ABdhPJyx4CoGEx93Lv+dfMY6vRRPu9DgHzmLGEYvyoB7XCk2BBQJtr8wCbS0mnkN9S2oHH2g1WsgrxXwxJljUxiQHZ4= X-Received: by 2002:a0d:d912:0:b0:2e6:4099:b34f with SMTP id b18-20020a0dd912000000b002e64099b34fmr2523138ywe.46.1647848274694; Mon, 21 Mar 2022 00:37:54 -0700 (PDT) MIME-Version: 1.0 References: <20220319030407.45503-1-jamrial@gmail.com> In-Reply-To: From: Hendrik Leppkes Date: Mon, 21 Mar 2022 08:37:43 +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 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. - 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".