diff mbox series

[FFmpeg-devel,v2,40/71] avcodec/mpegvideo_enc: Move copying properties to alloc_picture()

Message ID GV1P250MB0737522E6AF28EADB35D42FD8FE02@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,v2,01/71] avcodec/ratecontrol: Fix double free on error | expand

Commit Message

Andreas Rheinhardt May 11, 2024, 8:51 p.m. UTC 
This way said function sets everything (except for the actual
contents of the frame's data). Also rename it to prepare_picture()
given its new role.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mpegvideo_enc.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Comments

Michael Niedermayer May 12, 2024, 7:55 p.m. UTC | #1 
On Sat, May 11, 2024 at 10:51:04PM +0200, Andreas Rheinhardt wrote:
> This way said function sets everything (except for the actual
> contents of the frame's data). Also rename it to prepare_picture()
> given its new role.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/mpegvideo_enc.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

changes output for:

make -j32 && ./ffmpeg -y -i mm-short.mpg -qmin 1 -qmax 8 -vtag mx3p -vcodec mpeg2video -r 25 -pix_fmt yuv422p -minrate 30000k -maxrate 30000k -b 30000k -g 1 -flags +ildct+low_delay -dc 10  -ps 1 -qmin 1 -qmax 8 -top 1 -bufsize 1200000 -lmin QP2LAMBDA -t 2 -an -bitexact /tmp/file-qp1-hq-old.mpg

md5sum /tmp/file-qp1-hq-old.mpg /tmp/file-qp1-hq.mpg
dee0a5b29d2fbb63060ed43668db0df0  /tmp/file-qp1-hq-old.mpg
86c3ed0ec7a948e32888a444475439ae  /tmp/file-qp1-hq.mpg

did not investigate why

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c6f4cd9b0e..393b21823f 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1091,7 +1091,11 @@  static int get_intra_count(MpegEncContext *s, const uint8_t *src,
     return acc;
 }
 
-static int alloc_picture(MpegEncContext *s, AVFrame *f)
+/**
+ * Allocates new buffers for an AVFrame and copies the properties
+ * from another AVFrame.
+ */
+static int prepare_picture(MpegEncContext *s, AVFrame *f, const AVFrame *props_frame)
 {
     AVCodecContext *avctx = s->avctx;
     int ret;
@@ -1107,6 +1111,10 @@  static int alloc_picture(MpegEncContext *s, AVFrame *f)
     if (ret < 0)
         return ret;
 
+    ret = av_frame_copy_props(f, props_frame);
+    if (ret < 0)
+        return ret;
+
     for (int i = 0; f->data[i]; i++) {
         int offset = (EDGE_WIDTH >> (i ? s->chroma_y_shift : 0)) *
                      f->linesize[i] +
@@ -1186,14 +1194,9 @@  static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
                 return ret;
             pic->shared = 1;
         } else {
-            ret = alloc_picture(s, pic->f);
+            ret = prepare_picture(s, pic->f, pic_arg);
             if (ret < 0)
                 goto fail;
-            ret = av_frame_copy_props(pic->f, pic_arg);
-            if (ret < 0) {
-                ff_mpeg_unref_picture(pic);
-                return ret;
-            }
 
             for (int i = 0; i < 3; i++) {
                 ptrdiff_t src_stride = pic_arg->linesize[i];
@@ -1607,11 +1610,8 @@  no_output_pic:
             // input is a shared pix, so we can't modify it -> allocate a new
             // one & ensure that the shared one is reuseable
             av_frame_move_ref(s->new_pic, s->reordered_input_picture[0]->f);
-            ret = alloc_picture(s, s->reordered_input_picture[0]->f);
-            if (ret < 0)
-                goto fail;
 
-            ret = av_frame_copy_props(s->reordered_input_picture[0]->f, s->new_pic);
+            ret = prepare_picture(s, s->reordered_input_picture[0]->f, s->new_pic);
             if (ret < 0)
                 goto fail;
         } else {