diff mbox series

[FFmpeg-devel,1/2] avcodec/mpegvideo: prefer to use variable instead of type for sizeof

Message ID 1589125170-32558-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] avcodec/mpegvideo: prefer to use variable instead of type for sizeof
Related show

Checks

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

Commit Message

Limin Wang May 10, 2020, 3:39 p.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

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

Comments

Marton Balint May 10, 2020, 4:30 p.m. UTC | #1
On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:

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

If you find these cosmetics interesting, then I suggest you introduce a 
new macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().

E.g.:

FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)

Regards,
Marton

>
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index 49fd1c9..561062f 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
>
>     if (s->encoding) {
>         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
>         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
>         if (s->noise_reduction) {
>             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
> -                              2 * 64 * sizeof(int), fail)
> +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
>         }
>     }
> -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
> +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
>     s->block = s->blocks[0];
>
>     for (i = 0; i < 12; i++) {
> @@ -400,7 +400,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);
> +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
>                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
> 
> -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(int), fail)
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
>
>         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
> -                         mb_array_size * sizeof(float), fail);
> +                         mb_array_size * sizeof(*s->cplx_tab), fail);
>         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
> -                         mb_array_size * sizeof(float), fail);
> +                         mb_array_size * sizeof(*s->bits_tab), fail);
>
>     }
> 
> @@ -759,16 +759,16 @@ static int init_context_frame(MpegEncContext *s)
>                 for (k = 0; k < 2; k++) {
>                     FF_ALLOCZ_OR_GOTO(s->avctx,
>                                       s->b_field_mv_table_base[i][j][k],
> -                                      mv_table_size * 2 * sizeof(int16_t),
> +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
>                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
> +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
>         }
>     }
>     if (s->out_format == FMT_H263) {
> @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
>         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
> +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
> @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>         return ret;
>
>     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
> -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
> +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
>     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
>         s->picture[i].f = av_frame_alloc();
>         if (!s->picture[i].f)
> -- 
> 1.8.3.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".
Limin Wang May 10, 2020, 11:16 p.m. UTC | #2
On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:
> 
> 
> On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavcodec/mpegvideo.c | 48 ++++++++++++++++++++++++------------------------
> > 1 file changed, 24 insertions(+), 24 deletions(-)
> 
> If you find these cosmetics interesting, then I suggest you introduce a new
> macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().
> 
> E.g.:
> 
> FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)

Yeah, I have considered it so I change the type to use use variable first and
submit one typical for review. If the change is OK, then I'll go ahead next. 

