[FFmpeg-devel] [Toy] Xxan (aka Xan for WC4) decoder

Diego Biurrun diego
Fri Dec 18 15:51:37 CET 2009


On Thu, Dec 17, 2009 at 03:15:31PM +0200, Kostya wrote:
> Well, $subj. Now it plays files almost fine.
> Mike should have finished it long time ago though.

How does this differ from the xan.c we already have, i.e. why is it
not a patch?

> // for av_memcpy_backptr
> #include "libavutil/lzo.h"

Should that go in a more central place?

> typedef struct XanContext {
> 
>     AVCodecContext *avctx;
>     int frame_size;
> 
> } XanContext;

pointless empty lines

>     s->buffer1_size = avctx->width * avctx->height;
>     s->buffer1 = av_malloc(s->buffer1_size);
>     s->buffer2_size = avctx->width * avctx->height;
>     s->buffer2 = av_malloc(s->buffer2_size + 130);
>     s->buffer3_size = avctx->width * avctx->height;
>     s->buffer3 = av_malloc(s->buffer2_size + 130);

Could be more readable aligned.

> static int xan_huffman_decode(unsigned char *dest, const unsigned char *src,
>     int dest_len)

indentation

>     while ( val != 0x16 ) {
>         if ( val < 0x16 ) {
>             if (dest >= dest_end)

inconsistent spaces inside parentheses

> static inline void xan_wc3_output_pixel_run(XanContext *s,
>     const unsigned char *pixel_buffer, int x, int y, int pixel_count)

indentation again, same in other places

>     while(pixel_count && (index < s->frame_size)) {

while (, more below

> static void xan_wc3_decode_frame(XanContext *s) {

K&R function declarations please, more below

>     int width = s->avctx->width;
>     int height = s->avctx->height;

align

> static void xan_wc4_decode_frame(XanContext *s) {
>     uint8_t *Y, *pix;
>     int x, y;
> 
>     if (s->size < 16)
>         return;
> {
> static int frame = 0;
> av_log(NULL,0,"Frame %d(%d) - %d, %X %X %X\n",++frame,AV_RL32(s->buf),AV_RL32(s->buf+4),AV_RL16(s->buf + AV_RL32(&s->buf[4]) + 4),AV_RL32(s->buf+8),AV_RL32(s->buf+12));
> }

This block looks weird.

>     if (AV_RL32(&s->buf[0]) == 0)

!AV_RL32(&s->buf[0])

There are similar statements in other places.

> AVCodec xan_wc4_decoder = {
>     "xan_wc4",
>     CODEC_TYPE_VIDEO,
>     CODEC_ID_XAN_WC4,
>     sizeof(XanContext),
>     xan_decode_init,
>     NULL,
>     xan_decode_end,
>     xan_decode_frame,
>     CODEC_CAP_DR1,

long_name is missing.

If you commit this, please don't forget docs, changelog, minor bump, etc.

Diego



More information about the ffmpeg-devel mailing list