On Tue, Apr 8, 2008 at 4:24 AM, Michael Niedermayer <[EMAIL PROTECTED]>
wrote:

> On Tue, Apr 08, 2008 at 02:02:54AM +0200, Robert Marston wrote:
>

> > @@ -1235,6 +1246,68 @@
> >              }
> >          }
> >          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++;
> > +        }
> > +
> > +        for (count1 = 0; count1 < 14 ; count1++) {
> > +            next_left_sample = ((((*src >> 4) & 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;
> > +
> > +            next_left_sample = (((*src & 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);
> > +
> > +            if(st) {
> > +                src++;
> > +                next_right_sample = ((((*src >> 4) & 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;
> > +
> > +                *samples++ = (unsigned
> short)c->mxa_current_left_sample;
> > +
> > +                next_right_sample = (((*src & 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;
> > +            }
> > +            else {
> > +                *samples++ = (unsigned
> short)c->mxa_current_left_sample;
> > +            }
> > +            src++;
> > +        }
>
> This is full of code duplication, please remove all code duplication also
> it looks somewhat similar to the existing ADPCM_EA variants maybe some
> code
> can ba factored out or maybe it could be merged with one.
>

Removed a lot of redundancy in the code, there is still some duplication
when compared to the current EA ADPCM but to merge the two means creating
quiet a bit of confusing code and checking which format it is and offsetting
the data leads to code that is about the same in length. Can maybe look at
this again if you are really going to insist on it.


>
>
> [...]
> > typedef struct MaxisXADemuxContext {
>
> >     uint32_t xaId;
> >     uint32_t outSize;
>
> unused
>

 Removed


>
> >     int tag;
> >     int channels;
> >     uint32_t sampleRate;
>
> redundant
>

Removed tag.


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


>
>
> >     st->codec->codec_type = CODEC_TYPE_AUDIO;
> >     st->codec->codec_id = CODEC_ID_ADPCM_EA_MAXIS_XA;
> >     st->codec->codec_tag = xa->tag;
> >     st->codec->channels = xa->channels;
> >     st->codec->sample_rate = xa->sampleRate;
> >     st->codec->bits_per_sample = xa->bits;
> >     st->codec->bit_rate = xa->avgByteRate * 8;
> >     st->codec->block_align = xa->align;
>
> this can be vertically aligned like:
>
> st->codec->bits_per_sample = xa->bits;
> st->codec->bit_rate        = xa->avgByteRate * 8;
> st->codec->block_align     = xa->align;


Aligned these better.


> [...]
> > static int xa_read_packet(AVFormatContext *s,
> >                           AVPacket *pkt)
> > {
> >     MaxisXADemuxContext *xa = s->priv_data;
> >     ByteIOContext *pb = s->pb;
>
> >     int ret;
> >     unsigned int packet_size;
> >     packet_size = 15*xa->channels;/* 1 byte header and 14 byte samples *
> number channels per block */
> >     ret = av_get_packet(pb, pkt, packet_size);
>
> declaration and initialization can be merged
>

Done.


>
>
> >     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.


>
>
> >     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 += 2/xa->channels;
> >    }
>
> This looks wronge cannot see all ends.
>

Changed to what I think is more correct.

Also I attached a unified diff created with the GNU diff. If this is not
correct I am not sure what other diff program to use.
diff -urNt /home/rob/ffmpeg/libavcodec/adpcm.c ffmpeg/libavcodec/adpcm.c
--- /home/rob/ffmpeg/libavcodec/adpcm.c 2008-04-08 12:41:05.000000000 +0200
+++ ffmpeg/libavcodec/adpcm.c   2008-04-08 14:51:15.000000000 +0200
@@ -34,6 +34,7 @@
  * EA IMA EACS decoder by Peter Ross ([EMAIL PROTECTED])
  * EA IMA SEAD decoder by Peter Ross ([EMAIL PROTECTED])
  * EA ADPCM XAS decoder by Peter Ross ([EMAIL PROTECTED])
+ * MAXIS EA ADPCM decoder by Robert Marston ([EMAIL PROTECTED])
  * THP ADPCM decoder by Marco Gerards ([EMAIL PROTECTED])
  *
  * Features and limitations:
@@ -149,6 +150,10 @@
 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 */
@@ -669,6 +674,12 @@
     unsigned int max_channels = 2;
 
     switch(avctx->codec->id) {
+    case CODEC_ID_ADPCM_EA_MAXIS_XA:
+        c->mxa_current_left_sample = 0;
+        c->mxa_previous_left_sample = 0;
+        c->mxa_current_right_sample = 0;
+        c->mxa_previous_right_sample = 0;
+        break;
     case CODEC_ID_ADPCM_EA_R1:
     case CODEC_ID_ADPCM_EA_R2:
     case CODEC_ID_ADPCM_EA_R3:
@@ -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++;
+        }
+
+        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--;
+                }
+            }
+            src++;
+            if(st)
+                src++;/* With Stero source would have been decremented one to 
many times */
+        }
+        break;
     case CODEC_ID_ADPCM_EA_R1:
     case CODEC_ID_ADPCM_EA_R2:
     case CODEC_ID_ADPCM_EA_R3: {
@@ -1613,6 +1671,7 @@
 ADPCM_DECODER(CODEC_ID_ADPCM_4XM, adpcm_4xm);
 ADPCM_DECODER(CODEC_ID_ADPCM_CT, adpcm_ct);
 ADPCM_DECODER(CODEC_ID_ADPCM_EA, adpcm_ea);
+ADPCM_DECODER(CODEC_ID_ADPCM_EA_MAXIS_XA, adpcm_ea_maxis_xa);
 ADPCM_DECODER(CODEC_ID_ADPCM_EA_R1, adpcm_ea_r1);
 ADPCM_DECODER(CODEC_ID_ADPCM_EA_R2, adpcm_ea_r2);
 ADPCM_DECODER(CODEC_ID_ADPCM_EA_R3, adpcm_ea_r3);
diff -urNt /home/rob/ffmpeg/libavcodec/allcodecs.c ffmpeg/libavcodec/allcodecs.c
--- /home/rob/ffmpeg/libavcodec/allcodecs.c     2008-04-08 12:41:05.000000000 
+0200
+++ ffmpeg/libavcodec/allcodecs.c       2008-04-08 15:26:48.000000000 +0200
@@ -244,6 +244,7 @@
     REGISTER_ENCDEC  (ADPCM_ADX, adpcm_adx);
     REGISTER_DECODER (ADPCM_CT, adpcm_ct);
     REGISTER_DECODER (ADPCM_EA, adpcm_ea);
+    REGISTER_DECODER (ADPCM_EA_MAXIS_XA, adpcm_ea_maxis_xa);
     REGISTER_DECODER (ADPCM_EA_R1, adpcm_ea_r1);
     REGISTER_DECODER (ADPCM_EA_R2, adpcm_ea_r2);
     REGISTER_DECODER (ADPCM_EA_R3, adpcm_ea_r3);
diff -urNt /home/rob/ffmpeg/libavcodec/avcodec.h ffmpeg/libavcodec/avcodec.h
--- /home/rob/ffmpeg/libavcodec/avcodec.h       2008-04-08 12:41:05.000000000 
+0200
+++ ffmpeg/libavcodec/avcodec.h 2008-04-08 15:24:43.000000000 +0200
@@ -231,6 +231,7 @@
     CODEC_ID_ADPCM_IMA_EA_SEAD,
     CODEC_ID_ADPCM_IMA_EA_EACS,
     CODEC_ID_ADPCM_EA_XAS,
+    CODEC_ID_ADPCM_EA_MAXIS_XA,
 
     /* AMR */
     CODEC_ID_AMR_NB= 0x12000,
diff -urNt /home/rob/ffmpeg/libavformat/allformats.c 
ffmpeg/libavformat/allformats.c
--- /home/rob/ffmpeg/libavformat/allformats.c   2008-04-08 13:15:39.000000000 
+0200
+++ ffmpeg/libavformat/allformats.c     2008-04-08 15:28:45.000000000 +0200
@@ -169,6 +169,7 @@
     REGISTER_DEMUXER  (WSAUD, wsaud);
     REGISTER_DEMUXER  (WSVQA, wsvqa);
     REGISTER_DEMUXER  (WV, wv);
+    REGISTER_DEMUXER  (XA, xa);
     REGISTER_MUXDEMUX (YUV4MPEGPIPE, yuv4mpegpipe);
 
     /* external libraries */
diff -urNt /home/rob/ffmpeg/libavformat/Makefile ffmpeg/libavformat/Makefile
--- /home/rob/ffmpeg/libavformat/Makefile       2008-04-08 13:15:39.000000000 
+0200
+++ ffmpeg/libavformat/Makefile 2008-04-08 15:27:39.000000000 +0200
@@ -179,6 +179,7 @@
 OBJS-$(CONFIG_WSAUD_DEMUXER)             += westwood.o
 OBJS-$(CONFIG_WSVQA_DEMUXER)             += westwood.o
 OBJS-$(CONFIG_WV_DEMUXER)                += wv.o
+OBJS-$(CONFIG_XA_DEMUXER)                += xa.o
 OBJS-$(CONFIG_YUV4MPEGPIPE_MUXER)        += yuv4mpeg.o
 OBJS-$(CONFIG_YUV4MPEGPIPE_DEMUXER)      += yuv4mpeg.o
 
diff -urNt /home/rob/ffmpeg/libavformat/xa.c ffmpeg/libavformat/xa.c
--- /home/rob/ffmpeg/libavformat/xa.c   1970-01-01 02:00:00.000000000 +0200
+++ ffmpeg/libavformat/xa.c     2008-04-08 14:37:44.000000000 +0200
@@ -0,0 +1,131 @@
+/*
+ * Maxis XA (.xa) File Demuxer
+ * Copyright (c) 2008 Robert Marston
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file xa.c
+ * Maxis XA Audio Demuxer
+ * by Robert Marston ([EMAIL PROTECTED])
+ * for more information on the XA audio format visit
+ *   http://wiki.multimedia.cx/index.php?title=Maxis_XA
+ */
+
+#include "avformat.h"
+
+#define XA00_TAG MKTAG('X', 'A', 0, 0)
+#define XAI0_TAG MKTAG('X', 'A', 'I', 0)
+#define XAJ0_TAG MKTAG('X', 'A', 'J', 0)
+
+typedef struct MaxisXADemuxContext {
+    int channels;
+    uint32_t sampleRate;
+    int bits;
+    int audio_stream_index;
+    int64_t audio_frame_counter;
+} MaxisXADemuxContext;
+
+static int xa_probe(AVProbeData *p)
+{ 
+    switch(AV_RL32(&p->buf[0])) {
+    case XA00_TAG:
+    case XAI0_TAG:
+    case XAJ0_TAG:
+        return AVPROBE_SCORE_MAX;
+    }
+    return 0;
+}
+
+static int xa_extract_header_info(AVFormatContext *s)
+{
+    MaxisXADemuxContext *xa = s->priv_data;
+    ByteIOContext *pb = s->pb;
+
+    url_fskip(pb, 10); /* Skip the XA ID, data size and tag parameters */
+    xa->channels = get_le16(pb);
+    xa->sampleRate = get_le32(pb);
+    url_fskip(pb, 6); /* Skip the avgByteRate and Align parameters, since can 
be calculated */
+    xa->bits = get_le16(pb);
+
+    av_log(s, AV_LOG_DEBUG, "Channels = %d Sample Rate = %d, Bits = %d\n", 
xa->channels, xa->sampleRate, xa->bits);
+
+    return 1;
+}
+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;
+    st->codec->bit_rate        = st->codec->block_align * xa->sampleRate;
+    st->codec->channels        = xa->channels;
+    st->codec->sample_rate     = xa->sampleRate;
+
+    xa->audio_stream_index = st->index;
+    xa->audio_frame_counter = 0;
+
+    return 0;
+}
+
+static int xa_read_packet(AVFormatContext *s,
+                          AVPacket *pkt)
+{
+    MaxisXADemuxContext *xa = s->priv_data;
+    ByteIOContext *pb = s->pb;
+    unsigned int packet_size;
+
+    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);
+    }
+    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  */
+   }
+
+    return 0;
+}
+
+AVInputFormat xa_demuxer = {
+    "xa",
+    "Maxis XA Audio Format",
+    sizeof(MaxisXADemuxContext),
+    xa_probe,
+    xa_read_header,
+    xa_read_packet,
+};
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc

Reply via email to