[FFmpeg-devel] [PATCH] svq1dec: convert VLA to fixed size

[Måns Rullgård]

Christophe Gisquet <christophe.gisquet at gmail.com> writes:

> 2010/6/24 Mans Rullgard <mans at mansr.com>:
>> +#define SVQ1_MAX_WIDTH 4095
>> +
>> ?extern const uint8_t mvtab[33][2];
>>
>> ?static VLC svq1_block_type;
>> @@ -730,7 +732,7 @@ static int svq1_decode_frame(AVCodecContext *avctx,
>> ? ? ? ? current += 16*linesize;
>> ? ? ? }
>> ? ? } else {
>> - ? ? ?svq1_pmv pmv[width/8+3];
>> + ? ? ?svq1_pmv pmv[SVQ1_MAX_WIDTH/8+3];
>> ? ? ? /* delta frame */
>> ? ? ? memset (pmv, 0, ((width / 8) + 3) * sizeof(svq1_pmv));
>
> Sorry for the naive question, but svq1_pmv being a struct with 2 ints,
> isn't this going to create a local variable of around 4K at least on
> the stack?

It already does that for large inputs, so the patch changes nothing.

> From the mvtab content, it seems pmv could use int16_t instead. Then
> the attached patch might be more worthwhile.

Separate discussion.

> @@ -730,7 +730,7 @@
>          current += 16*linesize;
>        }
>      } else {
> -      svq1_pmv pmv[width/8+3];
> +      svq1_pmv *pmv = av_malloc((width/8+3)*sizeof(*pmv));
>        /* delta frame */
>        memset (pmv, 0, ((width / 8) + 3) * sizeof(svq1_pmv));
>  
> @@ -752,6 +752,8 @@
>  
>          current += 16*linesize;
>        }
> +
> +      av_free(pmv);
>      }
>    }

Where's the error checking?  This is also a silly place to do the
malloc/free.  Better to do it per frame instead of per plane.

-- 
M?ns Rullg?rd
mans at mansr.com