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

[Robert Marston]

On Tue, Apr 8, 2008 at 8:11 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Tue, Apr 08, 2008 at 04:43:25PM +0200, Robert Marston wrote:
>  [...]
>
>
>
>
>  >
>  >
>  > >
>  > >
>  > > [...]
>  > > > static int xa_read_header(AVFormatContext *s,
>  > > >                           AVFormatParameters *ap)
>  > > > {
>  > > >     MaxisXADemuxContext *xa = s->priv_data;
>  > > >     AVStream *st;
>  > > >
>  > > >     /* Extract Header Information from xa file */
>  > > >     if (!xa_extract_header_info(s))
>  > > >         return AVERROR(EIO);
>  > > >
>  > > >     /*Set up the XA Audio Decoder*/
>  > > >     st = av_new_stream(s, 0);
>  > > >     if (!st)
>  > > >         return AVERROR(ENOMEM);
>  > >
>  > > >     av_set_pts_info(st, 33, 1, xa->sampleRate);
>  > >
>  > > i doubt 33 is correct
>  >
>  >
>  > I am not entirely sure what the wrap bits is and what its value should be, I
>  > copied that from the original ADPCM EA in  electronicarts.c
>
>  Its the number of bits in the pts, mpeg stores them in 33 bits people
>  copied that all over the place without undestanding it ...
>

Set this to 64 then?

>
>  [...]
>
> > >
>  > >
>  > > >     if (ret != packet_size) {
>  > > >         av_log(s, AV_LOG_WARNING, "Size mismatch");
>  > > >         return AVERROR(EIO);
>  > > >     }
>  > >
>  > > memleak
>  > >
>  >
>  > Added a av_free for pkt here. The test files seem to have an extra few
>  > samples but according to the description the samples are contained in blocks
>  > of 14 bytes for each channel and so I am not sure why there are these extra
>  > samples in the file.
>
>  Probably because the source did not contain a multiple of 28 samples, it
>  would be nice if tha last incomplete packet would be decoded as well.
>  Assuming iam guessing correct that it is actual audio at all instead of
>  junk.
>

Will check this.

>
>  [...]
>  >  typedef struct ADPCMContext {
>  >      int channel; /* for stereo MOVs, decode left, then decode right, then tell it's decoded */
>  >      ADPCMChannelStatus status[6];
>  > +    int32_t mxa_current_left_sample; /*Needed to keep track of left and right samples for Maxis EA XA */
>  > +    int32_t mxa_previous_left_sample;
>  > +    int32_t mxa_current_right_sample;
>  > +    int32_t mxa_previous_right_sample;
>  >  } ADPCMContext;
>  >
>  >  /* XXX: implement encoding */
>
>  unneeded
>

Not sure why unneeded since for each block the previous blocks prev
and curr samples need to be remembered and are not present in the
stream.

>
>  [...]
>  > @@ -1235,6 +1246,53 @@
>
> >              }
>  >          }
>  >          break;
>  > +    case CODEC_ID_ADPCM_EA_MAXIS_XA:
>  > +        channel = avctx->channels;
>  > +
>  > +        if(channel > 2) {
>  > +            av_log(avctx, AV_LOG_ERROR, "Only 1 or 2 channels supported");
>  > +            return -1;
>  > +        }
>  > +
>
>  > +        coeff1l = ea_adpcm_table[(*src >> 4) & 0x0F];
>  > +        coeff2l = ea_adpcm_table[((*src >> 4) & 0x0F) + 4];
>  > +        shift_left = (*src & 0x0F) + 8;
>  > +        src++;
>  > +
>  > +        if(st) {
>  > +            coeff1r = ea_adpcm_table[(*src >> 4) & 0x0F];
>  > +            coeff2r = ea_adpcm_table[((*src >> 4) & 0x0F) + 4];
>  > +            shift_right = (*src & 0x0F) + 8;
>  > +            src++;
>  > +        }
>
>  duplicate
>

