[FFmpeg-devel] [PATCH] Bink file demuxer and audio decoder

[Diego Biurrun]

On Sat, Jun 20, 2009 at 11:49:05PM +1000, Peter Ross wrote:
> 
> --- libavcodec/Makefile	(revision 19182)
> +++ libavcodec/Makefile	(working copy)
> @@ -27,6 +27,7 @@
>  # parts needed for many different codecs
>  OBJS-$(CONFIG_AANDCT)                  += aandcttab.o
>  OBJS-$(CONFIG_ENCODERS)                += faandct.o jfdctfst.o jfdctint.o
> +OBJS-$(CONFIG_DCT)                     += dct.o
>  OBJS-$(CONFIG_FFT)                     += fft.o
>  OBJS-$(CONFIG_GOLOMB)                  += golomb.o
>  OBJS-$(CONFIG_MDCT)                    += mdct.o

Ahem...

> --- libavcodec/dct.c	(revision 0)
> +++ libavcodec/dct.c	(revision 0)
> @@ -0,0 +1,95 @@
> +
> +static void ff_dct_calc_c(DCTContext *s, FFTSample *data)

There is no need to give static functions ff_ prefixes.

> +        for(i=0; i < n; i++) {

consistent application of K&R please..

> +        for(i=0; i<n-1; i++) {

ditto

> +    }else{

ditto

> +        for(i=0; i < n; i++) {

ditto

> +    if (s->inverse) {
> +        for(i=0; i < n; i++)

ditto

> +            data[i] = s->data[n-(i+1)].re / (2 * n);
> +    }else {

ditto

> +        for(i=0; i < n; i++)

ditto

> --- libavcodec/binkaudio.c	(revision 0)
> +++ libavcodec/binkaudio.c	(revision 0)
> @@ -0,0 +1,307 @@
> +static void decode_block(BinkAudioContext *s, short *out)
> +{
> +    int ch, i, j, k;
> +    float q, quant[25];
> +    int width, coeff;
> +    GetBitContext *gb = &s->gb;
> +
> +    if (s->use_dct) {
> +        skip_bits(gb, 2);
> +    }

useless {}

> +        // find band (k)
> +        for (k = 0; s->bands[k] * 2 < 2; k++) {
> +            q = quant[k];
> +        }

ditto

> +AVCodec binkaudio1_decoder = {
> +    "binkaudio1",
> +    CODEC_TYPE_AUDIO,
> +    CODEC_ID_BINKAUDIO1,
> +    sizeof(BinkAudioContext),
> +    decode_init,
> +    NULL,
> +    decode_end,
> +    decode_frame
> +};
> +
> +AVCodec binkaudio2_decoder = {
> +    "binkaudio2",
> +    CODEC_TYPE_AUDIO,
> +    CODEC_ID_BINKAUDIO2,
> +    sizeof(BinkAudioContext),
> +    decode_init,
> +    NULL,
> +    decode_end,
> +    decode_frame
> +};

Long names are missing.

> --- doc/general.texi	(revision 19236)
> +++ doc/general.texi	(working copy)
> @@ -544,6 +544,8 @@
>  @item Apple lossless audio   @tab  X  @tab  X
>      @tab QuickTime fourcc 'alac'
>  @item Atrac 3                @tab     @tab  X
> + at item Bink Audio             @tab     @tab  X
> +    @tab Used in Bink and Smacker files in many games.

Is it worth mentioning that both types are supported?  I guess only if
other types remain..

> --- libavformat/bink.c	(revision 0)
> +++ libavformat/bink.c	(revision 0)
> @@ -0,0 +1,245 @@
> +    if (b[0] == 'B' && b[1] == 'I' && b[2] == 'K' &&
> +        (b[3] == 'b' || b[3] == 'f' || b[3] == 'g' || b[3] == 'h' || b[3] == 'i') &&
> +        AV_RL32(b+8) > 0 &&  // num_frames
> +        AV_RL32(b+20) > 0 && AV_RL32(b+20) <= BINK_MAX_WIDTH &&
> +        AV_RL32(b+24) > 0 && AV_RL32(b+24) <= BINK_MAX_HEIGHT &&
> +        AV_RL32(b+28) > 0 && AV_RL32(b+32) > 0)  // fps num,den

This could be more readable nicely aligned.

> +AVInputFormat bink_demuxer = {
> +    "bink",
> +    "Bink",
> +    sizeof(BinkDemuxContext),
> +    probe,
> +    read_header,
> +    read_packet,
> +};

Long name is missing.

Diego