diff mbox series

[FFmpeg-devel,30/33] avcodec/mpegpicture: Add function to completely free MPEG-Picture

Message ID AM7PR03MB6660BC2BFAA503885CD8F9DD8F209@AM7PR03MB6660.eurprd03.prod.outlook.com
State Accepted
Headers show
Series [FFmpeg-devel,01/21] avcodec/h263: Remove declaration for inexistent function | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished
andriy/make_aarch64_jetson success Make finished
andriy/make_fate_aarch64_jetson success Make fate finished

Commit Message

Andreas Rheinhardt Jan. 26, 2022, 9:34 p.m. UTC
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 <andreas.rheinhardt@outlook.com>
---
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(-)

Comments

James Almer Jan. 27, 2022, 2:21 p.m. UTC | #1
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 <andreas.rheinhardt@outlook.com>
> ---
> 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);
Andreas Rheinhardt Jan. 27, 2022, 2:35 p.m. UTC | #2
James Almer:
> 
> 
> 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 <andreas.rheinhardt@outlook.com>
>> ---
>> 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().
> 

I pondered using an ff_mpv prefix as lots of other mpegvideo functions
already do, but somehow didn't do it. I'll change it now.

- Andreas
diff mbox series

Patch

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);
 
     av_freep(&avctx->stats_out);
     av_freep(&s->ac_stats);