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

[Michael Niedermayer]

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?


> though the other decoders add it

Ill look into that later ...


> and the maxis xa wiki page warns that some compilers need it.

Why could some compilers need it?


[...]
> @@ -666,9 +667,15 @@
>  static av_cold int adpcm_decode_init(AVCodecContext * avctx)
>  {
>      ADPCMContext *c = avctx->priv_data;
> -    unsigned int max_channels = 2;
> +    unsigned int max_channels = 2, channel;
> 
>      switch(avctx->codec->id) {
> +    case CODEC_ID_ADPCM_EA_MAXIS_XA:
> +        for(channel = 0; channel < avctx->channels; channel++) {
> +                c->status[channel].sample1 = 0;
> +                c->status[channel].sample2 = 0;
> +        }
> +        break;

This shouldnt be needed.


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


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


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- 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/469219d3/attachment.pgp>