[FFmpeg-devel] [PATCH] atrac decoder

[Diego Biurrun]

On Mon, Jun 08, 2009 at 09:40:18PM +0200, Benjamin Larsson wrote:
> Hi, this is working decoder for atrac. The things I have commented in
> the code will be moved to a common file when the code is in shape for
> inclusion. I left them there if people feel the urge to try the decoder.

Documentation and changelog updates are missing.

> --- libavcodec/atrac1data.h	(revision 0)
> +++ libavcodec/atrac1data.h	(revision 0)
> @@ -0,0 +1,66 @@
> +
> +/**
> + * @file libavcodec/atrac1data.h
> + * Atrac 1 compatible decoder data.

Drop the period


This header is missing multiple inclusion guards.

> --- libavcodec/atrac1.c	(revision 0)
> +++ libavcodec/atrac1.c	(revision 0)
> @@ -0,0 +1,458 @@
> +
> +
> +typedef struct {
> +    int                 bsm[AT1_QMF_BANDS];                 //< block size mode (1 byte large in bitstream)
> +    int                 num_bfus;                           //< number of Block Floating Units
> +    int                 idwls[AT1_MAX_BFU];                 //< the word length indexes for each BFU
> +    int                 idsfs[AT1_MAX_BFU];                 //< the scalefactor indexes for each BFU
> +    DECLARE_ALIGNED_16(float,prev_spec[AT1_SU_SAMPLES]);    //< previous frame mdct spectrum for each band(used for TDAC overlap)
> +    DECLARE_ALIGNED_16(float,fst_qmf_delay[46]);            //< delay line for the 1st stacked qmf filter
> +    DECLARE_ALIGNED_16(float,snd_qmf_delay[46]);            //< delay line for the 2nd stacked qmf filter
> +    DECLARE_ALIGNED_16(float,last_qmf_delay[256+23]);       //< delay line for the last stacked qmf filter
> +} AT1SUContext;
> +
> +    DECLARE_ALIGNED_16(float,long_buf[512]);                //< buffer for join short blocks into long ones

"buffer for joining" or "buffer to join"

> +static void at1_imdct_transform(ATRAC1Context *q, float *spec, float *out, int nbits, int reverse_spectrum)

nit: Please break overly long lines, same below.

> +    switch(nbits) {

nit: K&R, i.e. switch (

> +    /* parse the info byte (2nd byte) telling how much BFUs were coded */

how many

> +        for (bfu_num=bfu_bands_t[band_num] ; bfu_num<bfu_bands_t[band_num+1] ; bfu_num++) {
> +            int         pos;

That looks weirdly indented.

> +            if (word_len) {
> +                float       max_quant = 1.0/(float)((1 << (word_len - 1)) - 1);

ditto

> +    for(i=0; i<nIn; i+=2){

nit: "for () {" like in other places

> +    memcpy( su->last_qmf_delay,     &su->last_qmf_delay[256], sizeof(float)*23);
> +    memcpy(&su->last_qmf_delay[23], q->bands[2],       sizeof(float)*256);

Align or don't align, but don't half-align ;)

> +static int atrac1_decode_frame(AVCodecContext *avctx,
> +            void *data, int *data_size,
> +            AVPacket *avpkt)

indentation

> +    const uint8_t *buf = avpkt->data;
> +    int buf_size = avpkt->size;

align

> +        if(ret)
> +            return ret;
> +
> +        ret = at1_unpack_dequant(&q->gb, su, q->spec);
> +        if(ret)
> +            return ret;
> +
> +        ret = at1_imdct(su, q);
> +        if(ret)

nit: if (

> +static av_cold void init_mdct_windows()

void

> +        qmf_window[i] = s;
> +        qmf_window[47 - i] = s;

align

> +AVCodec atrac1_decoder =
> +{

nit: { on the same line

> +    .long_name = NULL_IF_CONFIG_SMALL("Atrac (Adaptive TRansform Acoustic Coding)"),

I think the correct official spelling is "ATRAC" if it is an acronym.

> --- libavcodec/allcodecs.c	(revision 18866)
> +++ libavcodec/allcodecs.c	(working copy)
> @@ -198,6 +198,7 @@
>      REGISTER_DECODER (APE, ape);
>      REGISTER_DECODER (ATRAC3, atrac3);
> +    REGISTER_DECODER (ATRAC1, atrac1);
>      REGISTER_DECODER (COOK, cook);
> --- libavcodec/Makefile	(revision 18866)
> +++ libavcodec/Makefile	(working copy)
> @@ -46,6 +46,7 @@
>  OBJS-$(CONFIG_ASV2_ENCODER)            += asv1.o mpeg12data.o
>  OBJS-$(CONFIG_ATRAC3_DECODER)          += atrac3.o
> +OBJS-$(CONFIG_ATRAC1_DECODER)          += atrac1.o
>  OBJS-$(CONFIG_AVS_DECODER)             += avs.o

alphabetical order

Diego