[FFmpeg-devel] [PATCH 2/7] avcodec/vc1dec: Postpone allocating sprite frame to avoid segfault

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Up until now, the VC-1 decoders allocated an AVFrame for usage with
> sprites during vc1_decode_init(); yet said AVFrame can be freed if
> (re)initializing the context (which happens ordinarily during decoding)
> fails. The AVFrame does not get allocated again lateron in this case,
> leading to segfaults.
> 
> Fix this by moving the allocation of said frame immediately before it is
> used (this also means that said frame won't be allocated at all any more
> in case of a regular (i.e. non-image) stream).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavcodec/vc1dec.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/vc1dec.c b/libavcodec/vc1dec.c
> index 7809234ff7..5cdf197da7 100644
> --- a/libavcodec/vc1dec.c
> +++ b/libavcodec/vc1dec.c
> @@ -539,12 +539,6 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
>      ff_h264chroma_init(&v->h264chroma, 8);
>      ff_qpeldsp_init(&s->qdsp);
>  
> -    // Must happen after calling ff_vc1_decode_end
> -    // to avoid de-allocating the sprite_output_frame
> -    v->sprite_output_frame = av_frame_alloc();
> -    if (!v->sprite_output_frame)
> -        return AVERROR(ENOMEM);
> -
>      avctx->has_b_frames = !!avctx->max_b_frames;
>  
>      if (v->color_prim == 1 || v->color_prim == 5 || v->color_prim == 6)
> @@ -577,20 +571,15 @@ static av_cold int vc1_decode_init(AVCodecContext *avctx)
>              v->sprite_height > 1 << 14 ||
>              v->output_width  > 1 << 14 ||
>              v->output_height > 1 << 14) {
> -            ret = AVERROR_INVALIDDATA;
> -            goto error;
> +            return AVERROR_INVALIDDATA;
>          }
>  
>          if ((v->sprite_width&1) || (v->sprite_height&1)) {
>              avpriv_request_sample(avctx, "odd sprites support");
> -            ret = AVERROR_PATCHWELCOME;
> -            goto error;
> +            return AVERROR_PATCHWELCOME;
>          }
>      }
>      return 0;
> -error:
> -    av_frame_free(&v->sprite_output_frame);
> -    return ret;
>  }
>  
>  /** Close a VC1/WMV3 decoder
> @@ -1147,6 +1136,11 @@ image:
>          avctx->height = avctx->coded_height = v->output_height;
>          if (avctx->skip_frame >= AVDISCARD_NONREF)
>              goto end;
> +        if (!v->sprite_output_frame &&
> +            !(v->sprite_output_frame = av_frame_alloc())) {
> +            ret = AVERROR(ENOMEM);
> +            goto err;
> +        }
>  #if CONFIG_WMV3IMAGE_DECODER || CONFIG_VC1IMAGE_DECODER
>          if ((ret = vc1_decode_sprites(v, &s->gb)) < 0)
>              goto err;
> 
Will apply tonight unless there are objections.

- Andreas