> 
> Regards,
> Marton
> 
> > 
> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > index 49fd1c9..561062f 100644
> > --- a/libavcodec/mpegvideo.c
> > +++ b/libavcodec/mpegvideo.c
> > @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
> > 
> >     if (s->encoding) {
> >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
> >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
> >         if (s->noise_reduction) {
> >             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
> > -                              2 * 64 * sizeof(int), fail)
> > +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
> >         }
> >     }
> > -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
> > +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
> >     s->block = s->blocks[0];
> > 
> >     for (i = 0; i < 12; i++) {
> > @@ -400,7 +400,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);
> > +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
> >                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
> > 
> > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(int), fail)
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
> > 
> >         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
> > -                         mb_array_size * sizeof(float), fail);
> > +                         mb_array_size * sizeof(*s->cplx_tab), fail);
> >         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
> > -                         mb_array_size * sizeof(float), fail);
> > +                         mb_array_size * sizeof(*s->bits_tab), fail);
> > 
> >     }
> > 
> > @@ -759,16 +759,16 @@ static int init_context_frame(MpegEncContext *s)
> >                 for (k = 0; k < 2; k++) {
> >                     FF_ALLOCZ_OR_GOTO(s->avctx,
> >                                       s->b_field_mv_table_base[i][j][k],
> > -                                      mv_table_size * 2 * sizeof(int16_t),
> > +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
> >                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
> > +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
> >         }
> >     }
> >     if (s->out_format == FMT_H263) {
> > @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
> >         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
> > @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> >         return ret;
> > 
> >     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
> > -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
> > +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
> >     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
> >         s->picture[i].f = av_frame_alloc();
> >         if (!s->picture[i].f)
> > -- 
> > 1.8.3.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 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".
Marton Balint May 10, 2020, 11:22 p.m. UTC | #3
On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:

> On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:
>> 
>> 
>> On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:
>> 
>> > From: Limin Wang <lance.lmwang@gmail.com>
>> > 
>> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> > ---
>> > libavcodec/mpegvideo.c | 48 ++++++++++++++++++++++++------------------------
>> > 1 file changed, 24 insertions(+), 24 deletions(-)
>> 
>> If you find these cosmetics interesting, then I suggest you introduce a new
>> macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().
>> 
>> E.g.:
>> 
>> FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
>
> Yeah, I have considered it so I change the type to use use variable first and
> submit one typical for review. If the change is OK, then I'll go ahead next.

No need to do it in two steps, better touch the code once. E.g. patch 2 
might not even be needed if you do this in a single patch.

Regards,
Marton

>
>> 
>> Regards,
>> Marton
>> 
>> > 
>> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
>> > index 49fd1c9..561062f 100644
>> > --- a/libavcodec/mpegvideo.c
>> > +++ b/libavcodec/mpegvideo.c
>> > @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
>> > 
>> >     if (s->encoding) {
>> >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
>> > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
>> > +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
>> >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
>> > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
>> > +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
>> >         if (s->noise_reduction) {
>> >             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
>> > -                              2 * 64 * sizeof(int), fail)
>> > +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
>> >         }
>> >     }
>> > -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
>> > +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
>> >     s->block = s->blocks[0];
>> > 
>> >     for (i = 0; i < 12; i++) {
>> > @@ -400,7 +400,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);
>> > +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
>> >                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
>> > 
>> > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(int), fail)
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
>> > 
>> >         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
>> > -                         mb_array_size * sizeof(float), fail);
>> > +                         mb_array_size * sizeof(*s->cplx_tab), fail);
>> >         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
>> > -                         mb_array_size * sizeof(float), fail);
>> > +                         mb_array_size * sizeof(*s->bits_tab), fail);
>> > 
>> >     }
>> > 
>> > @@ -759,16 +759,16 @@ static int init_context_frame(MpegEncContext *s)
>> >                 for (k = 0; k < 2; k++) {
>> >                     FF_ALLOCZ_OR_GOTO(s->avctx,
>> >                                       s->b_field_mv_table_base[i][j][k],
>> > -                                      mv_table_size * 2 * sizeof(int16_t),
>> > +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
>> >                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
>> > +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
>> >         }
>> >     }
>> >     if (s->out_format == FMT_H263) {
>> > @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
>> >         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
>> > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
>> > @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>> >         return ret;
>> > 
>> >     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
>> > -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
>> > +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
>> >     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
>> >         s->picture[i].f = av_frame_alloc();
>> >         if (!s->picture[i].f)
>> > -- 
>> > 1.8.3.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 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".
>
> -- 
> Thanks,
> Limin Wang
> _______________________________________________
> 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".
Limin Wang May 11, 2020, 1:12 a.m. UTC | #4
On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
> 
> 
> On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:
> 
> > On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:
> > > 
> > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > ---
> > > > libavcodec/mpegvideo.c | 48 ++++++++++++++++++++++++------------------------
> > > > 1 file changed, 24 insertions(+), 24 deletions(-)
> > > 
> > > If you find these cosmetics interesting, then I suggest you introduce a new
> > > macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().
> > > 
> > > E.g.:
> > > 
> > > FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
> > 
> > Yeah, I have considered it so I change the type to use use variable first and
> > submit one typical for review. If the change is OK, then I'll go ahead next.
> 
> No need to do it in two steps, better touch the code once. E.g. patch 2
> might not even be needed if you do this in a single patch.

OK, I'll update the patch later.

> 
> Regards,
> Marton
> 
> > 
> > > 
> > > Regards,
> > > Marton
> > > 
> > > > > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > > > index 49fd1c9..561062f 100644
> > > > --- a/libavcodec/mpegvideo.c
> > > > +++ b/libavcodec/mpegvideo.c
> > > > @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
> > > > >     if (s->encoding) {
> > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > > > +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
> > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > > > +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
> > > >         if (s->noise_reduction) {
> > > >             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
> > > > -                              2 * 64 * sizeof(int), fail)
> > > > +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
> > > >         }
> > > >     }
> > > > -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
> > > > +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
> > > >     s->block = s->blocks[0];
> > > > >     for (i = 0; i < 12; i++) {
> > > > @@ -400,7 +400,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);
> > > > +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
> > > >                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
> > > > > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table,
> > > mb_array_size * sizeof(int), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
> > > > >         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
> > > > -                         mb_array_size * sizeof(float), fail);
> > > > +                         mb_array_size * sizeof(*s->cplx_tab), fail);
> > > >         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
> > > > -                         mb_array_size * sizeof(float), fail);
> > > > +                         mb_array_size * sizeof(*s->bits_tab), fail);
> > > > >     }
> > > > > @@ -759,16 +759,16 @@ static int
> > > init_context_frame(MpegEncContext *s)
> > > >                 for (k = 0; k < 2; k++) {
> > > >                     FF_ALLOCZ_OR_GOTO(s->avctx,
> > > >                                       s->b_field_mv_table_base[i][j][k],
> > > > -                                      mv_table_size * 2 * sizeof(int16_t),
> > > > +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
> > > >                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
> > > > +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
> > > >         }
> > > >     }
> > > >     if (s->out_format == FMT_H263) {
> > > > @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
> > > >         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
> > > > @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> > > >         return ret;
> > > > >     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
> > > > -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
> > > > +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
> > > >     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
> > > >         s->picture[i].f = av_frame_alloc();
> > > >         if (!s->picture[i].f)
> > > > -- > 1.8.3.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 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".
> > 
> > -- 
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > 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 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".
Limin Wang May 11, 2020, 2:08 a.m. UTC | #5
On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
> 
> 
> On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:
> 
> > On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:
> > > 
> > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > ---
> > > > libavcodec/mpegvideo.c | 48 ++++++++++++++++++++++++------------------------
> > > > 1 file changed, 24 insertions(+), 24 deletions(-)
> > > 
> > > If you find these cosmetics interesting, then I suggest you introduce a new
> > > macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().
> > > 
> > > E.g.:
> > > 
> > > FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
> > 
> > Yeah, I have considered it so I change the type to use use variable first and
> > submit one typical for review. If the change is OK, then I'll go ahead next.
> 
> No need to do it in two steps, better touch the code once. E.g. patch 2
> might not even be needed if you do this in a single patch.

Sorry, now for array alloc, you had to input the one elemeay size and its count
always. internal.h have defined it already:
FF_ALLOCZ_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)

