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

[Michael Niedermayer]

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

[...]
> +typedef struct CmvContext {
> +    AVCodecContext *avctx;
> +    AVFrame frame;        // current
> +    AVFrame last_frame;   // last
> +    AVFrame last2_frame;  // second-last
> +} CmvContext;

comments should be doxygen compatible


> +
> +static int cmv_decode_init(AVCodecContext *avctx){

should be av_cold


> +    CmvContext *s = avctx->priv_data;
> +    s->avctx = avctx;
> +    if (s->avctx->palctrl == NULL) {
> +        av_log(avctx, AV_LOG_ERROR, "cmv: palette expected.\n");
> +        return -1;
> +    }
> +    avctx->pix_fmt = PIX_FMT_PAL8;

> +    if (avcodec_check_dimensions(avctx, avctx->width, avctx->height))
> +        return -1;

that should be unneeded, avcodec_open() already checks it


[...]
> +static void cmv_decode_inter(CmvContext * s, const unsigned char *buf, int buf_size){
> +    const unsigned char *raw = buf + (s->avctx->width*s->avctx->height/16);
> +    int x,y,i;
> +
> +    i = 0;
> +    for(y=0; y<s->avctx->height/4; y++)
> +    for(x=0; x<s->avctx->width/4; x++) {
> +        if (buf[i]==0xFF) {
> +            unsigned char *dst = s->frame.data[0] + (y*4)*s->frame.linesize[0] + x*4;
> +            if (*raw==0xFF) { /* intra */
> +                raw++;
> +                memcpy(dst, raw, 4);
> +                memcpy(dst+s->frame.linesize[0], raw+4, 4);
> +                memcpy(dst+2*s->frame.linesize[0], raw+8, 4);
> +                memcpy(dst+3*s->frame.linesize[0], raw+12, 4);
> +                raw+=16;
> +            }else{  /* inter using second-last frame as reference */
> +                int xoffset = (*raw & 0x0F) - 7;
> +                int yoffset = ((*raw >> 4) & 0x0F) - 7;
> +                cmv_motcomp(s->frame.data[0], s->frame.linesize[0],
> +                            s->last2_frame.data[0], s->last2_frame.linesize[0],
> +                            x*4, y*4, xoffset, yoffset, s->avctx->width, s->avctx->height);
> +                raw++;
> +            }
> +        }else{  /* inter using last frame as reference */
> +            int xoffset = (buf[i] & 0x0F) - 7;

> +            int yoffset = ((buf[i] >> 4) & 0x0F) - 7;

the & 15 is unneeded


> +            cmv_motcomp(s->frame.data[0], s->frame.linesize[0],
> +                      s->last_frame.data[0], s->last_frame.linesize[0],
> +                      x*4, y*4, xoffset, yoffset, s->avctx->width, s->avctx->height);
> +        }
> +        i++;
> +    }
> +}
> +

> +static int cmv_decode_frame(AVCodecContext *avctx,
> +                            void *data, int *data_size,
> +                            uint8_t *buf, int buf_size)
> +{
> +    CmvContext *s = avctx->priv_data;
> +

> +    s->frame.data[0] = 0;

this looks wrong, this should not be needed


> +    if (avctx->get_buffer(avctx, &s->frame)) {
> +        av_log(avctx, AV_LOG_ERROR, "cmv: get_buffer() failed\n");
> +        return -1;
> +    }
> +

> +    memcpy(s->frame.data[1], avctx->palctrl->palette, AVPALETTE_SIZE);
> +    if (avctx->palctrl->palette_changed) {
> +        s->frame.palette_has_changed = 1;
> +        avctx->palctrl->palette_changed = 0;
> +    }

this way of passing the pallette around is deprecated and not thread safe.
Decoder and Demuxer can run in seperate threads and there can be a delay
of several frames between them ...


> +
> +    switch(buf[0]) {  // subtype
> +    case 0: cmv_decode_intra(s, buf+2, buf_size-2); break;
> +    case 1: cmv_decode_inter(s, buf+2, buf_size-2); break;
> +    default :
> +        return -1;
> +    }

doesnt this return leak some frames?


[...]
> @@ -241,6 +245,34 @@
>      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)

doxygen


> +{
> +    EaDemuxContext *ea = s->priv_data;
> +    ByteIOContext *pb = s->pb;
> +    int pal_start, pal_size, i;
> +
> +    url_fskip(pb, 4);

> +    ea->width = get_le16(pb);
> +    ea->height = get_le16(pb);

these could be vertically aligned

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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20080702/c4c6b03c/attachment.pgp>