[FFmpeg-soc] [Patch] Maxis EA XA decoder - GSoC Task
Robert Marston
rmarston at gmail.com
Wed Apr 9 16:28:50 CEST 2008
Attached patch has the following changes to it.
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:
> [...]
>
> > >
> > > > int tag;
> > > > int channels;
> > > > uint32_t sampleRate;
> > >
> > > redundant
> > >
> >
> > Removed tag.
>
> The others are redundant as well
>
Removed.
>
>
> >
> >
> > >
> > >
> > > [...]
> > > > 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 ...
>
Changed to 64.
>
> [...]
>
> > >
> > >
> > > > 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.
>
Changed to run check that haven't gone past size specified in header of file.
>
> [...]
> > 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 and hence they are just read
out. XA files don't have these and so the decoder needs to keep track
of them as far as I understand it.
>
>
> [...]
> > @@ -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
>
Removed duplicated code
>
>
> > +
> > + 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.
>
> [...]
> > +typedef struct MaxisXADemuxContext {
> > + int channels;
> > + uint32_t sampleRate;
> > + int bits;
>
> > + int audio_stream_index;
>
> unneeded
>
Removed.
>
> [...]
> > +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;
> > +}
>
> spliting this out in its own function makes no sense
>
>
Removed.
> > +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.
>
>
Changed values to be read from file. Unless there is something else
that was wrong with this?
> [...]
> > + 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
>
Changed the way the data was read so this shouldn't apply to the current method.
>
> > + 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.
Thanks for your help and patience Micheal.
-------------- 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/20080409/e5f4fe93/attachment.txt>
More information about the FFmpeg-soc
mailing list