[FFmpeg-devel] [PATCH] IFF demuxer and 8SVX decoder

[Diego Biurrun]

On Wed, Mar 26, 2008 at 11:43:28AM +0000, Jai Menon wrote:
> 
> I have completed testing of both the iff demuxer and 8svx audio decoder 
> locally on test samples i could find. They are working as expected and i'm 
> attaching the patch for review.

I think you should split demuxer and decoder into separate patches.

> please let me know your thoughts/comments and changes i have to make for svn 
> inclusion.

Here goes...

> --- libavcodec/Makefile	(revision 12552)
> +++ libavcodec/Makefile	(working copy)
> @@ -65,6 +65,8 @@
>  OBJS-$(CONFIG_EIGHTBPS_DECODER)        += 8bps.o
> +OBJS-$(CONFIG_EIGHTSVX_FIB_DECODER)    += 8svx.o
> +OBJS-$(CONFIG_EIGHTSVX_EXP_DECODER)    += 8svx.o

alphabetical order

> --- libavcodec/8svx.c	(revision 0)
> +++ libavcodec/8svx.c	(revision 0)
> @@ -0,0 +1,136 @@
> +/*
> + * 8SVX Audio Decoder

lowercase

> +static int eightsvx_decode_frame(AVCodecContext *avctx, void *data, int *data_size, const uint8_t *buf, int buf_size)

Please break long lines before 80 characters.

> +    if(esc->first_frame)
> +    {
> +        esc->fib_acc = in_data[1];
> +        in_data++;
> +    }

The preferred style is to keep { on the same line as the if.

> +/**
> + *
> + * init 8svx decoder
> + *
> + */

Initialize 8svx decoder.

Also, this could be done in 3 lines instead of 5.

> --- libavcodec/allcodecs.c	(revision 12552)
> +++ libavcodec/allcodecs.c	(working copy)
> @@ -78,6 +78,8 @@
>      REGISTER_DECODER (EIGHTBPS, eightbps);
> +    REGISTER_DECODER (EIGHTSVX_FIB, eightsvx_fib);
> +    REGISTER_DECODER (EIGHTSVX_EXP, eightsvx_exp);

ditto

> --- libavcodec/avcodec.h	(revision 12552)
> +++ libavcodec/avcodec.h	(working copy)
> @@ -179,6 +179,8 @@
>      CODEC_ID_RL2,
> +    CODEC_ID_8SVX_FIB,
> +    CODEC_ID_8SVX_EXP,

ditto

> --- libavformat/iff.c	(revision 0)
> +++ libavformat/iff.c	(revision 0)
> @@ -0,0 +1,220 @@
> +/*
> + * Iff (.iff) File Demuxer

lowercase

> +/** 8svx specific chunk ids */

IDs

> +/** 8svx Compression */

lowercase

Diego