On Mon, 11 May 2020, lance.lmw...@gmail.com wrote:

On Sun, May 10, 2020 at 06:30:41PM +0200, Marton Balint wrote:


On Sun, 10 May 2020, lance.lmw...@gmail.com wrote:

> From: Limin Wang <lance.lmw...@gmail.com>
> > Signed-off-by: Limin Wang <lance.lmw...@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-requ...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to