[FFmpeg-devel] [PATCH 01/13] avcodec/mpegvideo_enc: Fix abort on allocation errors

[FFmpeg-devel] [PATCH 01/13] avcodec/mpegvideo_enc: Fix abort on allocation errors

[Andreas Rheinhardt]

mpegvideo_enc uses a fixed-size array of Pictures; a slot is
considered taken if the Picture's AVFrame is set.
When an error happens after a slot has been taken, this Picture
has typically not been reset and is therefore not usable for
future requests. The code aborts when one runs out of slots
and this can happen in case of allocation failures.
Fix this by always unreferencing a Picture in case of errors.

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 5bf4b06a11..6cd9d89e1b 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1223,8 +1223,10 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             }
         }
         ret = av_frame_copy_props(pic->f, pic_arg);
-        if (ret < 0)
+        if (ret < 0) {
+            ff_mpeg_unref_picture(s->avctx, pic);
             return ret;
+        }
 
         pic->display_picture_number = display_picture_number;
         pic->f->pts = pts; // we set this here to avoid modifying pic_arg
@@ -1535,8 +1537,10 @@ static int select_input_picture(MpegEncContext *s)
                 }
             } else if (s->b_frame_strategy == 2) {
                 b_frames = estimate_best_b_count(s);
-                if (b_frames < 0)
+                if (b_frames < 0) {
+                    ff_mpeg_unref_picture(s->avctx, s->input_picture[0]);
                     return b_frames;
+                }
             }
 
             emms_c();
@@ -1591,7 +1595,7 @@ no_output_pic:
 
         if ((ret = av_frame_ref(s->new_picture,
                                 s->reordered_input_picture[0]->f)))
-            return ret;
+            goto fail;
 
         if (s->reordered_input_picture[0]->shared || s->avctx->rc_buffer_size) {
             // input is a shared pix, so we can't modify it -> allocate a new
@@ -1604,13 +1608,15 @@ no_output_pic:
             pic = &s->picture[i];
 
             pic->reference = s->reordered_input_picture[0]->reference;
-            if (alloc_picture(s, pic, 0) < 0) {
-                return -1;
-            }
+            ret = alloc_picture(s, pic, 0);
+            if (ret < 0)
+                goto fail;
 
             ret = av_frame_copy_props(pic->f, s->reordered_input_picture[0]->f);
-            if (ret < 0)
-                return ret;
+            if (ret < 0) {
+                ff_mpeg_unref_picture(s->avctx, pic);
+                goto fail;
+            }
             pic->coded_picture_number = s->reordered_input_picture[0]->coded_picture_number;
             pic->display_picture_number = s->reordered_input_picture[0]->display_picture_number;
 
@@ -1631,6 +1637,9 @@ no_output_pic:
 
     }
     return 0;
+fail:
+    ff_mpeg_unref_picture(s->avctx, s->reordered_input_picture[0]);
+    return ret;
 }
 
 static void frame_end(MpegEncContext *s)
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 02/13] avcodec/mpegvideo_enc: Remove always-false checks

[Andreas Rheinhardt]

In case "!direct" we are not reusing the input buffers
(due to e.g. insufficient alignment), but allocating
new ones. These of course do not alias with the ones
provided by the user, so these checks are always-false.

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 6cd9d89e1b..3be6c1f928 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1171,11 +1171,6 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             return ret;
 
         if (!direct) {
-            if (pic->f->data[0] + INPLACE_OFFSET == pic_arg->data[0] &&
-                pic->f->data[1] + INPLACE_OFFSET == pic_arg->data[1] &&
-                pic->f->data[2] + INPLACE_OFFSET == pic_arg->data[2]) {
-                // empty
-            } else {
                 int h_chroma_shift, v_chroma_shift;
                 av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt,
                                                  &h_chroma_shift,
@@ -1220,7 +1215,6 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
                     }
                 }
                 emms_c();
-            }
         }
         ret = av_frame_copy_props(pic->f, pic_arg);
         if (ret < 0) {
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 03/13] avcodec/mpegvideo_enc: Reindentation

