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

[Michael Niedermayer]

On Sun, Mar 30, 2008 at 11:43:32AM +0000, Jai Menon wrote:
> On Sunday 30 March 2008 11:26:16 Jai Menon wrote:
> > On Sunday 30 March 2008 05:40:03 Uoti Urpala wrote:
> > > On Sun, 2008-03-30 at 10:43 +0000, Jai Menon wrote:
> > > > On Saturday 29 March 2008 14:46:01 Michael Niedermayer wrote:
> > > > > Currently the code does practically:
> > > > >
> > > > > while(not end){
> > > > >     acc += constant_table[*x++];
> > > > >     *out++= acc<<8;
> > > > > }
> > > > >
> > > > > The code can be changed so it does not need the "<<8" while keeping
> > > > > the output the same 16bit data.
> > > >
> > > > I have removed the shifts. The changes should clarify some stuff on why
> > > > the previous code was like that.
> > >
> > > But you added a bswap in the loop instead of changing the table. I don't
> > > think that is what was intended...
> >
> > exactly....neither do i !!! i just wanted to write the decoding step in a
> > different way to show why i prefer the previous method. But i'm still
> > experimenting .......
> 
> Another interpretation........

[...]
> +const static int16_t fibonacci[16]   = { -8704, -5376, -3328, -2048, -1280, -768, -512, -256,
> +                                        0, 256, 512, 768, 1280, 2048, 3328, 5376 };
> +const static int16_t exponential[16] = { -32768, -16384, -8192, -4096, -2048, -1024, -512, -256,
> +                                        0, 256, 512, 1024, 2048, 4096, 8192, 16384};

These could be written like:
const static int16_t fibonacci[16]   = {-34<<8, -21<<8, -13<<8, ...

This is no problem as the compiler does these shifts during compilation, thus
no unneeded code nor time is wasted during execution.


> +
> +/**
> + * 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;
> +    int16_t *out_data = data;
> +    int consumed = buf_size;
> +    const uint8_t *buf_end = buf + buf_size;
> +
> +    if((*data_size >> 2) < buf_size)
> +        return -1;
> +
> +    if(avctx->frame_number == 0) {

> +        esc->fib_acc = bswap_16(buf[1]);

That can use a shift, my goal was to remove unneeded operations in the more
speed critical parts of the code, not to remove shifts "just because of it".


> +        buf_size -= 2;
> +        buf += 2;
> +    }
> +
> +    *data_size = buf_size << 2;
> +

> +    while(buf < buf_end) {
> +        uint8_t d = *buf++;
> +        esc->fib_acc += esc->table[d & 0x0f];
> +        *out_data++ = esc->fib_acc;
> +        esc->fib_acc += esc->table[d >> 4];
> +        *out_data++ = esc->fib_acc;
> +    }

good!

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080330/7936a102/attachment.pgp>