On Mon, Sep 30, 2013 at 02:38:57PM +0200, Maxim Polijakowski wrote:
> 
> From 7a850f8fc070436b20a5adee496fe81466d821d2 Mon Sep 17 00:00:00 2001
> From: Maxim Poliakovski <[email protected]>
> Date: Mon, 30 Sep 2013 14:00:18 +0200
> Subject: [PATCH 1/3] Move doxygen comments to the header file.
> 
> --- a/libavcodec/atrac.c
> +++ b/libavcodec/atrac.c
> @@ -44,9 +44,6 @@ static const float qmf_48tap_half[24] = {
>     -0.043596379,   -0.099384367,   0.13207909,    0.46424159
>  };
>  
> -/**
> - * Generate common tables
> - */
>  
>  void ff_atrac_generate_tables(void)
>  {
> @@ -67,18 +64,6 @@ void ff_atrac_generate_tables(void)
>  }
>  
>  
> -/**
> - * Quadrature mirror synthesis filter.
> - *
> - * @param inlo      lower part of spectrum
> - * @param inhi      higher part of spectrum
> - * @param nIn       size of spectrum buffer
> - * @param pOut      out buffer
> - * @param delayBuf  delayBuf buffer
> - * @param temp      temp buffer
> - */
> -
> -
>  void ff_atrac_iqmf (float *inlo, float *inhi, unsigned int nIn, float *pOut, 
> float *delayBuf, float *temp)

Two empty lines seems excessive.

> --- a/libavcodec/atrac.h
> +++ b/libavcodec/atrac.h
> @@ -30,7 +30,23 @@
>  
>  extern float ff_atrac_sf_table[64];
>  
> +
> +/**
> + * Generate common tables.
> + */
>  void ff_atrac_generate_tables(void);
> +
> +
> +/**
> + * Quadrature mirror synthesis filter.
> + *
> + * @param inlo      lower part of spectrum
> + * @param inhi      higher part of spectrum
> + * @param nIn       size of spectrum buffer
> + * @param pOut      out buffer
> + * @param delayBuf  delayBuf buffer
> + * @param temp      temp buffer
> + */
>  void ff_atrac_iqmf (float *inlo, float *inhi, unsigned int nIn, float *pOut, 
> float *delayBuf, float *temp);

same

> From 4161d2be843edf4bc409fc42b6cb5974c2eeed39 Mon Sep 17 00:00:00 2001
> From: Maxim Poliakovski <[email protected]>
> Date: Mon, 30 Sep 2013 14:04:43 +0200
> Subject: [PATCH 2/3] Improve file description and actualize the copyright
>  messages.

Careful, "actualize" is a false friend for German speakers; it does not mean
"aktualisieren", but rather "verwirklichen".

These changes should be folded into some other commit that touches the files.

> From b8db1f2eb215ca20fad0783c63742cac9ea1fbda Mon Sep 17 00:00:00 2001
> From: Maxim Poliakovski <[email protected]>
> Date: Mon, 30 Sep 2013 14:27:31 +0200
> Subject: [PATCH 3/3] Generalize ATRAC3 gain compensation code and move it to
>  atrac.c so it can be reused within ATRAC3+ decoder.

This patch contains trailing whitespace.

> --- a/libavcodec/atrac.c
> +++ b/libavcodec/atrac.c
> @@ -64,7 +64,69 @@ void ff_atrac_generate_tables(void)
> +    /* Generate gain level table. */
> +    for (i = 0; i < 16; i++)
> +        gctx->gain_tab1[i] = powf(2.0, (id2exp_offset - i));

pointless ()

> +    /* Generate gain interpolation table. */
> +    for (i = -15; i < 16; i++)
> +        gctx->gain_tab2[i + 15] = powf(2.0, i * (-1.0f / gctx->loc_size));

ditto

> +void ff_atrac_gain_compensation(GCContext *gctx, float *in, float *prev,
> +                                GainInfo *gc_now, GainInfo *gc_next,
> +                                int num_samples, float *out)
> +{
> +    float lev, gc_scale, gain_inc;
> +    int i, pos, lastpos;
> +
> +    gc_scale = (gc_next->num_points) ? gctx->gain_tab1[gc_next->levcode[0]] 
> : 1.0f;

ditto

> +            lev = gctx->gain_tab1[gc_now->levcode[i]];
> +            gain_inc = gctx->gain_tab2[(i+1 < gc_now->num_points ?
> +                                        gc_now->levcode[i+1] : 
> gctx->id2exp_offset)
> +                                       - gc_now->levcode[i] + 15];

spaces around +

> +    /* copy the overlapping part into delay buffer */
> +    memcpy(prev, &in[num_samples], num_samples*sizeof(float));

spaces around *

> -void ff_atrac_iqmf (float *inlo, float *inhi, unsigned int nIn, float *pOut, 
> float *delayBuf, float *temp)
> +void ff_atrac_iqmf (float *inlo, float *inhi, unsigned int nIn, float *pOut,
> +                    float *delayBuf, float *temp)

unrelated cosmetics

> --- a/libavcodec/atrac.h
> +++ b/libavcodec/atrac.h
> @@ -28,6 +28,28 @@
>  
> +/**
> + *  Gain control parameters for one subband.
> + */
> +typedef struct {
> +    int   num_points; ///< number of gain control points
> +    int   levcode[7]; ///< level at corresponding control point
> +    int   loccode[7]; ///< location of gain control points
> +} GainInfo;
> +
> +
> +/**
> + *  Gain compensation context structure.
> + */
> +typedef struct {
> +    float   gain_tab1[16]; ///< gain compensation level table
> +    float   gain_tab2[31]; ///< gain compensation interpolation table
> +    int     id2exp_offset; ///< offset for converting level index into level 
> exponent
> +    int     loc_scale;     ///< scale of location code = 2^loc_scale samples
> +    int     loc_size;      ///< size of location code in samples
> +} GCContext;

no anonymous structs please

Also, the names are rather generic and could use prefixing.

> @@ -38,6 +60,32 @@ void ff_atrac_generate_tables(void);
>  
>  
>  /**
> + *  Initialize gain compensation context.
> + *
> + * @param gctx            pointer to gain compensation context to initialize
> + * @param id2exp_offset   offset for converting level index into level 
> exponent
> + * @param loc_scale       location size factor
> + */
> +void ff_atrac_init_gain_compensation(GCContext *gctx, int id2exp_offset, int 
> loc_scale);

nit: long line

> @@ -47,6 +95,7 @@ void ff_atrac_generate_tables(void);
>   * @param delayBuf  delayBuf buffer
>   * @param temp      temp buffer
>   */
> -void ff_atrac_iqmf (float *inlo, float *inhi, unsigned int nIn, float *pOut, 
> float *delayBuf, float *temp);
> +void ff_atrac_iqmf (float *inlo, float *inhi, unsigned int nIn, float *pOut,
> +                    float *delayBuf, float *temp);

unrelated cosmetics

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to