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 93EFF4067D for ; Thu, 27 Jan 2022 14:22:15 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2277D68B18A; Thu, 27 Jan 2022 16:22:13 +0200 (EET) Received: from mail-oi1-f172.google.com (mail-oi1-f172.google.com [209.85.167.172]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 7534168B10B for ; Thu, 27 Jan 2022 16:22:06 +0200 (EET) Received: by mail-oi1-f172.google.com with SMTP id bb37so6169319oib.1 for ; Thu, 27 Jan 2022 06:22:06 -0800 (PST) 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=CymOS0Gas/8VZqj8rOupGIL4BDPB38YBXBwc5fwO3P8=; b=LGVHeaGqNenUD/mFDyWuNrpOiZWnsHPaI1cPhzH3Hb1ChQ2K7cBZad2ae+EiRYXru3 I4WoU9TCYWutP1AoXeacIkbJfiYk1UCwar+mct7YFvIiQtBdrXSHXfOxH2CWQJhvdvM+ WJ+VUgEBzCpHlgzJvrQQGhEgWDEqPSnbY2InbcxAyF8VrQGEojDVrAzZp8DRvQXDQvKj DKcvl0lnB29NEzD8ha+ITHoi7mVXOz7R+Gz97qvCID6RpRPBTdfNRrn0qTBbqLZ85dwQ 5/Y+piHexthyhV/qaGJ5+IJfO8DIgmPKOyTDew/FqxZGsRAIF+IEWMYDkAoaQgySxCMS Pt+A== 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=CymOS0Gas/8VZqj8rOupGIL4BDPB38YBXBwc5fwO3P8=; b=fIJlJWumGZF/EyX1pjGIMVPqyYx1f5JmuOyLU6GbSpUnrFRCdf9omYuIMilggEFeSb SzZ8l2j7J2Xa7bDCszlrEzBrH1nDhwtr1RhfTDk5Yu3Ew9VyTDobYNAqmm5JM51oCi17 djvaCm9hLv2vDbg0FL6dUQ6J57olBZiGJHWjGt6TmqGzMro8yUL+GCbGqeHNn1paFB/w 5+4vY3cEvUqM43+mYTdXXx8jg6o4//jfv0v87Z9gjuZnERO5UQg+/Ggm9iRw9v7Y5Eam 5FlQXOIh1deg6Ho6/5DoCUmMtpjAsW6QY1BvwaWJVWl26snIng+WduHcySC1MvsGCCRI oAFw== X-Gm-Message-State: AOAM5317Jy/KynfRSpmz9R3zJtYqZ/szidvYX2RoqcVl+XbmYi2bPO3T 8NM9h20tLTrMwmwVkJ2HR4LV04KIan8= X-Google-Smtp-Source: ABdhPJwRcYH7B4EPKAJ2L4Q8DnUWpyOViEMRAwKmM4LiodpkhIqNsS61GJDcHSMzw1XV0tWMthS2NA== X-Received: by 2002:a54:408e:: with SMTP id i14mr2500835oii.200.1643293323930; Thu, 27 Jan 2022 06:22:03 -0800 (PST) Received: from [192.168.0.13] ([186.136.131.95]) by smtp.gmail.com with ESMTPSA id 129sm6586213ooj.23.2022.01.27.06.22.02 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 27 Jan 2022 06:22:03 -0800 (PST) Message-ID: <3f3709d2-fbf7-9077-8340-224574930e79@gmail.com> Date: Thu, 27 Jan 2022 11:21:59 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: From: James Almer In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH 30/33] avcodec/mpegpicture: Add function to completely free MPEG-Picture 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 1/26/2022 6:34 PM, Andreas Rheinhardt wrote: > Also use said function in mpegvideo.c and mpegvideo_enc.c; > and make ff_free_picture_tables() static as it isn't needed anymore > outside of mpegpicture.c. > > Signed-off-by: Andreas Rheinhardt > --- > The new_picture is actually only used by encoders; > if it were not for svq1enc (which relies on ff_mpv_common_init() > and ff_mpv_common_end() to allocate and free it), I'd have moved > everything related to it to mpegvideo_enc.c. I probably do it later > anyway. > (And yes, I am aware of the fact that freeing this frame in > ff_mpv_encode_end() is redundant.) > > libavcodec/mpegpicture.c | 47 +++++++++++++++++++++----------------- > libavcodec/mpegpicture.h | 2 +- > libavcodec/mpegvideo.c | 25 +++++--------------- > libavcodec/mpegvideo_enc.c | 3 +-- > 4 files changed, 34 insertions(+), 43 deletions(-) > > diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c > index f78a3c23e3..349ab81055 100644 > --- a/libavcodec/mpegpicture.c > +++ b/libavcodec/mpegpicture.c > @@ -30,6 +30,24 @@ > #include "mpegpicture.h" > #include "mpegutils.h" > > +static void av_noinline free_picture_tables(Picture *pic) > +{ > + pic->alloc_mb_width = > + pic->alloc_mb_height = 0; > + > + av_buffer_unref(&pic->mb_var_buf); > + av_buffer_unref(&pic->mc_mb_var_buf); > + av_buffer_unref(&pic->mb_mean_buf); > + av_buffer_unref(&pic->mbskip_table_buf); > + av_buffer_unref(&pic->qscale_table_buf); > + av_buffer_unref(&pic->mb_type_buf); > + > + for (int i = 0; i < 2; i++) { > + av_buffer_unref(&pic->motion_val_buf[i]); > + av_buffer_unref(&pic->ref_index_buf[i]); > + } > +} > + > static int make_tables_writable(Picture *pic) > { > int ret, i; > @@ -240,7 +258,7 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me, > if (pic->qscale_table_buf) > if ( pic->alloc_mb_width != mb_width > || pic->alloc_mb_height != mb_height) > - ff_free_picture_tables(pic); > + free_picture_tables(pic); > > if (shared) { > av_assert0(pic->f->data[0]); > @@ -285,7 +303,7 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me, > fail: > av_log(avctx, AV_LOG_ERROR, "Error allocating a picture.\n"); > ff_mpeg_unref_picture(avctx, pic); > - ff_free_picture_tables(pic); > + free_picture_tables(pic); > return AVERROR(ENOMEM); > } > > @@ -310,7 +328,7 @@ void ff_mpeg_unref_picture(AVCodecContext *avctx, Picture *pic) > av_buffer_unref(&pic->hwaccel_priv_buf); > > if (pic->needs_realloc) > - ff_free_picture_tables(pic); > + free_picture_tables(pic); > > memset((uint8_t*)pic + off, 0, sizeof(*pic) - off); > } > @@ -331,7 +349,7 @@ int ff_update_picture_tables(Picture *dst, Picture *src) > } > > if (ret < 0) { > - ff_free_picture_tables(dst); > + free_picture_tables(dst); > return ret; > } > > @@ -450,22 +468,9 @@ int ff_find_unused_picture(AVCodecContext *avctx, Picture *picture, int shared) > return ret; > } > > -void ff_free_picture_tables(Picture *pic) > +void av_cold ff_free_picture(AVCodecContext *avctx, Picture *pic) > { > - int i; > - > - pic->alloc_mb_width = > - pic->alloc_mb_height = 0; > - > - av_buffer_unref(&pic->mb_var_buf); > - av_buffer_unref(&pic->mc_mb_var_buf); > - av_buffer_unref(&pic->mb_mean_buf); > - av_buffer_unref(&pic->mbskip_table_buf); > - av_buffer_unref(&pic->qscale_table_buf); > - av_buffer_unref(&pic->mb_type_buf); > - > - for (i = 0; i < 2; i++) { > - av_buffer_unref(&pic->motion_val_buf[i]); > - av_buffer_unref(&pic->ref_index_buf[i]); > - } > + free_picture_tables(pic); > + ff_mpeg_unref_picture(avctx, pic); > + av_frame_free(&pic->f); > } > diff --git a/libavcodec/mpegpicture.h b/libavcodec/mpegpicture.h > index a354c2a83c..cee16c07d3 100644 > --- a/libavcodec/mpegpicture.h > +++ b/libavcodec/mpegpicture.h > @@ -108,7 +108,7 @@ int ff_mpeg_framesize_alloc(AVCodecContext *avctx, MotionEstContext *me, > int ff_mpeg_ref_picture(AVCodecContext *avctx, Picture *dst, Picture *src); > void ff_mpeg_unref_picture(AVCodecContext *avctx, Picture *picture); > > -void ff_free_picture_tables(Picture *pic); > +void ff_free_picture(AVCodecContext *avctx, Picture *pic); > int ff_update_picture_tables(Picture *dst, Picture *src); > > int ff_find_unused_picture(AVCodecContext *avctx, Picture *picture, int shared); > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c > index 3b889e0791..7c63c738f3 100644 > --- a/libavcodec/mpegvideo.c > +++ b/libavcodec/mpegvideo.c > @@ -874,8 +874,6 @@ void ff_mpv_free_context_frame(MpegEncContext *s) > /* init common structure for both encoder and decoder */ > void ff_mpv_common_end(MpegEncContext *s) > { > - int i; > - > if (!s) > return; > > @@ -895,25 +893,14 @@ void ff_mpv_common_end(MpegEncContext *s) > return; > > if (s->picture) { > - for (i = 0; i < MAX_PICTURE_COUNT; i++) { > - ff_free_picture_tables(&s->picture[i]); > - ff_mpeg_unref_picture(s->avctx, &s->picture[i]); > - av_frame_free(&s->picture[i].f); > - } > + for (int i = 0; i < MAX_PICTURE_COUNT; i++) > + ff_free_picture(s->avctx, &s->picture[i]); > } > av_freep(&s->picture); > - ff_free_picture_tables(&s->last_picture); > - ff_mpeg_unref_picture(s->avctx, &s->last_picture); > - av_frame_free(&s->last_picture.f); > - ff_free_picture_tables(&s->current_picture); > - ff_mpeg_unref_picture(s->avctx, &s->current_picture); > - av_frame_free(&s->current_picture.f); > - ff_free_picture_tables(&s->next_picture); > - ff_mpeg_unref_picture(s->avctx, &s->next_picture); > - av_frame_free(&s->next_picture.f); > - ff_free_picture_tables(&s->new_picture); > - ff_mpeg_unref_picture(s->avctx, &s->new_picture); > - av_frame_free(&s->new_picture.f); > + ff_free_picture(s->avctx, &s->last_picture); > + ff_free_picture(s->avctx, &s->current_picture); > + ff_free_picture(s->avctx, &s->next_picture); > + ff_free_picture(s->avctx, &s->new_picture); > > s->context_initialized = 0; > s->context_reinit = 0; > diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c > index 9a5634c505..baa45d20ab 100644 > --- a/libavcodec/mpegvideo_enc.c > +++ b/libavcodec/mpegvideo_enc.c > @@ -941,8 +941,7 @@ av_cold int ff_mpv_encode_end(AVCodecContext *avctx) > for (i = 0; i < FF_ARRAY_ELEMS(s->tmp_frames); i++) > av_frame_free(&s->tmp_frames[i]); > > - ff_free_picture_tables(&s->new_picture); > - ff_mpeg_unref_picture(avctx, &s->new_picture); > + ff_free_picture(avctx, &s->new_picture); These names are too generic for what's apparently specific to mpeg. ff_free_picture (And the struct being called Picture) could apply to anything. Maybe the functions should all use the ff_mpeg_ namespace, and the struct be renamed to MPEGPicture, following decoders like h264 and hevc. I'm not telling you to do it if you don't want to, just throwing the idea out there. But IMO at least call the new function ff_mpeg_free_picture(). > > av_freep(&avctx->stats_out); > av_freep(&s->ac_stats); _______________________________________________ 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".