[Andreas Rheinhardt]

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 3be6c1f928..09b8348ae7 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1171,50 +1171,50 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             return ret;
 
         if (!direct) {
-                int h_chroma_shift, v_chroma_shift;
-                av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt,
-                                                 &h_chroma_shift,
-                                                 &v_chroma_shift);
-
-                for (i = 0; i < 3; i++) {
-                    int src_stride = pic_arg->linesize[i];
-                    int dst_stride = i ? s->uvlinesize : s->linesize;
-                    int h_shift = i ? h_chroma_shift : 0;
-                    int v_shift = i ? v_chroma_shift : 0;
-                    int w = s->width  >> h_shift;
-                    int h = s->height >> v_shift;
-                    const uint8_t *src = pic_arg->data[i];
-                    uint8_t *dst = pic->f->data[i];
-                    int vpad = 16;
-
-                    if (   s->codec_id == AV_CODEC_ID_MPEG2VIDEO
-                        && !s->progressive_sequence
-                        && FFALIGN(s->height, 32) - s->height > 16)
-                        vpad = 32;
-
-                    if (!s->avctx->rc_buffer_size)
-                        dst += INPLACE_OFFSET;
-
-                    if (src_stride == dst_stride)
-                        memcpy(dst, src, src_stride * h);
-                    else {
-                        int h2 = h;
-                        uint8_t *dst2 = dst;
-                        while (h2--) {
-                            memcpy(dst2, src, w);
-                            dst2 += dst_stride;
-                            src += src_stride;
-                        }
-                    }
-                    if ((s->width & 15) || (s->height & (vpad-1))) {
-                        s->mpvencdsp.draw_edges(dst, dst_stride,
-                                                w, h,
-                                                16 >> h_shift,
-                                                vpad >> v_shift,
-                                                EDGE_BOTTOM);
+            int h_chroma_shift, v_chroma_shift;
+            av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt,
+                                             &h_chroma_shift,
+                                             &v_chroma_shift);
+
+            for (int i = 0; i < 3; i++) {
+                int src_stride = pic_arg->linesize[i];
+                int dst_stride = i ? s->uvlinesize : s->linesize;
+                int h_shift = i ? h_chroma_shift : 0;
+                int v_shift = i ? v_chroma_shift : 0;
+                int w = s->width  >> h_shift;
+                int h = s->height >> v_shift;
+                const uint8_t *src = pic_arg->data[i];
+                uint8_t *dst = pic->f->data[i];
+                int vpad = 16;
+
+                if (   s->codec_id == AV_CODEC_ID_MPEG2VIDEO
+                    && !s->progressive_sequence
+                    && FFALIGN(s->height, 32) - s->height > 16)
+                    vpad = 32;
+
+                if (!s->avctx->rc_buffer_size)
+                    dst += INPLACE_OFFSET;
+
+                if (src_stride == dst_stride)
+                    memcpy(dst, src, src_stride * h);
+                else {
+                    int h2 = h;
+                    uint8_t *dst2 = dst;
+                    while (h2--) {
+                        memcpy(dst2, src, w);
+                        dst2 += dst_stride;
+                        src += src_stride;
                     }
                 }
-                emms_c();
+                if ((s->width & 15) || (s->height & (vpad-1))) {
+                    s->mpvencdsp.draw_edges(dst, dst_stride,
+                                            w, h,
+                                            16 >> h_shift,
+                                            vpad >> v_shift,
+                                            EDGE_BOTTOM);
+                }
+            }
+            emms_c();
         }
         ret = av_frame_copy_props(pic->f, pic_arg);
         if (ret < 0) {
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 04/13] avcodec/mpegvideo_enc: Don't pretend input to be non-refcounted

[Andreas Rheinhardt]

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 09b8348ae7..3b6386e144 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1140,8 +1140,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             }
         }
 
-        if (!pic_arg->buf[0] ||
-            pic_arg->linesize[0] != s->linesize ||
+        if (pic_arg->linesize[0] != s->linesize ||
             pic_arg->linesize[1] != s->uvlinesize ||
             pic_arg->linesize[2] != s->uvlinesize)
             direct = 0;
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 05/13] avcodec/mpegvideo_enc: Don't reget known values

[Andreas Rheinhardt]

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 3b6386e144..1e0aed8db9 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -40,7 +40,6 @@
 #include "libavutil/intmath.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/mem_internal.h"
-#include "libavutil/pixdesc.h"
 #include "libavutil/opt.h"
 #include "libavutil/thread.h"
 #include "avcodec.h"
@@ -1170,16 +1169,11 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             return ret;
 
         if (!direct) {
-            int h_chroma_shift, v_chroma_shift;
-            av_pix_fmt_get_chroma_sub_sample(s->avctx->pix_fmt,
-                                             &h_chroma_shift,
-                                             &v_chroma_shift);
-
             for (int i = 0; i < 3; i++) {
                 int src_stride = pic_arg->linesize[i];
                 int dst_stride = i ? s->uvlinesize : s->linesize;
-                int h_shift = i ? h_chroma_shift : 0;
-                int v_shift = i ? v_chroma_shift : 0;
+                int h_shift = i ? s->chroma_x_shift : 0;
+                int v_shift = i ? s->chroma_y_shift : 0;
                 int w = s->width  >> h_shift;
                 int h = s->height >> v_shift;
                 const uint8_t *src = pic_arg->data[i];
@@ -1640,9 +1634,8 @@ static void frame_end(MpegEncContext *s)
     if (s->unrestricted_mv &&
         s->current_picture.reference &&
         !s->intra_only) {
-        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(s->avctx->pix_fmt);
-        int hshift = desc->log2_chroma_w;
-        int vshift = desc->log2_chroma_h;
+        int hshift = s->chroma_x_shift;
+        int vshift = s->chroma_y_shift;
         s->mpvencdsp.draw_edges(s->current_picture.f->data[0],
                                 s->current_picture.f->linesize[0],
                                 s->h_edge_pos, s->v_edge_pos,
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 06/13] avcodec/mpegvideo_enc: Don't overallocate arrays

[Andreas Rheinhardt]

Only entries 0..max_b_frames are ever used.

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 1e0aed8db9..c06fdd08fe 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -819,8 +819,8 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
         !FF_ALLOCZ_TYPED_ARRAY(s->q_intra_matrix16,        32) ||
         !FF_ALLOCZ_TYPED_ARRAY(s->q_chroma_intra_matrix16, 32) ||
         !FF_ALLOCZ_TYPED_ARRAY(s->q_inter_matrix16,        32) ||
-        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_PICTURE_COUNT) ||
-        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_PICTURE_COUNT))
+        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_B_FRAMES + 1) ||
+        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_B_FRAMES + 1))
         return AVERROR(ENOMEM);
 
     /* Allocate MV tables; the MV and MB tables will be copied
@@ -1231,7 +1231,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
     }
 
     /* shift buffer entries */