You don't want to input elsize? isn't general usage?

> 
> Regards,
> Marton
> 
> > 
> > > 
> > > Regards,
> > > Marton
> > > 
> > > > > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > > > index 49fd1c9..561062f 100644
> > > > --- a/libavcodec/mpegvideo.c
> > > > +++ b/libavcodec/mpegvideo.c
> > > > @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
> > > > >     if (s->encoding) {
> > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > > > +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
> > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > > > +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
> > > >         if (s->noise_reduction) {
> > > >             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
> > > > -                              2 * 64 * sizeof(int), fail)
> > > > +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
> > > >         }
> > > >     }
> > > > -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
> > > > +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
> > > >     s->block = s->blocks[0];
> > > > >     for (i = 0; i < 12; i++) {
> > > > @@ -400,7 +400,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);
> > > > +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
> > > >                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
> > > > > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table,
> > > mb_array_size * sizeof(int), fail)
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
> > > > >         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
> > > > -                         mb_array_size * sizeof(float), fail);
> > > > +                         mb_array_size * sizeof(*s->cplx_tab), fail);
> > > >         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
> > > > -                         mb_array_size * sizeof(float), fail);
> > > > +                         mb_array_size * sizeof(*s->bits_tab), fail);
> > > > >     }
> > > > > @@ -759,16 +759,16 @@ static int
> > > init_context_frame(MpegEncContext *s)
> > > >                 for (k = 0; k < 2; k++) {
> > > >                     FF_ALLOCZ_OR_GOTO(s->avctx,
> > > >                                       s->b_field_mv_table_base[i][j][k],
> > > > -                                      mv_table_size * 2 * sizeof(int16_t),
> > > > +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
> > > >                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
> > > > +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
> > > >         }
> > > >     }
> > > >     if (s->out_format == FMT_H263) {
> > > > @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
> > > >         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
> > > > @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> > > >         return ret;
> > > > >     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
> > > > -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
> > > > +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
> > > >     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
> > > >         s->picture[i].f = av_frame_alloc();
> > > >         if (!s->picture[i].f)
> > > > -- > 1.8.3.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 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".
> > 
> > -- 
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > 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 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".
Marton Balint May 11, 2020, 7:16 a.m. UTC | #6
On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:

> On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
>> 
>> 
>> On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:
>> 
>> > On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:
>> > > 
>> > > 
>> > > On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:
>> > > 
>> > > > From: Limin Wang <lance.lmwang@gmail.com>
>> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> > > > ---
>> > > > libavcodec/mpegvideo.c | 48 ++++++++++++++++++++++++------------------------
>> > > > 1 file changed, 24 insertions(+), 24 deletions(-)
>> > > 
>> > > If you find these cosmetics interesting, then I suggest you introduce a new
>> > > macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().
>> > > 
>> > > E.g.:
>> > > 
>> > > FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE, fail)
>> > 
>> > Yeah, I have considered it so I change the type to use use variable first and
>> > submit one typical for review. If the change is OK, then I'll go ahead next.
>> 
>> No need to do it in two steps, better touch the code once. E.g. patch 2
>> might not even be needed if you do this in a single patch.
>
> Sorry, now for array alloc, you had to input the one elemeay size and its count
> always. internal.h have defined it already:
> FF_ALLOCZ_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)
>
> You don't want to input elsize? isn't general usage?

