[FFmpeg-soc] [Patch] Maxis EA XA decoder - GSoC Task
Michael Niedermayer
michaelni at gmx.at
Sun Apr 13 12:41:00 CEST 2008
On Sun, Apr 13, 2008 at 11:23:53AM +0200, Robert Marston wrote:
> On Sun, Apr 13, 2008 at 1:57 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, Apr 13, 2008 at 12:50:30AM +0200, Robert Marston wrote:
> > > On Sat, Apr 12, 2008 at 7:21 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > On Sat, Apr 12, 2008 at 06:12:15PM +0200, Robert Marston wrote:
> > > > > On Sat, Apr 12, 2008 at 3:00 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > > >
> > > > > > On Sat, Apr 12, 2008 at 01:39:45PM +0200, Robert Marston wrote:
> > > > > > >
> > > > > > > Thanks for pointing that out, I will be the first to admit that my c
> > > > > > > knowledge is probably not up to standard when it comes to FFMPEG and
> > > > > > > as such would required much feedback from my mentor. I see the GSoC as
> > > > > > > good learning opportunity for the myself and a chance to bolster open
> > > > > > > source development and potential to increase the code base of the
> > > > > > > mentor organizations project.
> > > > > > >
> > > > > > > Would casting the *(src + channel) to a int32_t stop the above from happening?
> > > > > >
> > > > > > i would put the cast after <<0x1C but before >>shift[channel]
> > > > > >
> > > > >
> > > > > As a matter of interest is that to avoid loosing higher order bits in
> > > > > the <<0x1C shift?
> > > >
> > > > no
> > > >
> > >
> > > Sorry that a was a stupid question, the cast would have dropped the
> > > bits anyway. What is the reason for putting the cast after the 0x1C
> > > shift?
> >
> > to get the msb into the sign bit
> > just look at these 2 to see the difference on normal x86
> > int x= 0xFFF0;
> > int y= (int16_t)0xFFF0;
> >
> >
> > [...]
> > > + 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 = (int32_t)(((*(src+channel) >> i) & 0x0F) << 0x1C) >> shift[channel];
> > > + sample = (sample +
> >
> > > + (c->status[channel].sample1 * coeff[channel][0]) +
> > > + (c->status[channel].sample2 * coeff[channel][1]) + 0x80) >> 8;
> >
> > superflous ()
> >
> >
> > [...]
> > > +static int xa_probe(AVProbeData *p)
> > > +{
> >
> > > + switch(AV_RL32(&p->buf[0])) {
> >
> > this can be written slightly simpler
> >
> >
> > [...]
> > > +static int xa_read_header(AVFormatContext *s,
> > > + AVFormatParameters *ap)
> > > +{
> > > + MaxisXADemuxContext *xa = s->priv_data;
> > > + ByteIOContext *pb = s->pb;
> > > + AVStream *st;
> > > +
> > > + /*Set up the XA Audio Decoder*/
> > > + st = av_new_stream(s, 0);
> > > + if (!st)
> > > + return AVERROR(ENOMEM);
> > > +
> > > + st->codec->codec_type = CODEC_TYPE_AUDIO;
> > > + st->codec->codec_id = CODEC_ID_ADPCM_EA_MAXIS_XA;
> > > + url_fskip(pb, 4); /* Skip the XA ID */
> > > + xa->out_size = get_le32(pb);
> > > + url_fskip(pb, 2); /* Skip the tag */
> > > + st->codec->channels = get_le16(pb);
> > > + st->codec->sample_rate = get_le32(pb);
> > > + /* Value in file is average byte rate*/
> > > + st->codec->bit_rate = get_le32(pb) * 8;
> > > + st->codec->block_align = get_le16(pb);
> > > + st->codec->bits_per_sample = get_le16(pb);
> > > +
> > > + av_set_pts_info(st, 64, 1, st->codec->sample_rate);
> >
> > > + xa->audio_frame_counter = 0;
> >
> > The context is magically initalized to all 0.
> >
> > [...]
>
> Corrected above issues, patch attached.
[...]
> @@ -1235,6 +1237,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++) {
^ ^
inconsistant whitespace placement
> + 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 = (int32_t)(((*(src+channel) >> i) & 0x0F) << 0x1C) >> shift[channel];
declaration and initalization can be merged
[...]
> +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);
ret is writen to twice but not read between.
> + pkt->stream_index = st->index;
> +
> + xa->sent_bytes += packet_size;
> + pkt->pts = xa->audio_frame_counter;
> + /* 14 bytes Samples per channel with 2 samples per byte */
> + xa->audio_frame_counter += (28 * st->codec->channels);
superflous ()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- 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/20080413/b2e424d9/attachment.pgp>
More information about the FFmpeg-soc
mailing list