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

[Michael Niedermayer]

On Sun, Mar 30, 2008 at 11:00:16AM +0000, Jai Menon wrote:
> On Sunday 30 March 2008 10:43:16 Jai Menon wrote:
> > On Saturday 29 March 2008 14:46:01 Michael Niedermayer wrote:
> > > On Sat, Mar 29, 2008 at 11:13:18AM +0000, Jai Menon wrote:
> > > > 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.
> > >
> > > 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.
> >
> > > > +#include <stdio.h>
> > > > +#include <stdlib.h>
> > >
> > > are all these includes needed?
> >
> > These must have been there in the existing codec/demuxer i used as
> > a "template". Sorry i didn't remove these earlier.
> >
> > whew!! day 7 rolls on.........
> 
> sorry.......the demuxer patch is an old one. new one attached below

[...]
> +static int iff_probe(AVProbeData *p)
> +{
> +    const uint8_t *d = p->buf;
> +
> +    if (AV_RL32(d) == ID_FORM
> +        && AV_RL32(d+8) == ID_8SVX)

if (   AV_RL32(d  ) == ID_FORM
    && AV_RL32(d+8) == ID_8SVX)

or
if (AV_RL32(d  ) == ID_FORM &&
    AV_RL32(d+8) == ID_8SVX)

look better IMHO


[...]

> +static int iff_read_packet(AVFormatContext *s,
> +                           AVPacket *pkt)
> +{
> +    IffDemuxContext *iff = s->priv_data;
> +    ByteIOContext *pb = s->pb;
> +    int ret;
> +
> +    if(iff->sent_bytes > iff->body_size)
> +        return AVERROR(EIO);
> +    ret = av_get_packet(pb, pkt, PACKET_SIZE);
> +    iff->sent_bytes += PACKET_SIZE;
> +
> +    pkt->stream_index = 0;
> +    pkt->pts = iff->audio_frame_count;
> +    iff->audio_frame_count += ret / s->streams[0]->codec->channels;
> +    return ret;
> +}

the first packet should have PKT_FLAG_KEY


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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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/538e4159/attachment.pgp>