I just found out that there is already an FF_ALLOCZ_ARRAY_OR_GOTO with 
elsize. So you should add a new macro which uses FF_ALLOCZ_ARRAY_OR_GOTO 
and calculates elszize automatically. I am not sure about the name:
FF_ALLOCZ_TYPED_ARRAY_OR_GOTO ?

Regards,
Marton

>
>> 
>> Regards,
>> Marton
>> 
>> > 
>> > > 
>> > > Regards,
>> > > Marton
>> > > 
>> > > > > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
>> > > > index 49fd1c9..561062f 100644
>> > > > --- a/libavcodec/mpegvideo.c
>> > > > +++ b/libavcodec/mpegvideo.c
>> > > > @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
>> > > > >     if (s->encoding) {
>> > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
>> > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
>> > > > +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
>> > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
>> > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
>> > > > +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
>> > > >         if (s->noise_reduction) {
>> > > >             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
>> > > > -                              2 * 64 * sizeof(int), fail)
>> > > > +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
>> > > >         }
>> > > >     }
>> > > > -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
>> > > > +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
>> > > >     s->block = s->blocks[0];
>> > > > >     for (i = 0; i < 12; i++) {
>> > > > @@ -400,7 +400,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);
>> > > > +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
>> > > >                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
>> > > > > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table,
>> > > mb_array_size * sizeof(int), fail)
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
>> > > > >         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
>> > > > -                         mb_array_size * sizeof(float), fail);
>> > > > +                         mb_array_size * sizeof(*s->cplx_tab), fail);
>> > > >         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
>> > > > -                         mb_array_size * sizeof(float), fail);
>> > > > +                         mb_array_size * sizeof(*s->bits_tab), fail);
>> > > > >     }
>> > > > > @@ -759,16 +759,16 @@ static int
>> > > init_context_frame(MpegEncContext *s)
>> > > >                 for (k = 0; k < 2; k++) {
>> > > >                     FF_ALLOCZ_OR_GOTO(s->avctx,
>> > > >                                       s->b_field_mv_table_base[i][j][k],
>> > > > -                                      mv_table_size * 2 * sizeof(int16_t),
>> > > > +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
>> > > >                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
>> > > > +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
>> > > >         }
>> > > >     }
>> > > >     if (s->out_format == FMT_H263) {
>> > > > @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
>> > > >         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
>> > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
>> > > > @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
>> > > >         return ret;
>> > > > >     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
>> > > > -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
>> > > > +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
>> > > >     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
>> > > >         s->picture[i].f = av_frame_alloc();
>> > > >         if (!s->picture[i].f)
>> > > > -- > 1.8.3.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 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".
>> > 
>> > -- 
>> > Thanks,
>> > Limin Wang
>> > _______________________________________________
>> > 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 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".
>
> -- 
> Thanks,
> Limin Wang
> _______________________________________________
> 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".
Limin Wang May 11, 2020, 1:24 p.m. UTC | #7
On Mon, May 11, 2020 at 09:16:39AM +0200, Marton Balint wrote:
> 
> 
> On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:
> 
> > On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Mon, 11 May 2020, lance.lmwang@gmail.com wrote:
> > > 
> > > > On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:
> > > > > > > > > On Sun, 10 May 2020, lance.lmwang@gmail.com wrote:
> > > > > > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > > > ---
> > > > > > libavcodec/mpegvideo.c | 48 ++++++++++++++++++++++++------------------------
> > > > > > 1 file changed, 24 insertions(+), 24 deletions(-)
> > > > > > > If you find these cosmetics interesting, then I suggest you
> > > introduce a new
> > > > > macro instead: FF_ALLOCZ_ARRAY_OR_GOTO().
> > > > > > > E.g.:
> > > > > > > FF_ALLOCZ_ARRAY_OR_GOTO(s->avctx, s->me.map, ME_MAP_SIZE,
> > > fail)
> > > > > Yeah, I have considered it so I change the type to use use
> > > variable first and
> > > > submit one typical for review. If the change is OK, then I'll go ahead next.
> > > 
> > > No need to do it in two steps, better touch the code once. E.g. patch 2
> > > might not even be needed if you do this in a single patch.
> > 
> > Sorry, now for array alloc, you had to input the one elemeay size and its count
> > always. internal.h have defined it already:
> > FF_ALLOCZ_ARRAY_OR_GOTO(ctx, p, nelem, elsize, label)
> > 
> > You don't want to input elsize? isn't general usage?
> 
> I just found out that there is already an FF_ALLOCZ_ARRAY_OR_GOTO with
> elsize. So you should add a new macro which uses FF_ALLOCZ_ARRAY_OR_GOTO and
> calculates elszize automatically. I am not sure about the name:
> FF_ALLOCZ_TYPED_ARRAY_OR_GOTO ?

OK, I'll add the new macro FF_ALLOCZ_TYPED_ARRAY_OR_GOTO.

> 
> Regards,
> Marton
> 
> > 
> > > 
> > > Regards,
> > > Marton
> > > 
> > > > > > > > Regards,
> > > > > Marton
> > > > > > > > > diff --git a/libavcodec/mpegvideo.c
> > > b/libavcodec/mpegvideo.c
> > > > > > index 49fd1c9..561062f 100644
> > > > > > --- a/libavcodec/mpegvideo.c
> > > > > > +++ b/libavcodec/mpegvideo.c
> > > > > > @@ -373,15 +373,15 @@ static int init_duplicate_context(MpegEncContext *s)
> > > > > > >     if (s->encoding) {
> > > > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
> > > > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > > > > > +                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
> > > > > >         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
> > > > > > -                          ME_MAP_SIZE * sizeof(uint32_t), fail)
> > > > > > +                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
> > > > > >         if (s->noise_reduction) {
> > > > > >             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
> > > > > > -                              2 * 64 * sizeof(int), fail)
> > > > > > +                              2 * 64 * sizeof(*s->dct_error_sum), fail)
> > > > > >         }
> > > > > >     }
> > > > > > -    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
> > > > > > +    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
> > > > > >     s->block = s->blocks[0];
> > > > > > >     for (i = 0; i < 12; i++) {
> > > > > > @@ -400,7 +400,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);
> > > > > > +                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
> > > > > >                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@ 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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
> > > > > > > -        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table,
> > > > > mb_array_size * sizeof(int), fail)
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
> > > > > > >         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
> > > > > > -                         mb_array_size * sizeof(float), fail);
> > > > > > +                         mb_array_size * sizeof(*s->cplx_tab), fail);
> > > > > >         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
> > > > > > -                         mb_array_size * sizeof(float), fail);
> > > > > > +                         mb_array_size * sizeof(*s->bits_tab), fail);
> > > > > > >     }
> > > > > > > @@ -759,16 +759,16 @@ static int
> > > > > init_context_frame(MpegEncContext *s)
> > > > > >                 for (k = 0; k < 2; k++) {
> > > > > >                     FF_ALLOCZ_OR_GOTO(s->avctx,
> > > > > >                                       s->b_field_mv_table_base[i][j][k],
> > > > > > -                                      mv_table_size * 2 * sizeof(int16_t),
> > > > > > +                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
> > > > > >                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
> > > > > > +                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
> > > > > >         }
> > > > > >     }
> > > > > >     if (s->out_format == FMT_H263) {
> > > > > > @@ -777,14 +777,14 @@ static int init_context_frame(MpegEncContext *s)
> > > > > >         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
> > > > > > +        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
> > > > > > @@ -935,7 +935,7 @@ av_cold int ff_mpv_common_init(MpegEncContext *s)
> > > > > >         return ret;
> > > > > > >     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
> > > > > > -                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
> > > > > > +                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
> > > > > >     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
> > > > > >         s->picture[i].f = av_frame_alloc();
> > > > > >         if (!s->picture[i].f)
> > > > > > -- > 1.8.3.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 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".
> > > > > -- > Thanks,
> > > > Limin Wang
> > > > _______________________________________________
> > > > 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 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".
> > 
> > -- 
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > 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 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..561062f 100644
--- a/libavcodec/mpegvideo.c
+++ b/libavcodec/mpegvideo.c
@@ -373,15 +373,15 @@  static int init_duplicate_context(MpegEncContext *s)
 
     if (s->encoding) {
         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.map,
-                          ME_MAP_SIZE * sizeof(uint32_t), fail)
+                          ME_MAP_SIZE * sizeof(*s->me.map), fail)
         FF_ALLOCZ_OR_GOTO(s->avctx, s->me.score_map,
-                          ME_MAP_SIZE * sizeof(uint32_t), fail)
+                          ME_MAP_SIZE * sizeof(*s->me.score_map), fail)
         if (s->noise_reduction) {
             FF_ALLOCZ_OR_GOTO(s->avctx, s->dct_error_sum,
-                              2 * 64 * sizeof(int), fail)
+                              2 * 64 * sizeof(*s->dct_error_sum), fail)
         }
     }