-    for (i = flush_offset; i < MAX_PICTURE_COUNT /*s->encoding_delay + 1*/; i++)
+    for (int i = flush_offset; i <= MAX_B_FRAMES; i++)
         s->input_picture[i - flush_offset] = s->input_picture[i];
 
     s->input_picture[encoding_delay] = pic;
@@ -1450,9 +1450,9 @@ static int select_input_picture(MpegEncContext *s)
 {
     int i, ret;
 
-    for (i = 1; i < MAX_PICTURE_COUNT; i++)
+    for (int i = 1; i <= MAX_B_FRAMES; i++)
         s->reordered_input_picture[i - 1] = s->reordered_input_picture[i];
-    s->reordered_input_picture[MAX_PICTURE_COUNT - 1] = NULL;
+    s->reordered_input_picture[MAX_B_FRAMES] = NULL;
 
     /* set next picture type & ordering */
     if (!s->reordered_input_picture[0] && s->input_picture[0]) {
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 07/13] avcodec/mpegvideo_enc: Don't set write-only properties

[Andreas Rheinhardt]

The frame is immediately reset in the ff_mpeg_unref_picture()
call below.

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c06fdd08fe..9e28304554 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1698,10 +1698,6 @@ static int frame_start(MpegEncContext *s)
     }
 
     s->current_picture_ptr->f->pict_type = s->pict_type;
-    if (s->pict_type == AV_PICTURE_TYPE_I)
-        s->current_picture.f->flags |= AV_FRAME_FLAG_KEY;
-    else
-        s->current_picture.f->flags &= ~AV_FRAME_FLAG_KEY;
 
     ff_mpeg_unref_picture(s->avctx, &s->current_picture);
     if ((ret = ff_mpeg_ref_picture(s->avctx, &s->current_picture,
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 08/13] avcodec/mpegvideo_enc: Remove dead block

[Andreas Rheinhardt]

None of the mpegvideo encoders support anything but coded frames;
and if this were to change, it is unclear whether they would need
the adjustment here. So remove it.

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 9e28304554..5dc95ef309 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1724,19 +1724,6 @@ static int frame_start(MpegEncContext *s)
             return ret;
     }
 
-    if (s->picture_structure!= PICT_FRAME) {
-        int i;
-        for (i = 0; i < 4; i++) {
-            if (s->picture_structure == PICT_BOTTOM_FIELD) {
-                s->current_picture.f->data[i] +=
-                    s->current_picture.f->linesize[i];
-            }
-            s->current_picture.f->linesize[i] *= 2;
-            s->last_picture.f->linesize[i]    *= 2;
-            s->next_picture.f->linesize[i]    *= 2;
-        }
-    }
-
     if (s->dct_error_sum) {
         av_assert2(s->noise_reduction && s->encoding);
         update_noise_reduction(s);
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 09/13] avcodec/mpegvideo_enc: Don't allocate buffers unnecessarily

[Andreas Rheinhardt]

ff_alloc_picture() performs two tasks: a) In most instances,
it allocates frame buffers and b) it allocates certain
auxiliary buffers.

The exception to a) is the case when the encoder can reuse
user-supplied frames. And for these frames the auxiliary buffers
are unused, because this frame will never be used as current_picture
(and therefore also not as next_picture or last_picture);
see select_input_picture().

This means that we can simply avoid calling ff_alloc_picture()
with user-supplied frames at all.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mpegpicture.c   |  7 +------
 libavcodec/mpegpicture.h   |  2 +-
 libavcodec/mpegvideo_dec.c |  2 +-
 libavcodec/mpegvideo_enc.c | 16 ++++++++--------
 4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index b7c804c8ec..a0f5181325 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -247,7 +247,7 @@ static int alloc_picture_tables(AVCodecContext *avctx, Picture *pic, int encodin
  * The pixels are allocated/set by calling get_buffer() if shared = 0
  */
 int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
-                     ScratchpadContext *sc, int shared, int encoding,
+                     ScratchpadContext *sc, int encoding,
                      int chroma_x_shift, int chroma_y_shift, int out_format,
                      int mb_stride, int mb_width, int mb_height, int b8_stride,
                      ptrdiff_t *linesize, ptrdiff_t *uvlinesize)
@@ -259,10 +259,6 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
             || pic->alloc_mb_height != mb_height)
             free_picture_tables(pic);
 