Not sure what the duplication refers too? If statement has duplicate
code to what precedes it or duplicate to already implemented EA ADPCM?

>
>
>  > +
>  > +        for (count1 = 0; count1 < 14 ; count1++) {
>  > +            for(count2 = 4; count2 >= 0; count2-=4) { /* Pairwise samples LL RR (st) or LL LL (mono) */
>
>  > +                next_left_sample = ((((*src >> count2) & 0x0F) << 0x1C) >> shift_left);
>
> > +                next_left_sample = (next_left_sample +
>  > +                    (c->mxa_current_left_sample * coeff1l) +
>  > +                    (c->mxa_previous_left_sample * coeff2l) + 0x80) >> 8;
>  > +                c->mxa_previous_left_sample = c->mxa_current_left_sample;
>  > +                c->mxa_current_left_sample = av_clip_int16(next_left_sample);
>  > +                *samples++ = (unsigned short)c->mxa_current_left_sample;
>  > +
>  > +                if(st) {
>  > +                    src++;
>  > +                    next_right_sample = ((((*src >> count2) & 0x0F) << 0x1C) >> shift_right);
>
> > +                    next_right_sample = (next_right_sample +
>  > +                        (c->mxa_current_right_sample * coeff1r) +
>  > +                        (c->mxa_previous_right_sample * coeff2r) + 0x80) >> 8;
>  > +                    c->mxa_previous_right_sample = c->mxa_current_right_sample;
>  > +                    c->mxa_current_right_sample = av_clip_int16(next_right_sample);
>  > +                    *samples++ = (unsigned short)c->mxa_current_right_sample;
>  > +                    src--;
>  > +                }
>
>  duplicate
>

ditto

> [...]
>
>  > +static int xa_read_header(AVFormatContext *s,
>
> > +               AVFormatParameters *ap)
>  > +{
>  > +    MaxisXADemuxContext *xa = s->priv_data;
>  > +    AVStream *st;
>  > +
>  > +    /* Extract Header Information from xa file */
>  > +    if (!xa_extract_header_info(s))
>  > +        return AVERROR(EIO);
>  > +
>  > +    /*Set up the XA Audio Decoder*/
>  > +    st = av_new_stream(s, 0);
>  > +    if (!st)
>  > +        return AVERROR(ENOMEM);
>  > +    av_set_pts_info(st, 33, 1, xa->sampleRate);
>  > +    st->codec->codec_type      = CODEC_TYPE_AUDIO;
>
> > +    st->codec->codec_id        = CODEC_ID_ADPCM_EA_MAXIS_XA;
>  > +    st->codec->bits_per_sample = xa->bits;
>
>  > +    st->codec->block_align     = (xa->bits/8) * xa->channels;
>
>  This is wrong.
>
>
>  [...]
>  > +    packet_size = 15*xa->channels;/* 1 byte header and 14 bytes worth of samples * number channels per block */
>  > +    if (av_get_packet(pb, pkt, packet_size) != packet_size) {
>  > +        av_log(s, AV_LOG_WARNING, "Size mismatch when reading data from file\n");
>  > +        av_free_packet(pkt);
>  > +        return AVERROR(EIO);
>  > +    }
>
>  This is wrong as well
>

What are you referring to when you say this is wrong?

>
>  > +    else {
>
> > +        pkt->stream_index = xa->audio_stream_index;
>  > +        pkt->pts = 90000;
>  > +        pkt->pts *= xa->audio_frame_counter;
>  > +        pkt->pts /= xa->sampleRate;
>  > +        xa->audio_frame_counter += (14 * xa->channels);  /* 14 Samples per channel  */
>
>  to quote the documentation
>  "int64_t pts;                            ///< presentation time stamp in time_base units"
>
>  This is not what you set it to
>
>

Is this then number_samples per packet / sample rate?

>  [...]
>