[FFmpeg-devel] [PATCH 0/2] Origin Wing Commander IV video decoder

[Kostya]

On Thu, Feb 03, 2011 at 12:30:17PM +0100, Diego Biurrun wrote:
> On Thu, Feb 03, 2011 at 10:32:25AM +0100, Kostya wrote:
> > Here's mammoth-age project that should put both Mike (since he has been not
> > working on it since 2003 or 2004) and Peter (since it can be considered to
> > be EA codec) to shame.
> 
> :)
> 
> Funny, I remember fiddling with MPlayer trying to get Xxan samples to work...

Hasn't everybody tried it?
 
> > --- /dev/null
> > +++ b/libavcodec/xxan.c
> > @@ -0,0 +1,423 @@
> > +/*
> > + * Wing Commander/Xan Video Decoder
> > + * Copyright (C) 2011 Konstantin Shishkov
> > + * based on work of Mike Melanson
> 
> s/of/by

done

> > +static int xan_unpack_luma(const uint8_t *src, int src_size, uint8_t *dst, int dst_size)
> 
> Long line; src_size and dst_size should be const.

should be better now

> > +   uint8_t *dst_end = dst + dst_size;
> > +   const uint8_t *src_end = src + src_size;
> 
> dst_end should be const as well.

and this

> > +/* almost the same as in xan_wc3 decoder */
> > +static int xan_unpack(uint8_t *dest, int dest_len, const uint8_t *src, int src_len)
> 
> Long line; dest_len and src_len should be const.

amended

> Can the function be shared with wc3?

only after you clean and reformat xan.c

> > +    const uint8_t *src_end = src + src_len;
> > +    uint8_t *dest_end = dest + dest_len;
> 
> I think dest_end can be const as well.

why not, changed

> > +            if ( (opcode & 0x80) == 0 ) {
> > +            } else if ( (opcode & 0x40) == 0 ) {
> 
> nit: extra spaces inside parentheses

legacy

> > +                size = *src >> 6;
> > +                back  = (bytestream_get_be16(&src) & 0x3fff) + 1;
> > +                size2 = (opcode & 0x3f) + 4;
> > +            } else {
> > +                size = opcode & 3;
> > +                back  = ((opcode & 0x10) << 12) + bytestream_get_be16(&src) + 1;
> > +                size2 = ((opcode & 0x0c) <<  6) + *src++ + 5;
> 
> nit: Could be aligned, same above and below.

aligned

> > +            memcpy(dest, src, size);  dest += size;  src += size;
> 
> nit: Multiple statements on one line confuse me, same below.

changed a bit

> > +    src = s->y_buffer;
> > +    ybuf = s->pic.data[0];
> 
> nit: align

aligned

> > >From 02c8c1fac247673435b4ea2c18ef321538d5b4fe Mon Sep 17 00:00:00 2001
> > From: Kostya <kostya.shishkov at gmail.com>
> > Date: Thu, 3 Feb 2011 10:06:12 +0100
> > Subject: [PATCH 2/2] Remove Xan WC4 cruft from xan.c
> 
> OK I guess.

It's independent so no worries.
 
> Diego