-    if (shared) {
-        av_assert0(pic->f->data[0]);
-        pic->shared = 1;
-    } else {
         av_assert0(!pic->f->buf[0]);
         if (alloc_frame_buffer(avctx, pic, me, sc,
                                chroma_x_shift, chroma_y_shift,
@@ -271,7 +267,6 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
 
         *linesize   = pic->f->linesize[0];
         *uvlinesize = pic->f->linesize[1];
-    }
 
     if (!pic->qscale_table_buf)
         ret = alloc_picture_tables(avctx, pic, encoding, out_format,
diff --git a/libavcodec/mpegpicture.h b/libavcodec/mpegpicture.h
index 7919aa402c..a04820237a 100644
--- a/libavcodec/mpegpicture.h
+++ b/libavcodec/mpegpicture.h
@@ -86,7 +86,7 @@ typedef struct Picture {
  * The pixels are allocated/set by calling get_buffer() if shared = 0.
  */
 int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
-                     ScratchpadContext *sc, int shared, int encoding,
+                     ScratchpadContext *sc, int encoding,
                      int chroma_x_shift, int chroma_y_shift, int out_format,
                      int mb_stride, int mb_width, int mb_height, int b8_stride,
                      ptrdiff_t *linesize, ptrdiff_t *uvlinesize);
diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c
index b4ecb56987..07febcfe55 100644
--- a/libavcodec/mpegvideo_dec.c
+++ b/libavcodec/mpegvideo_dec.c
@@ -236,7 +236,7 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
 
 static int alloc_picture(MpegEncContext *s, Picture *pic)
 {
-    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 0, 0,
+    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 0,
                             s->chroma_x_shift, s->chroma_y_shift, s->out_format,
                             s->mb_stride, s->mb_width, s->mb_height, s->b8_stride,
                             &s->linesize, &s->uvlinesize);
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 5dc95ef309..26a596430d 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1090,9 +1090,9 @@ static int get_intra_count(MpegEncContext *s, const uint8_t *src,
     return acc;
 }
 
-static int alloc_picture(MpegEncContext *s, Picture *pic, int shared)
+static int alloc_picture(MpegEncContext *s, Picture *pic)
 {
-    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, shared, 1,
+    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 1,
                             s->chroma_x_shift, s->chroma_y_shift, s->out_format,
                             s->mb_stride, s->mb_width, s->mb_height, s->b8_stride,
                             &s->linesize, &s->uvlinesize);
@@ -1163,12 +1163,12 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
         if (direct) {
             if ((ret = av_frame_ref(pic->f, pic_arg)) < 0)
                 return ret;
-        }
-        ret = alloc_picture(s, pic, direct);
-        if (ret < 0)
-            return ret;
+            pic->shared = 1;
+        } else {
+            ret = alloc_picture(s, pic);
+            if (ret < 0)
+                return ret;
 
-        if (!direct) {
             for (int i = 0; i < 3; i++) {
                 int src_stride = pic_arg->linesize[i];
                 int dst_stride = i ? s->uvlinesize : s->linesize;
@@ -1595,7 +1595,7 @@ no_output_pic:
             pic = &s->picture[i];
 
             pic->reference = s->reordered_input_picture[0]->reference;
-            ret = alloc_picture(s, pic, 0);
+            ret = alloc_picture(s, pic);
             if (ret < 0)
                 goto fail;
 
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 10/13] avcodec/mpegvideo_enc: Don't call av_frame_copy_props() unnecessarily

[Andreas Rheinhardt]

It is unnecessary in case of user-supplied frames, because
it happens directly after a av_frame_ref() with the same
src and dst.

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

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index 26a596430d..cb6b801b2a 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1168,6 +1168,11 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             ret = alloc_picture(s, pic);
             if (ret < 0)
                 return ret;
+            ret = av_frame_copy_props(pic->f, pic_arg);
+            if (ret < 0) {
+                ff_mpeg_unref_picture(s->avctx, pic);
+                return ret;
+            }
 
             for (int i = 0; i < 3; i++) {
                 int src_stride = pic_arg->linesize[i];
@@ -1209,11 +1214,6 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
             }
             emms_c();
         }
-        ret = av_frame_copy_props(pic->f, pic_arg);
-        if (ret < 0) {
-            ff_mpeg_unref_picture(s->avctx, pic);
-            return ret;
-        }
 
         pic->display_picture_number = display_picture_number;
         pic->f->pts = pts; // we set this here to avoid modifying pic_arg
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 11/13] avcodec/mpegpicture: Move caller-specific parts of function to callers

[Andreas Rheinhardt]

Since at least commit c954cf1e1b766a0d1992d5be0a8be0055a8e1a6a
(adding ff_encode_alloc_frame()), a large part of ff_alloc_picture()
is completely separate for the two callers. Move the caller-specific
parts out to the callers.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
I know that the name ff_alloc_picture() (like lots of names in
mpegpicture) is no longer appropriate; I will fix this later
in rewrite of mpegpicture (soon...).

 libavcodec/mpegpicture.c   | 72 ++++++--------------------------------
 libavcodec/mpegpicture.h   |  6 ++--
 libavcodec/mpegvideo_dec.c | 31 ++++++++++++++--
 libavcodec/mpegvideo_enc.c | 22 ++++++++++--
 4 files changed, 62 insertions(+), 69 deletions(-)

diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index a0f5181325..44c4c8fe93 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -26,9 +26,6 @@
 #include "libavutil/imgutils.h"
 
 #include "avcodec.h"
-#include "encode.h"
-#include "internal.h"
-#include "decode.h"
 #include "motion_est.h"
 #include "mpegpicture.h"
 #include "mpegutils.h"
@@ -124,57 +121,13 @@ int ff_mpeg_framesize_alloc(AVCodecContext *avctx, MotionEstContext *me,
 }
 
 /**
- * Allocate a frame buffer
+ * Check the pic's linesize and allocate linesize dependent scratch buffers
  */
-static int alloc_frame_buffer(AVCodecContext *avctx,  Picture *pic,
-                              MotionEstContext *me, ScratchpadContext *sc,
-                              int chroma_x_shift, int chroma_y_shift,
-                              int linesize, int uvlinesize)
+static int handle_pic_linesizes(AVCodecContext *avctx, Picture *pic,
+                                MotionEstContext *me, ScratchpadContext *sc,
+                                int linesize, int uvlinesize)
 {
-    int edges_needed = av_codec_is_encoder(avctx->codec);
-    int r, ret;
-
-    pic->tf.f = pic->f;
-
-    if (edges_needed) {
-        pic->f->width  = avctx->width  + 2 * EDGE_WIDTH;
-        pic->f->height = avctx->height + 2 * EDGE_WIDTH;
-
-        r = ff_encode_alloc_frame(avctx, pic->f);
-    } else if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
-               avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
-               avctx->codec_id != AV_CODEC_ID_MSS2) {
-        r = ff_thread_get_ext_buffer(avctx, &pic->tf,
-                                     pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
-    } else {
-        pic->f->width  = avctx->width;
-        pic->f->height = avctx->height;
-        pic->f->format = avctx->pix_fmt;
-        r = avcodec_default_get_buffer2(avctx, pic->f, 0);
-    }
-
-    if (r < 0 || !pic->f->buf[0]) {
-        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed (%d %p)\n",
-               r, pic->f->data[0]);
-        return -1;
-    }
-
-    if (edges_needed) {
-        int i;
-        for (i = 0; pic->f->data[i]; i++) {
-            int offset = (EDGE_WIDTH >> (i ? chroma_y_shift : 0)) *
-                         pic->f->linesize[i] +
-                         (EDGE_WIDTH >> (i ? chroma_x_shift : 0));
-            pic->f->data[i] += offset;
-        }
-        pic->f->width  = avctx->width;
-        pic->f->height = avctx->height;
-    }
-
-    ret = ff_hwaccel_frame_priv_alloc(avctx, &pic->hwaccel_picture_private,
-                                      &pic->hwaccel_priv_buf);
-    if (ret < 0)
-        return ret;
+    int ret;
 
     if ((linesize   &&   linesize != pic->f->linesize[0]) ||
         (uvlinesize && uvlinesize != pic->f->linesize[1])) {
@@ -247,8 +200,7 @@ static int alloc_picture_tables(AVCodecContext *avctx, Picture *pic, int encodin
  * The pixels are allocated/set by calling get_buffer() if shared = 0
  */
 int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
-                     ScratchpadContext *sc, int encoding,
-                     int chroma_x_shift, int chroma_y_shift, int out_format,
+                     ScratchpadContext *sc, int encoding, int out_format,
                      int mb_stride, int mb_width, int mb_height, int b8_stride,
                      ptrdiff_t *linesize, ptrdiff_t *uvlinesize)
 {
@@ -259,14 +211,12 @@ int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
             || pic->alloc_mb_height != mb_height)
             free_picture_tables(pic);
 
-        av_assert0(!pic->f->buf[0]);
-        if (alloc_frame_buffer(avctx, pic, me, sc,
-                               chroma_x_shift, chroma_y_shift,
-                               *linesize, *uvlinesize) < 0)
-            return -1;
+    if (handle_pic_linesizes(avctx, pic, me, sc,
+                             *linesize, *uvlinesize) < 0)
+        return -1;
 
-        *linesize   = pic->f->linesize[0];
-        *uvlinesize = pic->f->linesize[1];
+    *linesize   = pic->f->linesize[0];
+    *uvlinesize = pic->f->linesize[1];
 
     if (!pic->qscale_table_buf)
         ret = alloc_picture_tables(avctx, pic, encoding, out_format,
diff --git a/libavcodec/mpegpicture.h b/libavcodec/mpegpicture.h
index a04820237a..1c1e5ddbd1 100644
--- a/libavcodec/mpegpicture.h
+++ b/libavcodec/mpegpicture.h
@@ -82,12 +82,10 @@ typedef struct Picture {
 } Picture;
 
 /**
- * Allocate a Picture.
- * The pixels are allocated/set by calling get_buffer() if shared = 0.
+ * Allocate a Picture's accessories, but not the AVFrame's buffer itself.
  */
 int ff_alloc_picture(AVCodecContext *avctx, Picture *pic, MotionEstContext *me,
-                     ScratchpadContext *sc, int encoding,
-                     int chroma_x_shift, int chroma_y_shift, int out_format,
+                     ScratchpadContext *sc, int encoding, int out_format,
                      int mb_stride, int mb_width, int mb_height, int b8_stride,
                      ptrdiff_t *linesize, ptrdiff_t *uvlinesize);
 
diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c
index 07febcfe55..4e00137cbb 100644
--- a/libavcodec/mpegvideo_dec.c
+++ b/libavcodec/mpegvideo_dec.c
@@ -31,6 +31,7 @@
 #include "libavutil/video_enc_params.h"
 
 #include "avcodec.h"
+#include "decode.h"
 #include "h264chroma.h"
 #include "internal.h"
 #include "mpegutils.h"
@@ -236,10 +237,36 @@ int ff_mpv_common_frame_size_change(MpegEncContext *s)
 
 static int alloc_picture(MpegEncContext *s, Picture *pic)
 {
-    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 0,
-                            s->chroma_x_shift, s->chroma_y_shift, s->out_format,
+    AVCodecContext *avctx = s->avctx;
+    int ret;
+
+    pic->tf.f = pic->f;
+
+    if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
+        avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
+        avctx->codec_id != AV_CODEC_ID_MSS2) {
+        ret = ff_thread_get_ext_buffer(avctx, &pic->tf,
+                                       pic->reference ? AV_GET_BUFFER_FLAG_REF : 0);
+    } else {
+        pic->f->width  = avctx->width;
+        pic->f->height = avctx->height;
+        pic->f->format = avctx->pix_fmt;
+        ret = avcodec_default_get_buffer2(avctx, pic->f, 0);
+    }
+    if (ret < 0)
+        goto fail;
+
+    ret = ff_hwaccel_frame_priv_alloc(avctx, &pic->hwaccel_picture_private,
+                                      &pic->hwaccel_priv_buf);
+    if (ret < 0)
+        goto fail;
+
+    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 0, s->out_format,
                             s->mb_stride, s->mb_width, s->mb_height, s->b8_stride,
                             &s->linesize, &s->uvlinesize);
+fail:
+    ff_mpeg_unref_picture(avctx, pic);
+    return ret;
 }
 
 static void color_frame(AVFrame *frame, int luma)
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index cb6b801b2a..fe097c2c3b 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -1092,8 +1092,26 @@ static int get_intra_count(MpegEncContext *s, const uint8_t *src,
 
 static int alloc_picture(MpegEncContext *s, Picture *pic)
 {
-    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 1,
-                            s->chroma_x_shift, s->chroma_y_shift, s->out_format,
+    AVCodecContext *avctx = s->avctx;
+    int ret;
+
+    pic->f->width  = avctx->width  + 2 * EDGE_WIDTH;
+    pic->f->height = avctx->height + 2 * EDGE_WIDTH;
+
+    ret = ff_encode_alloc_frame(avctx, pic->f);
+    if (ret < 0)
+        return ret;
+
+    for (int i = 0; pic->f->data[i]; i++) {
+        int offset = (EDGE_WIDTH >> (i ? s->chroma_y_shift : 0)) *
+                     pic->f->linesize[i] +
+                     (EDGE_WIDTH >> (i ? s->chroma_x_shift : 0));
+        pic->f->data[i] += offset;
+    }
+    pic->f->width  = avctx->width;
+    pic->f->height = avctx->height;
+
+    return ff_alloc_picture(s->avctx, pic, &s->me, &s->sc, 1, s->out_format,
                             s->mb_stride, s->mb_width, s->mb_height, s->b8_stride,
                             &s->linesize, &s->uvlinesize);
 }
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 12/13] avcodec/mpeg(picture|video_dec): Move comment to more appropriate place

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mpegpicture.c   | 2 --
 libavcodec/mpegvideo_dec.c | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index 44c4c8fe93..f859f163a6 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -252,8 +252,6 @@ fail:
 void ff_mpeg_unref_picture(AVCodecContext *avctx, Picture *pic)
 {
     pic->tf.f = pic->f;
-    /* WM Image / Screen codecs allocate internal buffers with different
-     * dimensions / colorspaces; ignore user-defined callbacks for these. */
     if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
         avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
         avctx->codec_id != AV_CODEC_ID_MSS2)
diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c
index 4e00137cbb..7a9101a781 100644
--- a/libavcodec/mpegvideo_dec.c
+++ b/libavcodec/mpegvideo_dec.c
@@ -242,6 +242,8 @@ static int alloc_picture(MpegEncContext *s, Picture *pic)
 
     pic->tf.f = pic->f;
 
+    /* WM Image / Screen codecs allocate internal buffers with different
+     * dimensions / colorspaces; ignore user-defined callbacks for these. */
     if (avctx->codec_id != AV_CODEC_ID_WMV3IMAGE &&
         avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
         avctx->codec_id != AV_CODEC_ID_MSS2) {
-- 
2.34.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 13/13] avcodec/vdpau_vc1: Fix indentation

[Andreas Rheinhardt]

Forgotten after af6e232ccf8db2341361f02852017aac48772950.

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

diff --git a/libavcodec/vdpau_vc1.c b/libavcodec/vdpau_vc1.c
index 4e5ee14428..0eacc4477d 100644
--- a/libavcodec/vdpau_vc1.c
+++ b/libavcodec/vdpau_vc1.c
@@ -48,16 +48,16 @@ static int vdpau_vc1_start_frame(AVCodecContext *avctx,
     switch (s->pict_type) {
     case AV_PICTURE_TYPE_B:
         if (s->next_picture_ptr) {
-        ref = ff_vdpau_get_surface_id(s->next_picture.f);
-        assert(ref != VDP_INVALID_HANDLE);
-        info->backward_reference = ref;
+            ref = ff_vdpau_get_surface_id(s->next_picture.f);
+            assert(ref != VDP_INVALID_HANDLE);
+            info->backward_reference = ref;
         }
         /* fall-through */
     case AV_PICTURE_TYPE_P:
         if (s->last_picture_ptr) {
-        ref = ff_vdpau_get_surface_id(s->last_picture.f);
-        assert(ref != VDP_INVALID_HANDLE);
-        info->forward_reference  = ref;
+            ref = ff_vdpau_get_surface_id(s->last_picture.f);
+            assert(ref != VDP_INVALID_HANDLE);
+            info->forward_reference  = ref;
         }
     }
 
-- 
2.34.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 06/13] avcodec/mpegvideo_enc: Don't overallocate arrays

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2218 bytes --]

On Fri, Oct 06, 2023 at 04:46:29AM +0200, Andreas Rheinhardt wrote:
> Only entries 0..max_b_frames are ever used.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/mpegvideo_enc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index 1e0aed8db9..c06fdd08fe 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -819,8 +819,8 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
>          !FF_ALLOCZ_TYPED_ARRAY(s->q_intra_matrix16,        32) ||
>          !FF_ALLOCZ_TYPED_ARRAY(s->q_chroma_intra_matrix16, 32) ||
>          !FF_ALLOCZ_TYPED_ARRAY(s->q_inter_matrix16,        32) ||
> -        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_PICTURE_COUNT) ||
> -        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_PICTURE_COUNT))
> +        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_B_FRAMES + 1) ||
> +        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_B_FRAMES + 1))
>          return AVERROR(ENOMEM);
>  
>      /* Allocate MV tables; the MV and MB tables will be copied
> @@ -1231,7 +1231,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
>      }
>  
>      /* shift buffer entries */
> -    for (i = flush_offset; i < MAX_PICTURE_COUNT /*s->encoding_delay + 1*/; i++)
> +    for (int i = flush_offset; i <= MAX_B_FRAMES; i++)
>          s->input_picture[i - flush_offset] = s->input_picture[i];
>  
>      s->input_picture[encoding_delay] = pic;
> @@ -1450,9 +1450,9 @@ static int select_input_picture(MpegEncContext *s)
>  {
>      int i, ret;
>  
> -    for (i = 1; i < MAX_PICTURE_COUNT; i++)
> +    for (int i = 1; i <= MAX_B_FRAMES; i++)
>          s->reordered_input_picture[i - 1] = s->reordered_input_picture[i];

I see the addition of "int" and that seems neither needed nor
explained why in the commit message

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 06/13] avcodec/mpegvideo_enc: Don't overallocate arrays

