[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes

[Diego Biurrun]

On Sat, Dec 19, 2009 at 03:17:37PM +0100, Vitor Sessak wrote:
>
> New patch attached.
>
> --- libavcodec/siprdata.h	(revision 0)
> +++ libavcodec/siprdata.h	(revision 0)
> @@ -0,0 +1,270 @@
> +
> +static const float pow_0_5[] = {
> +    1./(1 <<  1), 1./(1 <<  2), 1./(1 <<  3), 1./(1 <<  4),
> +    1./(1 <<  5), 1./(1 <<  6), 1./(1 <<  7), 1./(1 <<  8),
> +    1./(1 <<  9), 1./(1 << 10), 1./(1 << 11), 1./(1 << 12),
> +    1./(1 << 13), 1./(1 << 14), 1./(1 << 15), 1./(1 << 16)

I think this would be more readable with spaces around the /.

Is it just me that gets confused by '1.' instead of '1.0'?

> --- libavcodec/sipr.c	(revision 0)
> +++ libavcodec/sipr.c	(revision 0)
> @@ -0,0 +1,603 @@
> +
> +    uint8_t gp_index_bits;
> +    uint8_t fc_index_bits[10]; ///< size in bits of the fixed codebook indexes
> +    uint8_t gc_index_bits; ///< size in bits of the gain codebook indexes

nit: Align the comment markers.

> +static const SiprModeParam modes[MODE_COUNT] ={

= {

> +/** Apply pitch lag to the fixed vector (AMR section 6.1.2) */
> +static void pitch_sharpening(int pitch_lag_int, float beta, float *fixed_vector)

long line

> +/**
> + * Extract decoding parameters from the input bitstream.
> + * @param parms          parameters structure
> + * @param pgb            pointer to initialized GetbitContext structure
> + */
> +static void decode_parameters(SiprParameters* parms, GetBitContext *pgb,
> +                              const SiprModeParam *p)

Is there a reason why some static functions have Doxygen comments and
others do not?

> +static void lsp2lpc_sipr(const double *isp, float *Az)

Az?  Capitalized variable names look weird...

> +static void sipr_decode_lp(float *lsfnew, const float *lsfold, float *Az,
> +                           int num_subfr)
> +{
> +    float *pAz = Az;

Maybe just pass **Az?

> +/**
> + * Evaluate the adaptative impulse response

.

> +/**
> + * Evaluate the convolution of a vector with an sparse vector

s/an/a/

.

> +            float energy = ff_dot_productf(
> +                ctx->postfilter_syn5k0 + LP_FILTER_ORDER + i*L_SUBFR_SIPR,
> +                ctx->postfilter_syn5k0 + LP_FILTER_ORDER + i*L_SUBFR_SIPR,
> +                L_SUBFR_SIPR);

still ugly ;)


> +    ff_acelp_apply_order_2_transfer_function(synth,
> +                                    (const float[2]) {-1.99997   , 1.000000000},
> +                                    (const float[2]) {-1.93307352, 0.935891986},
> +                                    0.939805806, ctx->ymem, frame_size);

Indentation is off.  Maybe it's because this_function_has_a_very_long_name.

Diego