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

[Robert Marston]

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.

>
>[...]
>
>  > and the maxis xa wiki page warns that some compilers need it.
>
>  Why could some compilers need it?
>

Not sure, most architectures zero fill but I guess there is odd chance
one might not.

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

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

Thanks
Robert
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: maxis_ea_xa_format.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20080410/78973a54/attachment.txt>