[FFmpeg-soc] [Patch] Maxis EA XA decoder - GSoC Task

[Michael Niedermayer]

On Thu, Apr 10, 2008 at 02:34:21PM +0200, Robert Marston wrote:
> On Thu, Apr 10, 2008 at 1:43 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Thu, Apr 10, 2008 at 12:56:11PM +0200, Robert Marston wrote:
> >
> > > On Wed, Apr 9, 2008 at 7:50 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >  > > On Wed, Apr 09, 2008 at 04:28:50PM +0200, Robert Marston wrote:
> >  > >  > Attached patch has the following changes to it.
> >  [...]
> >
> >  > >
> >  > >  [...]
> >
> > > >  > @@ -1235,6 +1242,30 @@
> >  > >
> >  > > >              }
> >  > >  >          }
> >  > >  >          break;
> >  > >  > +    case CODEC_ID_ADPCM_EA_MAXIS_XA:
> >  > >
> >  > >  > +        for(channel = 0; channel < avctx->channels; channel++) {
> >  > >  > +              for (i=0; i<2; i++)
> >  > >  > +                    coeff[channel][i] = ea_adpcm_table[((*src >> 4) & 0x0F)+(4*i)];
> >  > >
> >  > >  one of the operations in there does nothing
> >  > >
> >  > I know the shift may not be needed,
> >
> >  Well, then could you just show how you would write it without the shift?
> >
> 
> I removed the shift the with the latest attempted.

You removed the & with the last 2 attempts, but where claiming the >> may not
be needed. Now i certainly did mean the & when i spoke of a "one of the 
operations in there does nothing". I was just a little curious what you where
talking about ...


[...]
> 
> >
> >  [...]
> >
> >  > @@ -1235,6 +1243,29 @@
> >
> > >              }
> >  >          }
> >  >          break;
> >  > +    case CODEC_ID_ADPCM_EA_MAXIS_XA:
> >  > +        for(channel = 0; channel < avctx->channels; channel++) {
> >  > +            for (i=0; i<2; i++)
> >  > +                coeff[channel][i] = ea_adpcm_table[(*src >> 4) +(4*i)];
> >
> > > +            shift[channel] = (*src & 0x0F) + 8;
> >  > +            src++;
> >  > +        }
> >
> > > +        for (count1 = 0; count1 < ((buf_size - avctx->channels) / avctx->channels) ; count1++) {
> >  > +            for(i = 4; i >= 0; i-=4) { /* Pairwise samples LL RR (st) or LL LL (mono) */
> >  > +                int32_t sample;
> >
> >
> > > +                for(channel = 0; channel < avctx->channels; channel++) {
> >
> > > +                    sample = ((((*(src+channel) >> i) & 0x0F) << 0x1C) >> shift[channel]);
> >
> >  This looks buggy.
> >
> 
> Where do you think the error would occur?

on some non x86 hardware


> 
> >
> >  [...]
> >  > +static int xa_read_packet(AVFormatContext *s,
> >  > +                          AVPacket *pkt)
> >  > +{
> >  > +    MaxisXADemuxContext *xa = s->priv_data;
> >  > +    AVStream *st = s->streams[0];
> >  > +    ByteIOContext *pb = s->pb;
> >  > +    unsigned int packet_size;
> >  > +    int ret = 0;
> >  > +
> >  > +    if(xa->sent_bytes > xa->out_size)
> >  > +        return AVERROR(EIO);
> >  > +    /* 1 byte header and 14 bytes worth of samples * number channels per block */
> >  > +    packet_size = 15*st->codec->channels;
> >  > +
> >  > +    ret = av_get_packet(pb, pkt, packet_size);
> >  > +    pkt->stream_index = st->index;
> >  > +
> >  > +    xa->sent_bytes += packet_size;
> >
> >
> >  > +    pkt->pts = 90000;
> >  > +    pkt->pts *= xa->audio_frame_counter;
> >  > +    pkt->pts /= st->codec->sample_rate;
> >  > +    /* 14 Samples per channel  */
> >  > +    xa->audio_frame_counter += 14;
> >
> >  Still wrong
> >
> > [...]
> 
> Corrected?

no

[...]
> @@ -1235,6 +1237,29 @@
>              }
>          }
>          break;
> +    case CODEC_ID_ADPCM_EA_MAXIS_XA:
> +        for(channel = 0; channel < avctx->channels; channel++) {
> +            for (i=0; i<2; i++)
> +                coeff[channel][i] = ea_adpcm_table[(*src >> 4) +(4*i)];

superflous ()


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- 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-soc/attachments/20080410/f93e78f8/attachment.pgp>