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

[Robert Marston]

On Thu, Apr 10, 2008 at 4:03 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>
>  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 ...
>

Yes, sorry not sure why I went on about the shift there. I meant to
say the & 0x0F. I noticed some of the other decoders have it and the
original overview of the format mentioned that some compilers might
required it.

>
>  [...]
>
> >
>  > >
>  > >  [...]
>  > >
>  > >  > @@ -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
>
>

What exactly you referring to here? The shift operators? A problem
with the Endianess maybe?

>
>  >
>  > >
>  > >  [...]
>  > >  > +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
>

Am I right in saying the pts should be incremented by 28 *
90K/(1/sample_rate) * 90K ... assuming we use a 90 KHz clock? and 28
being the number of samples per channel.

> [...]

Thanks
Robert