-    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(int16_t), fail)
+    FF_ALLOCZ_OR_GOTO(s->avctx, s->blocks, 64 * 12 * 2 * sizeof(*s->blocks), fail)
     s->block = s->blocks[0];
 
     for (i = 0; i < 12; i++) {
@@ -400,7 +400,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);
+                          yc_size * sizeof(*s->ac_val_base) * 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 +715,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_OR_GOTO(s->avctx, s->mb_index2xy, (s->mb_num + 1) * sizeof(*s->mb_index2xy),
                       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 +725,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_OR_GOTO(s->avctx, s->p_mv_table_base,                 mv_table_size * 2 * sizeof(*s->p_mv_table_base), fail)
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_forw_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_forw_mv_table_base), fail)
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_back_mv_table_base,            mv_table_size * 2 * sizeof(*s->b_back_mv_table_base), fail)
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_forw_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_forw_mv_table_base), fail)
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_bidir_back_mv_table_base,      mv_table_size * 2 * sizeof(*s->b_bidir_back_mv_table_base), fail)
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->b_direct_mv_table_base,          mv_table_size * 2 * sizeof(*s->b_direct_mv_table_base), 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 +739,14 @@  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_OR_GOTO(s->avctx, s->mb_type, mb_array_size * sizeof(*s->mb_type), fail) // needed for encoding
 
