[FFmpeg-devel] [PATCH 01/18] mpeg4videodec: do not copy a range of fields at once

[James Almer]

On 3/13/2020 7:28 AM, Anton Khirnov wrote:
> This is extremely fragile against reordering and hides what is actually
> being copied. Copy all the fields manually instead.
> ---
>  libavcodec/mpeg4videodec.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> index cc03486646..a51985d51b 100644
> --- a/libavcodec/mpeg4videodec.c
> +++ b/libavcodec/mpeg4videodec.c
> @@ -3460,7 +3460,32 @@ static int mpeg4_update_thread_context(AVCodecContext *dst,
>      if (ret < 0)
>          return ret;
>  
> -    memcpy(((uint8_t*)s) + sizeof(MpegEncContext), ((uint8_t*)s1) + sizeof(MpegEncContext), sizeof(Mpeg4DecContext) - sizeof(MpegEncContext));
> +    s->time_increment_bits       = s1->time_increment_bits;
> +    s->shape                     = s1->shape;
> +    s->vol_sprite_usage          = s1->vol_sprite_usage;
> +    s->sprite_brightness_change  = s1->sprite_brightness_change;
> +    s->num_sprite_warping_points = s1->num_sprite_warping_points;
> +    s->rvlc                      = s1->rvlc;
> +    s->resync_marker             = s1->resync_marker;
> +    s->t_frame                   = s1->t_frame;
> +    s->new_pred                  = s1->new_pred;
> +    s->enhancement_type          = s1->enhancement_type;
> +    s->scalability               = s1->scalability;
> +    s->use_intra_dc_vlc          = s1->use_intra_dc_vlc;
> +    s->intra_dc_threshold        = s1->intra_dc_threshold;
> +    s->divx_version              = s1->divx_version;
> +    s->divx_build                = s1->divx_build;
> +    s->xvid_build                = s1->xvid_build;
> +    s->lavc_build                = s1->lavc_build;
> +    s->showed_packed_warning     = s1->showed_packed_warning;
> +    s->vol_control_parameters    = s1->vol_control_parameters;
> +    s->cplx_estimation_trash_i   = s1->cplx_estimation_trash_i;
> +    s->cplx_estimation_trash_p   = s1->cplx_estimation_trash_p;
> +    s->cplx_estimation_trash_b   = s1->cplx_estimation_trash_b;
> +    s->rgb                       = s1->rgb;
> +
> +    memcpy(s->sprite_shift, s1->sprite_shift, sizeof(s1->sprite_shift));
> +    memcpy(s->sprite_traj,  s1->sprite_traj,  sizeof(s1->sprite_traj));

IMO, add a comment about this, in case someone unaware in the future
sees this block of code and feels clever after figuring it could be done
with a single memcpy().

>  
>      if (CONFIG_MPEG4_DECODER && !init && s1->xvid_build >= 0)
>          ff_xvid_idct_init(&s->m.idsp, dst);
>