diff mbox series

[FFmpeg-devel,v2,2/4] avcodec/mpegvideo: use FF_ALLOC{Z}_TYPED_ARRAY_OR_GOTO instead

Message ID 1589212343-8334-2-git-send-email-lance.lmwang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,v2,1/4] avutil/internal: add FF_ALLOC_TYPED_ARRAY_OR_GOTO & FF_ALLOCZ_TYPED_ARRAY_OR_GOTO | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Lance Wang May 11, 2020, 3:52 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/mpegvideo.c | 60 ++++++++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 34 deletions(-)

Comments

James Almer May 11, 2020, 4:27 p.m. UTC | #1
On 5/11/2020 12:52 PM, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavcodec/mpegvideo.c | 60 ++++++++++++++++++++++----------------------------
>  1 file changed, 26 insertions(+), 34 deletions(-)
> 
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index 49fd1c9..6f52e75 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -372,16 +372,13 @@ static int init_duplicate_context(MpegEncContext *s)
>      s->sc.obmc_scratchpad = NULL;
>  
>      if (s->encoding) {
> -        FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> -        FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> +        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
> +        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->me.score_map, ME_MAP_SIZE, fail)

Why not just do FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.score_map,
ME_MAP_SIZE, sizeof(*s->me.score_map), fail)
Same for every other case in the patchset.

I don't know if it's worth adding a new macro just to save writing one
argument, but i have no strong feelings about it, so if others are ok
with it then it's fine.
Lance Wang May 12, 2020, 12:10 a.m. UTC | #2
On Mon, May 11, 2020 at 01:27:36PM -0300, James Almer wrote:
> On 5/11/2020 12:52 PM, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavcodec/mpegvideo.c | 60 ++++++++++++++++++++++----------------------------
> >  1 file changed, 26 insertions(+), 34 deletions(-)
> > 
> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > index 49fd1c9..6f52e75 100644
> > --- a/libavcodec/mpegvideo.c
> > +++ b/libavcodec/mpegvideo.c
> > @@ -372,16 +372,13 @@ static int init_duplicate_context(MpegEncContext *s)
> >      s->sc.obmc_scratchpad = NULL;
> >  
> >      if (s->encoding) {
> > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > +        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
> > +        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->me.score_map, ME_MAP_SIZE, fail)
> 
> Why not just do FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.score_map,
> ME_MAP_SIZE, sizeof(*s->me.score_map), fail)
> Same for every other case in the patchset.
> 
> I don't know if it's worth adding a new macro just to save writing one
> argument, but i have no strong feelings about it, so if others are ok
> with it then it's fine.

Now I'm prefer to the new macro, for some sizeof is too long and had to split two
line. But it's more fine if the macro is short name.


> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
index 49fd1c9..6f52e75 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -372,16 +372,13 @@  static int init_duplicate_context(MpegEncContext *s)
     s->sc.obmc_scratchpad = NULL;
 
     if (s->encoding) {
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
-                          ME_MAP_SIZE * sizeof(uint32_t), fail)
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
-                          ME_MAP_SIZE * sizeof(uint32_t), fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->me.score_map, ME_MAP_SIZE, fail)
         if (s->noise_reduction) {
-            FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
-                              2 * 64 * sizeof(int), fail)
+            FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->dct_error_sum, 2 * 64, fail)
         }
     }
-    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
+    FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2, fail)
     s->block = s->blocks[0];
 
     for (i = 0; i < 12; i++) {
@@ -399,8 +396,7 @@  static int init_duplicate_context(MpegEncContext *s)
 
     if (s->out_format == FMT_H263) {
         /* ac values */
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->ac_val_base,
-                          yc_size * sizeof(int16_t) * 16, fail);
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->ac_val_base, yc_size * 16, fail);
         s->ac_val[0] = s->ac_val_base + s->b8_stride + 1;
         s->ac_val[1] = s->ac_val_base + y_size + s->mb_stride + 1;
         s->ac_val[2] = s->ac_val[1] + c_size;
@@ -715,7 +711,7 @@  static int init_context_frame(MpegEncContext *s)
     if (s->mb_height & 1)
         yc_size += 2*s->b8_stride + 2*s->mb_stride;
 
