[FFmpeg-devel] atrac1 decoder and aea demuxer rev 6

[Diego Biurrun]

On Sun, Sep 06, 2009 at 08:12:04PM +0200, Benjamin Larsson wrote:
> 
> --- libavcodec/atrac1.c	(revision 0)
> +++ libavcodec/atrac1.c	(revision 0)
> @@ -0,0 +1,462 @@
> +
> +    DECLARE_ALIGNED_16(float,spec1[AT1_SU_SAMPLES]);        ///< mdct buffer
> +    DECLARE_ALIGNED_16(float,spec2[AT1_SU_SAMPLES]);        ///< mdct buffer
> +    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
> +
> +    DECLARE_ALIGNED_16(float,spec[AT1_SU_SAMPLES]);         ///< the mdct spectrum buffer
> +    DECLARE_ALIGNED_16(float,short_buf[64]);                ///< buffer for the short mode
> +    DECLARE_ALIGNED_16(float, low[256]);
> +    DECLARE_ALIGNED_16(float, mid[256]);
> +    DECLARE_ALIGNED_16(float,high[512]);

This would be more readable with spaces after the commas.

> +    if (rev_spec) {
> +        int i;
> +        for (i=0 ; i<transf_size/2 ; i++)
> +            FFSWAP(float, spec[i], spec[transf_size-1-i]);
> +
> +    for (band_num=0 ; band_num<AT1_QMF_BANDS ; band_num++) {
> +
> +        if (nbits!=5 && nbits!=7 && nbits!=8)

Spaces around operators would help here IMO.  Sometimes you use spaces,
sometimes you don't, so please use them everywhere.

> +                /* overlap and window between short blocks */
> +                q->dsp.vector_fmul_window(&su->spectrum[0][ref_pos+start_pos],
> +                    &su->spectrum[0][ref_pos+start_pos],q->short_buf,short_window, 0, 16);

Indentation is off, spaces after commas.

> +        /* overlap and window with the previous frame and output the result */
> +        q->dsp.vector_fmul_window(q->bands[band_num], &su->spectrum[1][ref_pos+band_samples/2],
> +            &su->spectrum[0][ref_pos], window_per_band[band_num], 0, band_samples/2);

ditto

> +    /* delay the signal of the high band by 23 samples */
> +    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);

weird alignment intent

> +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 (q->channels == 1) {
> +        /* mono */
> +        for (i = 0; i<AT1_SU_SAMPLES; i++)
> +            samples[i]     = av_clipf(q->out_samples[0][i], -32700./(1<<15), 32700./(1<<15));
> +    } else {
> +        /* stereo */
> +        for (i = 0; i < AT1_SU_SAMPLES; i++) {
> +            samples[i*2]   = av_clipf(q->out_samples[0][i], -32700./(1<<15), 32700./(1<<15));
> +            samples[i*2+1] = av_clipf(q->out_samples[1][i], -32700./(1<<15), 32700./(1<<15));

Some spaces around operators would help here IMO.

Also, 32700. instead of 32700.0 always confuses me.

> +    /* Init the mdct transforms */
> +    ff_mdct_init(&q->mdct_ctx[0], 6, 1, -1.0/ (1<<15));
> +    ff_mdct_init(&q->mdct_ctx[1], 8, 1, -1.0/ (1<<15));
> +    ff_mdct_init(&q->mdct_ctx[2], 9, 1, -1.0/ (1<<15));
> +    init_mdct_windows();

Again, spaces would IMO help.

> --- libavformat/aea.c	(revision 0)
> +++ libavformat/aea.c	(revision 0)
> @@ -0,0 +1,110 @@
> +
> +    st->codec->channel_layout = (st->codec->channels==1) ? CH_LAYOUT_MONO : CH_LAYOUT_STEREO;
> +
> +    st->codec->block_align = AT1_SU_SIZE * st->codec->channels;

align

Diego