[FFmpeg-devel] [PATCH] atrac decoder

[Vitor Sessak]

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.
> 
> I will upload some sample files to the samples archive.

Just some nits:

> +/* start position of each BFU in the MDCT spectrum for the long mode */

Non doxy comment

> +static const uint16_t bfu_start_long[52] = {
> +      0,   8,  16,  24,  32,  36,  40,  44,  48,  56,  64,  72,  80,  86,  92,  98, 104, 110, 116, 122,
> +    128, 134, 140, 146, 152, 159, 166, 173, 180, 189, 198, 207, 216, 226, 236, 246,
> +    256, 268, 280, 292, 304, 316, 328, 340, 352, 372, 392, 412, 432, 452, 472, 492,
> +};
> +
> +/* start position of each BFU in the MDCT spectrum for the short mode */
> +static const uint16_t bfu_start_short[52] = {
> +      0,  32,  64,  96,   8,  40,  72, 104,  12,  44,  76, 108,  20,  52,  84, 116, 26, 58, 90, 122,
> +    128, 160, 192, 224, 134, 166, 198, 230, 141, 173, 205, 237, 150, 182, 214, 246,
> +    256, 288, 320, 352, 384, 416, 448, 480, 268, 300, 332, 364, 396, 428, 460, 492
> +};
> +

These two fit in uint8_t if divided by two (but I don't know if it is a
good idea speed wise).

> Index: libavcodec/atrac1.c
> ===================================================================
> --- libavcodec/atrac1.c	(revision 0)
> +++ libavcodec/atrac1.c	(revision 0)
> @@ -0,0 +1,458 @@
> +/*
> + * Atrac 1 compatible decoder
> + * Copyright (c) 2009 Maxim Poliakovski
> + * Copyright (c) 2009 Benjamin Larsson
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file libavcodec/atrac1.c
> + * Atrac 1 compatible decoder.
> + * This decoder handles raw ATRAC1 data.
> + */
> +
> +#include <math.h>
> +#include <stddef.h>
> +#include <stdio.h>
> +
> +#include "avcodec.h"
> +#include "get_bits.h"
> +#include "dsputil.h"
> +
> +#include "atrac1data.h"
> +
> +#define AT1_MAX_BFU      52  //< max number of block floating units in a sound unit

This is not a doxygen comment, "///<" will work (this happens in other
comments too).

> +static av_cold void init_mdct_windows()
> +{
> +    int i;
> +    /* Generate the short window */
> +    ff_sine_window_init(short_window,32);
> +    for (i=0 ; i<32; i++)
> +        short_window[63-i] = short_window[i];
> +
> +    /** The mid and long windows uses the same sine window splitted
> +     *  in the middle and wrapped into zero/one regions as follows:
> +     *
> +     *                   region of "ones"
> +     *               ---------------------------
> +     *              /                           \
> +     *             / 1st half                    \ 2nd half
> +     *            / of the sine                   \ of the sine window
> +     *           /  window                         \
> +     * ---------/                                   \----------------
> +     * zero region                                      zero region
> +     */

This comment will be parsed by doxygen, but it will make an ugly mess
out of it. I suggest putting it between \verbatim and \endverbatim.

-Vitor