[FFmpeg-devel] [PATCH] Electronic Arts CMV decoder

[Michael Niedermayer]

On Thu, Jul 03, 2008 at 10:58:05PM +1000, pross at xvid.org wrote:
> On Wed, Jul 02, 2008 at 08:27:01PM +0200, Michael Niedermayer wrote:
> > On Tue, Jul 01, 2008 at 10:36:41PM +1000, pross at xvid.org wrote:
> > > Hi,
> > > 
> > > This patch adds EA CMV support to FFmpeg.
> > > 
> > > Apply the decoder patch first, then the demuxer patch.
> > > 
> > > Samples: http://samples.mplayerhq.hu/game-formats/ea-cmv/
> > > Information: http://wiki.multimedia.cx/index.php?title=Electronic_Arts_CMV
> > 
> > [...]
> 
> Thanks to Diego and Michael for taking the time to review.
> 
> Revised patch enclosed.
[...]
> +    buf += 16;
> +    for (i=pal_start; i<pal_start+pal_count && i<AVPALETTE_COUNT; i++) {

> +        s->palette[i] = (buf[0] << 16) | (buf[1] << 8) | buf[2];
> +        buf += 3;

AV_RB24() or something from bytestream.h


> +    }
> +}
> +
> +#define EA_PREAMBLE_SIZE 8
> +#define MVIh_TAG MKTAG('M', 'V', 'I', 'h')
> +
> +static int cmv_decode_frame(AVCodecContext *avctx,
> +                            void *data, int *data_size,
> +                            const uint8_t *buf, int buf_size)
> +{
> +    CmvContext *s = avctx->priv_data;
> +
> +    if (AV_RL32(buf)==MVIh_TAG||AV_RB32(buf)==MVIh_TAG) {
> +        cmv_process_header(s, buf+EA_PREAMBLE_SIZE, buf_size-EA_PREAMBLE_SIZE);
> +        return buf_size;
> +    }
> +

> +    /* shuffle */
> +    if (s->last2_frame.data[0])
> +        avctx->release_buffer(avctx, &s->last2_frame);
> +    s->last2_frame = s->last_frame;
> +    s->last_frame = s->frame;
> +
> +    s->frame.data[0] = NULL;

i would rotate the 3 so they are 123->312 not 112 that should avoid the
= NULL


> +    s->frame.reference = 1;
> +    s->frame.buffer_hints = FF_BUFFER_HINTS_VALID;
> +    if (avctx->get_buffer(avctx, &s->frame)<0) {
> +        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    memcpy(s->frame.data[1], s->palette, AVPALETTE_SIZE);
> +
> +    if ((buf[EA_PREAMBLE_SIZE]&1)) {  // subtype
> +        cmv_decode_inter(s, buf+EA_PREAMBLE_SIZE+2, buf_size-(EA_PREAMBLE_SIZE+2)); 
> +        s->frame.key_frame = 0;
> +        s->frame.pict_type = FF_P_TYPE;
> +    }else{
> +        s->frame.key_frame = 1;
> +        s->frame.pict_type = FF_I_TYPE;
> +        cmv_decode_intra(s, buf+EA_PREAMBLE_SIZE+2, buf_size-(EA_PREAMBLE_SIZE+2));
> +    }

a buf_end would avoid the - (EA_PREAMBLE_SIZE+2)
also a
buf += EA_PREAMBLE_SIZE would simplify the code a little


[...]
> @@ -241,6 +244,24 @@
>      return 1;
>  }
>  
> +/**
> + * Process CMV video header
> + * @return 1 if success, 0 if invalid format, otherwise AVERROR_xxx
> + */
> +static int process_video_header_cmv(AVFormatContext *s)
> +{
> +    EaDemuxContext *ea = s->priv_data;
> +    ByteIOContext *pb = s->pb;
> +
> +    url_fskip(pb, 4);
> +    ea->width         = get_le16(pb);
> +    ea->height        = get_le16(pb);
> +    url_fskip(pb, 2);
> +    ea->time_base.num = 1;
> +    ea->time_base.den = get_le16(pb);
> +    return 1;
> +}

As this is already passed to the codec, it as well could set width/height
and time_base
this would also mean less codec specific code in the demuxer. Though it will
require a avcodec_check_dimensions()

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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-devel/attachments/20080703/11b2d98a/attachment.pgp>