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

[Robert Marston]

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.
>  [...]
>
> > >
>  > >  [...]
>  > >  >  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 */
>  > >
>  > Unless there is a way of getting the samples from the previous decoded
>  > frame then this is needed
>
>  > since other ADPCM formats have the previous
>  > and current samples encoded in the stream
>
>  No some do not.
>
>
Correct my code to use the variables the other formats use.
>  [...]
>
> > >
>  > >  > +    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
>  > >
>  > Removed this as I am unsure of the time base.
>
>  So i assume you are sure that its unneeded to set pts here? If so please
>  explain why it is unneeded. If not please set pts to a correct value.
>
Ignore the previous email with the reply to this, I think this is now correct?
>
>  [...]
>  > @@ -667,8 +669,12 @@
>  >  {
>  >      ADPCMContext *c = avctx->priv_data;
>  >      unsigned int max_channels = 2;
>  > -
>
>  cosmetic change
>
Mistake, corrected.
>
>  [...]
>  > @@ -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, though the other decoders add it
and the maxis xa wiki page warns that some compilers need it.
>
>  > +            shift[channel] = (*src & 0x0F) + 8;
>  > +            src++;
>  > +        }
>
>  indention is inconsistant
>
>
Corrected.
>
>  > +        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;
>
>  random indention
>
Ditto.
>
>  > +            int prev_next = 0;
>
>  contradictionary variable name
>
No longer used.
>
>  > +                for(channel = 0; channel < avctx->channels; channel++, prev_next+=2) {
>  > +                    sample = ((((*(src+channel) >> i) & 0x0F) << 0x1C) >> shift[channel]);
>  > +                    sample = (sample +
>
>  > +                        (c->curr_prev_samples[prev_next+1] * coeff[channel][0]) +
>  > +                        (c->curr_prev_samples[prev_next] * coeff[channel][1]) + 0x80) >> 8;
>
>
>  this can be vertically aligned
>
Aligned a bit better.
>
>  > +                    c->curr_prev_samples[prev_next] = c->curr_prev_samples[prev_next+1];
>  > +                    c->curr_prev_samples[prev_next+1] = av_clip_int16(sample);
>
>  > +                    *samples++ = (unsigned short)c->curr_prev_samples[prev_next+1];
>
>  unneeded cast
>
Removed.
>
>  [...]
>  > +typedef struct MaxisXADemuxContext {
>
>  > +    uint32_t out_size;
>  > +    uint32_t  sent_bytes;
>
>  inconsistant whitespace
>
Corrected.
>  [...]
>
-------------- 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/9b6a53a3/attachment.txt>