-        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(int), fail)
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->lambda_table, mb_array_size * sizeof(*s->lambda_table), fail)
 
         FF_ALLOC_OR_GOTO(s->avctx, s->cplx_tab,
-                         mb_array_size * sizeof(float), fail);
+                         mb_array_size * sizeof(*s->cplx_tab), fail);
         FF_ALLOC_OR_GOTO(s->avctx, s->bits_tab,
-                         mb_array_size * sizeof(float), fail);
+                         mb_array_size * sizeof(*s->bits_tab), fail);
 
     }
 
@@ -759,16 +759,16 @@  static int init_context_frame(MpegEncContext *s)
                 for (k = 0; k < 2; k++) {
                     FF_ALLOCZ_OR_GOTO(s->avctx,
                                       s->b_field_mv_table_base[i][j][k],
-                                      mv_table_size * 2 * sizeof(int16_t),
+                                      mv_table_size * 2 * sizeof(*s->b_field_mv_table_base[i][j][k]),
                                       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_OR_GOTO(s->avctx, s->b_field_select_table [i][j], mb_array_size * 2 * sizeof(*s->b_field_select_table [i][j]), fail)
+                FF_ALLOCZ_OR_GOTO(s->avctx, s->p_field_mv_table_base[i][j], mv_table_size * 2 * sizeof(*s->p_field_mv_table_base[i][j]), 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_OR_GOTO(s->avctx, s->p_field_select_table[i], mb_array_size * 2 * sizeof(*s->p_field_select_table[i]), fail)
         }
     }
     if (s->out_format == FMT_H263) {
@@ -777,14 +777,14 @@  static int init_context_frame(MpegEncContext *s)
         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_OR_GOTO(s->avctx, s->cbp_table     , mb_array_size * sizeof(*s->cbp_table), fail);
+        FF_ALLOCZ_OR_GOTO(s->avctx, s->pred_dir_table, mb_array_size * sizeof(*s->pred_dir_table), 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_OR_GOTO(s->avctx, s->dc_val_base, yc_size * sizeof(*s->dc_val_base), 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;
@@ -935,7 +935,7 @@  av_cold int ff_mpv_common_init(MpegEncContext *s)
         return ret;
 
     FF_ALLOCZ_OR_GOTO(s->avctx, s->picture,
-                      MAX_PICTURE_COUNT * sizeof(Picture), fail_nomem);
+                      MAX_PICTURE_COUNT * sizeof(*s->picture), fail_nomem);
     for (i = 0; i < MAX_PICTURE_COUNT; i++) {
         s->picture[i].f = av_frame_alloc();
         if (!s->picture[i].f)