[Andreas Rheinhardt]

Michael Niedermayer:
> On Fri, Oct 06, 2023 at 04:46:29AM +0200, Andreas Rheinhardt wrote:
>> Only entries 0..max_b_frames are ever used.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavcodec/mpegvideo_enc.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>> index 1e0aed8db9..c06fdd08fe 100644
>> --- a/libavcodec/mpegvideo_enc.c
>> +++ b/libavcodec/mpegvideo_enc.c
>> @@ -819,8 +819,8 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_intra_matrix16,        32) ||
>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_chroma_intra_matrix16, 32) ||
>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_inter_matrix16,        32) ||
>> -        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_PICTURE_COUNT) ||
>> -        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_PICTURE_COUNT))
>> +        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_B_FRAMES + 1) ||
>> +        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_B_FRAMES + 1))
>>          return AVERROR(ENOMEM);
>>  
>>      /* Allocate MV tables; the MV and MB tables will be copied
>> @@ -1231,7 +1231,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
>>      }
>>  
>>      /* shift buffer entries */
>> -    for (i = flush_offset; i < MAX_PICTURE_COUNT /*s->encoding_delay + 1*/; i++)
>> +    for (int i = flush_offset; i <= MAX_B_FRAMES; i++)
>>          s->input_picture[i - flush_offset] = s->input_picture[i];
>>  
>>      s->input_picture[encoding_delay] = pic;
>> @@ -1450,9 +1450,9 @@ static int select_input_picture(MpegEncContext *s)
>>  {
>>      int i, ret;
>>  
>> -    for (i = 1; i < MAX_PICTURE_COUNT; i++)
>> +    for (int i = 1; i <= MAX_B_FRAMES; i++)
>>          s->reordered_input_picture[i - 1] = s->reordered_input_picture[i];
> 
> I see the addition of "int" and that seems neither needed nor
> explained why in the commit message
> 

It's part of the general switch to the loop-based iterators wherever
possible (it is better because it automatically indicates that the value
at the end of the loop doesn't matter and it also allows to more easily
move blocks of code around). I always use them when I touch a loop.

If it matters: That i in the outer scope survives this patchset, but it
won't survive for long.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 12/13] avcodec/mpeg(picture|video_dec): Move comment to more appropriate place

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 564 bytes --]

