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

[Jai Menon]

On Friday 28 March 2008 18:29:35 Michael Niedermayer wrote:
> On Fri, Mar 28, 2008 at 09:21:36PM +0000, Jai Menon wrote:
> > On Thursday 27 March 2008 20:00:48 Michael Niedermayer wrote:
> > > On Thu, Mar 27, 2008 at 11:48:10PM +0000, Jai Menon wrote:
> > > > On Thursday 27 March 2008 15:23:54 Michael Niedermayer wrote:
> > > > > On Thu, Mar 27, 2008 at 08:44:54PM +0000, Jai Menon wrote:
> > > > > > On Wednesday 26 March 2008 21:12:26 Michael Niedermayer wrote:
> > > > > > > uint8_t d = *buf++;
> > > > > > >
> > > > > > > > +        esc->fib_acc += esc->table[d & 0x0f];
> > > > > > > > +        *out_data++ = esc->fib_acc << 8;
> > > > > > > > +        esc->fib_acc += esc->table[d >> 4];
> > > > > > > > +        *out_data++ = esc->fib_acc << 8;
> > > > > > > > +    }
> > > > > > >
> > > > > > > you can do this with one subtraction and 2 shifts less
> > > > > >
> > > > > > I still don't know how i can eliminate the two shifts?
> > > > >
> > > > > change the table ...
> > > >
> > > > I could change it to int16_t, and remove the 2 shifts.....but then i
> > > > would need to clip twice before adding the table value to the
> > > > accumulator......in which case imho we should stick to 2 shifts.
> > >
> > > Why would you need to clip?
> >
> >  Thats because the encoding scheme requires adding an 8 bit signed value
> > (from the table) to fib_acc. So if we change the table size to int16_t ,
> > we could do away with the shifts but the value can't be directly added to
> > fib_acc without clipping.
>
> Could you give me an example with numbers where a single value ending
> up in out_data differs?

I'll phrase this in a different way. After the decoding, the fib_acc variable 
stores the signed pcm data. The shifts are required because the decoder's 
output is supposed to be 16 bit. If i convert the table to use 16 bit data 
instead and also make fib_acc an int16_t, i'll need to extract just the 
lsbyte since the msbyte is not part of the sample information.
Otherwise, i could use a couple of casts to ensure that the msbyte remains 
0x00, but that wouldn't be too clean.

> > the av_clip macro iirc uses comparisons. you could actually
> > try making the change and running it on the compressed sample Dennis
> > posted earlier on the list. Instead of the sound sample, you will get a
> > highly distorted waveform which sounds nothing like the original sample.
> >
> > > + ? ?for(;buf_size>0;buf_size--) {
> > > + ? ? ? ?uint8_t d = *buf++;
> > > + ? ? ? ?esc->fib_acc += esc->table[d & 0x0f];
> > > + ? ? ? ?*out_data++ = esc->fib_acc << 8;
> > > + ? ? ? ?esc->fib_acc += esc->table[d >> 4];
> > > + ? ? ? ?*out_data++ = esc->fib_acc << 8;
> > > + ? ?}
> > >
> > >  this still does a unneeded subtraction besides the shifts
> >
> > where? is it buf_size-- ?
>
> yes buf_size-- is a unneeded subtraction of 1
I do this check in a different way now.....is this what you intended?

patch attached

Regards,
Jai Menon
<realityman at gmx.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 8svx_decoder.patch
Type: text/x-diff
Size: 5132 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080329/053826c1/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iff_demuxer.patch
Type: text/x-diff
Size: 6392 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080329/053826c1/attachment-0001.patch>