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

2020-05-11 Thread lance . lmwang
On Mon, May 11, 2020 at 09:16:39AM +0200, Marton Balint wrote:
> 
> 
> On Mon, 11 May 2020, lance.lmw...@gmail.com wrote:
> 
> > On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
> > > 
> > > 
> > > 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 
> > > > > > > Signed-off-by: Limin Wang 
> > > > > > ---
> > > > > > 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, 

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

2020-05-11 Thread Marton Balint



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


On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:



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 

> > > > Signed-off-by: Limin Wang 
> > > ---
> > > 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,  

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

2020-05-10 Thread lance . lmwang
On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
> 
> 
> 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 
> > > > > Signed-off-by: Limin Wang 
> > > > ---
> > > > 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 * 

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

2020-05-10 Thread lance . lmwang
On Mon, May 11, 2020 at 01:22:24AM +0200, Marton Balint wrote:
> 
> 
> 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 
> > > > > Signed-off-by: Limin Wang 
> > > > ---
> > > > 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)
> > > > + 

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

2020-05-10 Thread Marton Balint



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 
> 
> Signed-off-by: Limin Wang 

> ---
> 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 

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

2020-05-10 Thread lance . lmwang
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 
> > 
> > Signed-off-by: Limin Wang 
> > ---
> > 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 

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

2020-05-10 Thread Marton Balint



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


From: Limin Wang 

Signed-off-by: Limin Wang 
---
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)

 

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

2020-05-10 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavcodec/mpegvideo.c | 48 
 1 file changed, 24 insertions(+), 24 deletions(-)

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,