[FFmpeg-devel] [PATCH] tta: replace potentially huge VLAs with malloc/free in context

[Jai Menon]

On Thu, Jun 24, 2010 at 10:23 PM, Mans Rullgard <mans at mansr.com> wrote:
> ---
> ?libavcodec/tta.c | ? 51 ++++++++++++++++++++++++++++++---------------------
> ?1 files changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/libavcodec/tta.c b/libavcodec/tta.c
> index 4bdfd73..9014553 100644
> --- a/libavcodec/tta.c
> +++ b/libavcodec/tta.c
> @@ -36,6 +36,24 @@
> ?#define FORMAT_INT 1
> ?#define FORMAT_FLOAT 3
>
> +#define MAX_ORDER 16
> +typedef struct TTAFilter {
> + ? ?int32_t shift, round, error, mode;
> + ? ?int32_t qm[MAX_ORDER];
> + ? ?int32_t dx[MAX_ORDER];
> + ? ?int32_t dl[MAX_ORDER];
> +} TTAFilter;
> +
> +typedef struct TTARice {
> + ? ?uint32_t k0, k1, sum0, sum1;
> +} TTARice;
> +
> +typedef struct TTAChannel {
> + ? ?int32_t predictor;
> + ? ?TTAFilter filter;
> + ? ?TTARice rice;
> +} TTAChannel;
> +
> ?typedef struct TTAContext {
> ? ? AVCodecContext *avctx;
> ? ? GetBitContext gb;
> @@ -44,6 +62,8 @@ typedef struct TTAContext {
> ? ? int frame_length, last_frame_length, total_frames;
>
> ? ? int32_t *decode_buffer;
> +
> + ? ?TTAChannel *ch_ctx;;
> ?} TTAContext;
>
> ?#if 0
> @@ -79,14 +99,6 @@ static const uint32_t shift_1[] = {
> ?static const uint32_t * const shift_16 = shift_1 + 4;
> ?#endif
>
> -#define MAX_ORDER 16
> -typedef struct TTAFilter {
> - ? ?int32_t shift, round, error, mode;
> - ? ?int32_t qm[MAX_ORDER];
> - ? ?int32_t dx[MAX_ORDER];
> - ? ?int32_t dl[MAX_ORDER];
> -} TTAFilter;
> -
> ?static const int32_t ttafilter_configs[4][2] = {
> ? ? {10, 1},
> ? ? {9, 1},
> @@ -175,10 +187,6 @@ static inline void ttafilter_process(TTAFilter *c, int32_t *in, int32_t mode) {
> ? ? memshl(c->dx, c->dx + 1);
> ?}
>
> -typedef struct TTARice {
> - ? ?uint32_t k0, k1, sum0, sum1;
> -} TTARice;
> -
> ?static void rice_init(TTARice *c, uint32_t k0, uint32_t k1)
> ?{
> ? ? c->k0 = k0;
> @@ -277,6 +285,9 @@ static av_cold int tta_decode_init(AVCodecContext * avctx)
> ? ? ? ? }
>
> ? ? ? ? s->decode_buffer = av_mallocz(sizeof(int32_t)*s->frame_length*s->channels);
> + ? ? ? ?s->ch_ctx = av_malloc(avctx->channels * sizeof(*s->ch_ctx));
> + ? ? ? ?if (!s->ch_ctx)
> + ? ? ? ? ? ?return AVERROR(ENOMEM);

Based on most real world use cases, I'd even suggest restricting the
maximum number of channels to 8 and allocating statically (almost all
usage in the wild is stereo anyway). But I dunno if imposing an
artificial limit (which could be increased if users complain) like
this is okay. So patch by itself should be fine.

-- 
Jai Menon