On Fri, Oct 06, 2023 at 04:46:35AM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/mpegpicture.c   | 2 --
>  libavcodec/mpegvideo_dec.c | 2 ++
>  2 files changed, 2 insertions(+), 2 deletions(-)

ok

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 05/13] avcodec/mpegvideo_enc: Don't reget known values

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]

On Fri, Oct 06, 2023 at 04:46:28AM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/mpegvideo_enc.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)

LGTM

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 06/13] avcodec/mpegvideo_enc: Don't overallocate arrays

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Michael Niedermayer:
>> On Fri, Oct 06, 2023 at 04:46:29AM +0200, Andreas Rheinhardt wrote:
>>> Only entries 0..max_b_frames are ever used.
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>>  libavcodec/mpegvideo_enc.c | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>>> index 1e0aed8db9..c06fdd08fe 100644
>>> --- a/libavcodec/mpegvideo_enc.c
>>> +++ b/libavcodec/mpegvideo_enc.c
>>> @@ -819,8 +819,8 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
>>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_intra_matrix16,        32) ||
>>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_chroma_intra_matrix16, 32) ||
>>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_inter_matrix16,        32) ||
>>> -        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_PICTURE_COUNT) ||
>>> -        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_PICTURE_COUNT))
>>> +        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_B_FRAMES + 1) ||
>>> +        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_B_FRAMES + 1))
>>>          return AVERROR(ENOMEM);
>>>  
>>>      /* Allocate MV tables; the MV and MB tables will be copied
>>> @@ -1231,7 +1231,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
>>>      }
>>>  
>>>      /* shift buffer entries */
>>> -    for (i = flush_offset; i < MAX_PICTURE_COUNT /*s->encoding_delay + 1*/; i++)
>>> +    for (int i = flush_offset; i <= MAX_B_FRAMES; i++)
>>>          s->input_picture[i - flush_offset] = s->input_picture[i];
>>>  
>>>      s->input_picture[encoding_delay] = pic;
>>> @@ -1450,9 +1450,9 @@ static int select_input_picture(MpegEncContext *s)
>>>  {
>>>      int i, ret;
>>>  
>>> -    for (i = 1; i < MAX_PICTURE_COUNT; i++)
>>> +    for (int i = 1; i <= MAX_B_FRAMES; i++)
>>>          s->reordered_input_picture[i - 1] = s->reordered_input_picture[i];
>>
>> I see the addition of "int" and that seems neither needed nor
>> explained why in the commit message
>>
> 
> It's part of the general switch to the loop-based iterators wherever
> possible (it is better because it automatically indicates that the value
> at the end of the loop doesn't matter and it also allows to more easily
> move blocks of code around). I always use them when I touch a loop.
> 
> If it matters: That i in the outer scope survives this patchset, but it
> won't survive for long.
> 

Are you ok with this patch and the rest of the patchset? I'd like to
apply it tomorrow unless there are objections.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 06/13] avcodec/mpegvideo_enc: Don't overallocate arrays

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3140 bytes --]

