[FFmpeg-devel] [PATCH] IFF demuxer and 8SVX decoder

[Reimar Döffinger]

On Wed, Mar 26, 2008 at 11:43:28AM +0000, Jai Menon wrote:
> +/**
> + *
> + * decode a frame
> + *
> + */
> +static int eightsvx_decode_frame(AVCodecContext *avctx, void *data, int *data_size, const uint8_t *buf, int buf_size)
> +{
> +    EightSvxContext *esc = avctx->priv_data;
> +    int8_t *in_data = buf;
> +    int16_t *out_data = data;
> +    int i;
> +    int8_t d;
> +
> +    if(!buf_size)
> +        return 0;
> +
> +    *data_size = 0;

data_size contains the size of the data buffer, you should not just
ignore that.

> +    if(esc->first_frame)
> +    {
> +        esc->fib_acc = in_data[1];
> +        in_data++;
> +    }
> +
> +    for(i = 0;i < buf_size - (esc->first_frame << 1);i++)

You already have a esc->first_frame check above, handle it there.
Also, decrementing buf_size (keeping a backup or whatever for teh return
value) and checking against 0 will be faster.
And you will not need the i variable at all if you instead compare
in_data against e.g. a in_end pointer.

> +    {
> +        d = *in_data;
> +        in_data++;

IMO should be
d = *in_data++;

> +        esc->fib_acc += table[(uint8_t)d & 0x0f];

Use the right type instead of casting.

> +        CLIP8(esc->fib_acc);
> +        *out_data = esc->fib_acc << 8;
> +        out_data++;

*out_data++ = av_clip_uint8(esc->fib_acc) << 8;

> +        esc->fib_acc += table[(uint8_t)d >> 4];
> +        CLIP8(esc->fib_acc);
> +        *out_data = esc->fib_acc << 8;
> +        out_data++;
> +        *data_size = *data_size + 4;

There is no reason to do this in the inner loop!

> +        case CODEC_ID_8SVX_FIB:
> +          table = &fibonacci[0];

That's obfuscated, just table = fibonacci;