-    FF_ALLOCZ_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(int),
+    FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->mb_index2xy, s->mb_num + 1,
                       fail); // error resilience code looks cleaner with this
     for (y = 0; y < s->mb_height; y++)
         for (x = 0; x < s->mb_width; x++)
@@ -725,12 +721,12 @@  static int init_context_frame(MpegEncContext *s)
 
     if (s->encoding) {
         /* Allocate MV tables */
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(int16_t), fail)
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(int16_t), fail)
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(int16_t), fail)
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(int16_t), fail)
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(int16_t), fail)
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(int16_t), fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size, fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size, fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size, fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size, fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size, fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size, fail)
         s->p_mv_table            = s->p_mv_table_base + s->mb_stride + 1;
         s->b_forw_mv_table       = s->b_forw_mv_table_base + s->mb_stride + 1;
         s->b_back_mv_table       = s->b_back_mv_table_base + s->mb_stride + 1;
@@ -739,14 +735,12 @@  static int init_context_frame(MpegEncContext *s)
         s->b_direct_mv_table     = s->b_direct_mv_table_base + s->mb_stride + 1;
 
         /* Allocate MB type table */
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(uint16_t), fail) // needed for encoding
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->mb_type, mb_array_size, fail) // needed for encoding
 
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(int), fail)
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->lambda_table, mb_array_size, fail)
 
-        FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
-                         mb_array_size * sizeof(float), fail);
-        FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
-                         mb_array_size * sizeof(float), fail);
+        FF_ALLOC_TYPED_ARRAY_OR_GOTO(s->avctx, s->cplx_tab, mb_array_size, fail);
+        FF_ALLOC_TYPED_ARRAY_OR_GOTO(s->avctx, s->bits_tab, mb_array_size, fail);
 
     }
 
@@ -757,34 +751,33 @@  static int init_context_frame(MpegEncContext *s)
             int j, k;
             for (j = 0; j < 2; j++) {
                 for (k = 0; k < 2; k++) {
-                    FF_ALLOCZ_OR_GOTO(s->avctx,
+                    FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx,
                                       s->b_field_mv_table_base[i][j][k],
-                                      mv_table_size * 2 * sizeof(int16_t),
-                                      fail);
+                                      mv_table_size * 2, fail);
                     s->b_field_mv_table[i][j][k] = s->b_field_mv_table_base[i][j][k] +
                                                    s->mb_stride + 1;
                 }
-                FF_ALLOCZ_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(uint8_t), fail)
-                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(int16_t), fail)
+                FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2, fail)
+                FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2, fail)
                 s->p_field_mv_table[i][j] = s->p_field_mv_table_base[i][j] + s->mb_stride + 1;
             }
-            FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(uint8_t), fail)
+            FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2, fail)
         }
     }
     if (s->out_format == FMT_H263) {
         /* cbp values */
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->coded_block_base, y_size + (s->mb_height&1)*2*s->b8_stride, fail);
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->coded_block_base, y_size + (s->mb_height&1)*2*s->b8_stride, fail);
         s->coded_block = s->coded_block_base + s->b8_stride + 1;
 
         /* cbp, ac_pred, pred_dir */
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(uint8_t), fail);
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(uint8_t), fail);
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size, fail);
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size, fail);
     }
 
     if (s->h263_pred || s->h263_plus || !s->encoding) {
         /* dc values */
         // MN: we need these for error resilience of intra-frames
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(int16_t), fail);
+        FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->dc_val_base, yc_size, fail);
         s->dc_val[0] = s->dc_val_base + s->b8_stride + 1;
         s->dc_val[1] = s->dc_val_base + y_size + s->mb_stride + 1;
         s->dc_val[2] = s->dc_val[1] + c_size;
@@ -934,8 +927,7 @@  av_cold int ff_mpv_common_init(MpegEncContext *s)
     if (ret)
         return ret;
 
-    FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
-                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
+    FF_ALLOCZ_TYPED_ARRAY_OR_GOTO(s->avctx, s->picture, MAX_PICTURE_COUNT, fail_nomem);
     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
         s->picture[i].f = av_frame_alloc();
         if (!s->picture[i].f)