On Mon, Oct 09, 2023 at 02:30:21PM +0200, Andreas Rheinhardt wrote:
> Andreas Rheinhardt:
> > Michael Niedermayer:
> >> On Fri, Oct 06, 2023 at 04:46:29AM +0200, Andreas Rheinhardt wrote:
> >>> Only entries 0..max_b_frames are ever used.
> >>>
> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> >>> ---
> >>>  libavcodec/mpegvideo_enc.c | 10 +++++-----
> >>>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> >>> index 1e0aed8db9..c06fdd08fe 100644
> >>> --- a/libavcodec/mpegvideo_enc.c
> >>> +++ b/libavcodec/mpegvideo_enc.c
> >>> @@ -819,8 +819,8 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
> >>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_intra_matrix16,        32) ||
> >>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_chroma_intra_matrix16, 32) ||
> >>>          !FF_ALLOCZ_TYPED_ARRAY(s->q_inter_matrix16,        32) ||
> >>> -        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_PICTURE_COUNT) ||
> >>> -        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_PICTURE_COUNT))
> >>> +        !FF_ALLOCZ_TYPED_ARRAY(s->input_picture,           MAX_B_FRAMES + 1) ||
> >>> +        !FF_ALLOCZ_TYPED_ARRAY(s->reordered_input_picture, MAX_B_FRAMES + 1))
> >>>          return AVERROR(ENOMEM);
> >>>  
> >>>      /* Allocate MV tables; the MV and MB tables will be copied
> >>> @@ -1231,7 +1231,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg)
> >>>      }
> >>>  
> >>>      /* shift buffer entries */
> >>> -    for (i = flush_offset; i < MAX_PICTURE_COUNT /*s->encoding_delay + 1*/; i++)
> >>> +    for (int i = flush_offset; i <= MAX_B_FRAMES; i++)
> >>>          s->input_picture[i - flush_offset] = s->input_picture[i];
> >>>  
> >>>      s->input_picture[encoding_delay] = pic;
> >>> @@ -1450,9 +1450,9 @@ static int select_input_picture(MpegEncContext *s)
> >>>  {
> >>>      int i, ret;
> >>>  
> >>> -    for (i = 1; i < MAX_PICTURE_COUNT; i++)
> >>> +    for (int i = 1; i <= MAX_B_FRAMES; i++)
> >>>          s->reordered_input_picture[i - 1] = s->reordered_input_picture[i];
> >>
> >> I see the addition of "int" and that seems neither needed nor
> >> explained why in the commit message
> >>
> > 
> > It's part of the general switch to the loop-based iterators wherever
> > possible (it is better because it automatically indicates that the value
> > at the end of the loop doesn't matter and it also allows to more easily
> > move blocks of code around). I always use them when I touch a loop.
> > 
> > If it matters: That i in the outer scope survives this patchset, but it
> > won't survive for long.
> > 
> 
> Are you ok with this patch

ok


> and the rest of the patchset?  I'd like to
> apply it tomorrow unless there are objections.

i have no objections




thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".