[FFmpeg-devel] [PATCH] G.729A (now fixed-point) decoder
Michael Niedermayer
michaelni
Sun Mar 23 20:00:32 CET 2008
On Thu, Mar 20, 2008 at 10:12:54PM +0600, Vladimir Voroshilov wrote:
> Hi, Michael.
>
> On Thu, Mar 20, 2008 at 8:29 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, Mar 16, 2008 at 12:24:58AM +0600, Vladimir Voroshilov wrote:
> >
> > > +Per-vector results (PASS means no hearable differences, FAILED means hearable artefacts):
> > > +algthm : PASS
> > > +erasure : PASS
> > > +fixed : PASS
> > > +lsp : PASS
> >
> > > +overflow: FAIL
> >
> > fix it!
>
> "FAIL" here means 39 PSNR vs 65-88 in other tests.
> Searching for error still in progress, though.
just keep in mind this has to be fixed before it will hit svn!
Also i like to have binary identical output or an explanation why precissely
it is not.
[...]
> > [...]
> > > + int16_t pred_energ_q[4]; ///< (Q10) past quantized energies
> >
> > energY
>
> Was undestood and fixed as "(Q10) past quantized energy"
s/pred_energ_q/quant_energy/
[...]
> +#define PITCH_MIN 20
> +#define PITCH_MAX 143
s/PITCH_/PITCH_LAG_/
[...]
> +#define P1_BITS 8 ///< zdaptive codebook index for first subframe (size in bits)
^^^^^^^^
no en. word
[...]
> + int subframe_idx; ///< subframe index (for debugging)
unused remove it!
[...]
> +/* 4.2.2 */
> +#define GAMMA_N 18022 //0.55 in Q15
> +#define GAMMA_D 22938 //0.70 in Q15
s/N/NUM/
s/D/DEN/
s/GAMMA/FORMANT_PP_FACTOR/
> +#define GAMMA_T 26214 //0.80 in Q15
TILT_FACTOR
[...]
> +#define Q12_BASE 4096.0
> +#define Q13_BASE 8192.0
> +#define Q15_BASE 32768.0
unused (and remove all other unused things as well
[...]
> +/**
> + * GA codebook (3.9.2)
> + */
Do you know what GA stands for? hint: gain
Write it in the comment!
[...]
> +/**
> + * MA predictor (3.2.4)
> + */
do you know what "MA" stands for?
I dont, but you should write it in the comment! Its more usefull
than to repeat the abbreviation of the variable name.
[...]
> +/**
> + * \brief divide two fixed point numbers
> + * \param num numenator
> + * \param denom denumenator
> + * \param base base to scale result to
> + *
> + * \return result of division scaled to given base
> + */
> +int l_div(int num, int denom, uint8_t base)
> +{
> + int diff, sig;
> +
> + if(!num)
> + return 0;
> +
> + sig = (num ^ denom) >> 31;
> +
> + num = FFABS(num);
> + denom = FFABS(denom);
> +
> + diff = 26 - av_log2(num);
> + assert(diff >= 0 && base >= 0);
> +
> + num <<= FFMIN(base, diff);
> + denom >>= FFMAX(base, diff) - diff;
> +
> + assert(denom); //division by zero or base too large (overflow)
> +
> + if(sig)
> + return -num/denom;
> + else
> + return num/denom;
if(sig)
num= -num;
return num/denom;
you can also get rid of the >> 31
and use if(sig<0)
and the assert(denom) is useless, it will kill the prog with a div by
zero anyway, this isnt float stuff with infinity.
[...]
> +/**
> + * \brief Decoding of the adaptive-codebook vector (4.1.3)
> + * \param pitch_delay_int pitch delay, integer part
> + * \param pitch_delay_frac pitch delay, fraction part [-1, 0, 1]
> + * \param ac_v [out] (Q0) buffer to store decoded vector into
> + * \param subframe_size length of subframe
> + */
> +static void g729_decode_ac_vector(int pitch_delay_int, int pitch_delay_frac, int16_t* ac_v, int subframe_size)
> +{
Maybe this should rather be caled interpolate_excitation() with the
meaningless spec name just in a comment?
> + int n, i;
> + int v, tmp;
> +
> + //Make sure that pitch_delay_frac will be always positive
> + pitch_delay_frac =- pitch_delay_frac;
> + if(pitch_delay_frac < 0)
> + {
> + pitch_delay_frac += 3;
> + pitch_delay_int++;
> + }
> +
> + //t [0, 1, 2]
> + //k [PITCH_MIN-1; PITCH_MAX]
> + for(n=0; n<subframe_size; n++)
> + {
> + /* 3.7.1, Equation 40 */
> + v=0;
> + for(i=0; i<10; i++)
> + {
> + /* R(x):=ac_v[-k+x] */
> + tmp = ac_v[n - pitch_delay_int - i ] * interp_filter[i][ pitch_delay_frac];
> + v = av_clip(v + tmp, INT_MIN >> 1, INT_MAX >> 1); //v += R(n-i)*interp_filter(t+3i)
v+= ac_v[n - pitch_delay_int - i ] * interp_filter[i][ pitch_delay_frac];
v= av_clip(v, INT_MIN >> 1, INT_MAX >> 1); //v += R(n-i)*interp_filter(t+3i)
[...]
> +/**
> + * \brief fixed codebook vector modification if delay is less than 40 (4.1.4 and 3.8)
really? it seems you apply it always?
[...]
> +
> +/**
> + * \brief Decoding of the adaptive codebook gain (4.1.5 and 3.9.1)
> + * \param ctx private data structure
> + * \param ga_cb_index GA gain codebook index (stage 2)
> + * \param gb_cb_index GB gain codebook (stage 2)
> + * \param fc_v (Q13) fixed-codebook vector
> + * \param pred_energ_q [in/out] (Q10) past quantized energies
> + * \param subframe_size length of subframe
> + *
> + * \return (Q1) quantized adaptive-codebook gain (gain code)
> + */
> +static int16_t g729_get_gain_code(int ga_cb_index, int gb_cb_index, const int16_t* fc_v, int16_t* pred_energ_q, int subframe_size)
> +{
This predicts the gain and then "adds" the correction from the bitstream.
maybe that should be split in 2 functions
> + int i;
> + int cb1_sum; // Q12
> + int energy;
> + int exp;
> +
> + /* 3.9.1, Equation 66 */
> + energy = sum_of_squares(fc_v, subframe_size, 0, 0);
> +
> + /*
> + energy=mean_energy-E
> + mean_energy=30dB
> + E is calculated in 3.9.1 Equation 66
> +
> + (energy is in Q26)
> + energy = 30 - 10 * log10(energy / (2^26 * subframe_size))
> + =30 - 10 * log2(energy / (2^26 * subframe_size)) / log2(10)
> + =30 - 10*log2(energy/2^26)/log2(10) + 10*log2(subframe_size)/log2(10)
> + =30 - [10/log2(10)] * log2(energy/2^26) + [10/log2(10)] * log2(subframe_size)
> + = -24660 * log2(energy) + 24660 * log2(subframe_size) + 24660 * 26 + 30<<13
> +
trailing whitespace
[...]
> + /* 3.9.1, Equation 71 */
> + /*
> + energy = 10^(energy / 20) = 2^(3.3219 * energy / 20) = 2^ (0.166 * energy)
> + 5439 = 0.166 in Q15
> + */
> + energy = (5439 * (energy >> 15)) >> 8; // Q23->Q8, Q23 -> Q15
> +
> + /*
> + Following code will calculate energy*2^14 instead of energy*2^exp
> + due to recent change of energy_int's integer part.
> + This is done to avoid overflow. Result fits into 16-bit.
> + */
> + exp = (energy >> 15); // integer part (exponent)
> + energy = l_pow2(energy & 0x7fff) & 0x7fff; // Only fraction part of Q15
> +
> + // shift prediction error vector
> + for(i=3; i>0; i--)
> + pred_energ_q[i] = pred_energ_q[i-1];
s/error/energy/
[...]
> +/**
> + * \brief Memory update (3.10)
> + * \param fc_v (Q13) fixed-codebook vector
> + * \param gp (Q14) quantized fixed-codebook gain (gain pitch)
> + * \param gc (Q1) quantized adaptive-codebook gain (gain code)
> + * \param exc [in/out] (Q0) last excitation signal buffer for current subframe
> + * \param subframe_size length of subframe
> + */
> +static void g729_mem_update(const int16_t *fc_v, int16_t gp, int16_t gc, int16_t* exc, int subframe_size)
update_excitation()
[...]
> +
> +/**
> + * \brief LP synthesis filter
> + * \param lp (Q12) filter coefficients
if so, rename lp to filter_coeffs
> + * \param in (Q0) input signal
> + * \param out [out] (Q0) output (filtered) signal
> + * \param filter_data [in/out] (Q0) filter data array (previous synthesis data)
> + * \param subframe_size length of subframe
> + * \param exit_on_overflow 1 - If overflow occured routine updates neither out nor
> + * filter data arrays, 0 - always update
> + *
> + * \return 1 if overflow occured, 0 - otherwise
> + *
> + * Routine applies 1/A(z) filter to given speech data
> + */
> +static int g729_lp_synthesis_filter(const int16_t* lp, const int16_t *in, int16_t *out, int16_t *filter_data, int subframe_size, int exit_on_overflow)
> +{
> + int16_t tmp_buf[MAX_SUBFRAME_SIZE+10];
> + int16_t* tmp=tmp_buf+10;
> + int i,n;
> + int sum;
> +
> + memcpy(tmp_buf, filter_data, 10*sizeof(int16_t));
> +
> + for(n=0; n<subframe_size; n++)
> + {
> + sum = in[n] << 12;
> + for(i=0; i<10; i++)
> + sum -= lp[i] * tmp[n-i-1];
> + sum >>= 12;
> + if(sum > SHRT_MAX || sum < SHRT_MIN)
> + {
> + if(exit_on_overflow)
> + return 1;
> + sum = av_clip_int16(sum);
> + }
> + tmp[n] = sum;
> + }
> +
> + memcpy(filter_data, tmp + subframe_size - 10, 10*sizeof(int16_t));
> + memcpy(out, tmp, subframe_size*sizeof(int16_t));
int i,n;
for(n=0; n<subframe_size; n++){
int sum = in[n] << 12;
for(i=0; i<10; i++)
sum -= filter_coeffs[i] * filter_data[9+n-i];
sum >>= 12;
if(sum > SHRT_MAX || sum < SHRT_MIN){
if(exit_on_overflow)
return 1;
sum = av_clip_int16(sum);
}
out[n] =
filter_data[10+n] = sum;
}
memcpy(filter_data, filter_data + subframe_size - 10, 10*sizeof(int16_t));
> +
> + return 0;
> +}
> +
> +/**
> + * \brief Adaptive gain control (4.2.4)
> + * \param gain_before (Q0) gain of speech before applying postfilters
> + * \param gain_after (Q0) gain of speech after applying postfilters
> + * \param speech [in/out] (Q0) signal buffer
> + * \param subframe_size length of subframe
> + * \param gain_prev previous value of gain coefficient
> + *
> + * \return last value of gain coefficient
> + */
> +static int16_t g729a_adaptive_gain_control(int gain_before, int gain_after, int16_t *speech, int subframe_size, int16_t gain_prev)
> +{
> + int gain; // Q12
> + int n;
> +
> + if(!gain_after)
> + return gain_prev;
> +
> + if(gain_before)
> + {
> + gain = l_div(gain_after, gain_before, 12); // Q12
> + gain = l_inv_sqrt(gain) >> 11; // Q23 -> Q12
> + }
> + else
> + gain = 0;
> +
> + for(n=0; n<subframe_size; n++)
> + {
> + // 0.9 * ctx->gain_coeff + 0.1 * gain
> + gain_prev = (29491 * gain_prev + 3276 * gain) >> 15;
the >>15 are done at the wrong spot
> + speech[n] = (speech[n] * gain_prev) >> 12;
> + }
> + return gain_prev;
> +}
> +
> +/**
> + * \brief Calculates coefficients of weighted A(z/GAMMA) filter
> + * \param Az (Q12) source filter
> + * \param gamma (Q15) weight coefficients
> + * \param Azg [out] (Q12) resulted weighted A(z/GAMMA) filter
> + *
> + * Azg[i]=GAMMA^i*Az[i] , i=0..subframe_size
> + */
> +static void g729a_weighted_filter(const int16_t* Az, int16_t gamma, int16_t *Azg)
What is gamma? This is not a greek book, gamma identifies a letter. But this
clearly is not about that letter.
[...]
> +/**
> + * \brief long-term postfilter (4.2.1)
> + * \param intT1 integer part of the pitch delay T1 in the first subframe
> + * \param residual (Q0) filtering input data
> + * \param residual_filt [out] (Q0) speech signal with applied A(z/GAMMA_N) filter
> + * \param subframe_size size of subframe
> + */
> +static void g729a_long_term_filter(int intT1, const int16_t* residual, int16_t *residual_filt, int subframe_size)
> +{
> + int k, n, intT0;
> + int gl; // gain coefficient for long-term postfilter
> + int corr_t0; // correlation of residual signal with delay intT0
> + int corr_0; // correlation of residual signal with delay 0
> + int correlation, corr_max;
> + int inv_glgp; // Q15 1.0/(1+gl*GAMMA_P)
> + int glgp_inv_glgp; // Q15 gl*GAMMA_P/(1+gl*GAMMA_P);
int coeff_a; // Q15 1 / (1 + gl*GAMMA_P)
int coeff_b; // Q15 gl*GAMMA_P / (1 + gl*GAMMA_P);
these names are just confusing i think
[...]
> + tmp = av_log2(FFMAX(corr_0, FFMAX(corr_t0, corr_max)));
add a FFMAX3(a,b,c) function
also send patch replacing `grep 'FFMAX(.*FFMAX' *` by it
> + if(tmp > 14)
> + {
> + corr_t0 >>= tmp - 14;
> + corr_0 >>= tmp - 14;
> + corr_max >>= tmp - 14;
> + }
> +
> + /* 4.2.1, Equation 82. check if filter should be disabled */
> + if(corr_max * corr_max < (corr_0 * corr_t0) >> 1)
> + gl = 0;
> + else if(!corr_t0 || corr_max > corr_t0)
> + gl = 32768; // 1.0 in Q15
> + else
> + gl=l_div(corr_max, corr_t0, 15);
> +
> + gl = (gl * GAMMA_P) >> 15;
> +
> + if (gl < -32768) // -1.0 in Q15
> + inv_glgp = 0;
> + else
> + inv_glgp = l_div(32768, 32768 + gl, 15); // 1/(1+gl) in Q15
> +
> + glgp_inv_glgp = 32768 - inv_glgp; // 1.0 in Q15
shouldnt this be 32767?
> +
> + /* 4.2.1, Equation 78, reconstruct delayed signal */
> + for(n=0; n<subframe_size; n++)
> + residual_filt[n] = (residual[n + PITCH_MAX ] * inv_glgp +
> + residual[n + PITCH_MAX - intT0] * glgp_inv_glgp) >> 15;
The position where the >> 15 is done is wrong i think
> +}
> +
> +/**
> + * \brief compensates the tilt in the short-term postfilter (4.2.3)
> + * \param ctx private data structure
> + * \param lp_gn (Q12) coefficients of A(z/GAMMA_N) filter
> + * \param lp_gd (Q12) coefficients of A(z/GAMMA_D) filter
> + * \param res_pst [in/out] (Q0) residual signal (partially filtered)
> +*/
> +static void g729a_tilt_compensation(G729A_Context *ctx, const int16_t *lp_gn, const int16_t *lp_gd, int16_t* res_pst)
> +{
> + int tmp;
> + int gt; // Q12
> + int rh1,rh0; // Q12
> + int16_t hf_buf[11+22]; // Q12 A(Z/GAMMA_N)/A(z/GAMMA_D) filter impulse response
> + int sum;
> + int i, n;
> +
> + memset(hf_buf, 0, 33 * sizeof(int16_t));
> +
> + hf_buf[10] = 4096; //1.0 in Q12
> + for(i=0; i<10; i++)
> + hf_buf[i+11] = lp_gn[i];
> +
> + /* Apply 1/A(z/GAMMA_D) filter to hf */
> + for(n=0; n<22; n++)
> + {
> + sum=hf_buf[n+10];
> + for(i=0; i<10; i++)
> + sum -= (lp_gd[i] * hf_buf[n+10-i-1]) >> 12;
> + hf_buf[n+10] = sum;
> + }
for(n=0; n<22; n++){
sum= lp_gn[n];
for(i=0; i<10; i++)
sum -= (lp_gd[i] * hf_buf[n+10-i-1]) >> 12;
hf_buf[n+10] = sum;
}
> +
> + /* Now hf_buf (starting with 10) contains impulse response of A(z/GAMMA_N)/A(z/GAMMA_D) filter */
> +
> + /* A.4.2.3, Equation A.14, calcuate rh(0) */
> + rh0 = sum_of_squares(hf_buf+10, 22, 0, 0) >> 12; // Q24 -> Q12
> +
> + /* A.4.2.3, Equation A.14, calcuate rh(1) */
> + rh1 = sum_of_squares(hf_buf+10, 22-1, 1, 0) >> 12; // Q24 -> Q12
> +
> + rh1 = rh1 * GAMMA_T >> 15; // Q12 * Q15 = Q27 -> Q12
> +
> + /* A.4.2.3, Equation A.14 */
> + if(rh1>0)
> + gt = -l_div(rh1, rh0, 12); // l_div accepts only positive parameters
The comment is wrong
> + else
> + gt = 0;
> +
and a FFMAX(0, ) can achive this cleaner
[...]
> +/**
> + * \brief Residual signal calculation (4.2.1)
> + * \param lp (Q12) A(z/GAMMA_N) filter coefficients
if lp are filter coeffs name the variable appropriately
> + * \param speech (Q0) input speech data
> + * \param residual [out] (Q0) output data filtered through A(z/GAMMA_N)
> + * \param subframe_size size of one subframe
> + * \param pos_filter_data [in/out] (Q0) speech data of previous subframe
> + */
> +static void g729_residual(int16_t* lp, const int16_t* speech, int16_t* residual, int subframe_size, int16_t* pos_filter_data)
> +{
> + int i, n, sum;
> + int16_t tmp_speech_buf[MAX_SUBFRAME_SIZE+10];
> + int16_t *tmp_speech=tmp_speech_buf+10;
> +
> + // Copy data from previous frame
> + for(i=0; i<10; i++)
> + tmp_speech[-10+i] = pos_filter_data[i];
> +
> + // Copy the rest of speech data
> + for(i=0; i<subframe_size; i++)
> + tmp_speech[i] = speech[i];
> + /*
> + 4.2.1, Equation 79 Residual signal calculation
> + ( filtering through A(z/GAMMA_N) , one half of short-term filter)
> + */
> + for(n=0; n<subframe_size; n++)
> + {
> + sum = tmp_speech[n] << 12;
> + for(i=0; i<10; i++)
> + sum += lp[i] * tmp_speech[n-i-1];
> + sum = av_clip(sum, SHRT_MIN << 12, SHRT_MAX << 12);
> + residual[n+PITCH_MAX] = g729_round(sum << 4);
> + }
> +
> + // Save data to use it in the next subframe
> + for(i=0; i<10; i++)
> + pos_filter_data[i] = speech[subframe_size-10+i];
there are 3 memcpy() in there please get rid of 2 of them
the same applies to other routines which similarly are full of memcpys
get rid of ALL unneeded mempcpy()
[...]
> +/**
> + * \brief high-pass filtering and upscaling (4.2.5)
> + * \param ctx private data structure
> + * \param speech [in/out] reconstructed speech signal for applying filter to
> + * \param length size of input data
> + *
> + * Filter has cut-off frequency 100Hz
> + */
> +static void g729_high_pass_filter(G729A_Context* ctx, int16_t* speech, int length)
> +{
> + int i;
> +
> + for(i=0; i<length; i++)
> + {
> + memmove(ctx->hpf_z+1, ctx->hpf_z, 2*sizeof(ctx->hpf_z[0]));
> + ctx->hpf_z[0]=speech[i];
> +
> + ctx->hpf_f[0] = mul_24_15(ctx->hpf_f[1], 15836)
> + + mul_24_15(ctx->hpf_f[2], -7667)
vertical align
[...]
> +/**
> + * \brief save quantized LSF coefficients for using in next frame
> + * \param ctx context structure
> + * \param lq quantized LSF coefficients
> + */
> +static void g729_lq_rotate(G729A_Context *ctx, int* lq)
> +{
> + int i, k;
> +
> + /* Rotate lq_prev */
> + for(i=0; i<10; i++)
> + {
> + for(k=MA_NP-1; k>0; k--)
> + ctx->lq_prev[k][i] = ctx->lq_prev[k-1][i];
> +
> + ctx->lq_prev[0][i] = lq[i];
> + }
reorder and use memcpy()
also pass the arrays not the context as parameters, this is cleaner IMHO
and makes it more clear what is changed
[...]
> +/**
> + * \brief Decode LSP coefficients from L0-L3 (3.2.4)
> + * \param ctx private data structure
> + * \param L0 Switched MA predictor of LSP quantizer
> + * \param L1 First stage vector of quantizer
> + * \param L2 Second stage lower vector of LSP quantizer
> + * \param L3 Second stage higher vector of LSP quantizer
I do not like the variable names at all, these are indexes into VQ tables
nothing in their name nor comment gives a hint to that.
> + * \param lsfq [out] (Q13) Decoded LSP coefficients
> + */
> +static void g729_lsf_decode(G729A_Context* ctx, int16_t L0, int16_t L1, int16_t L2, int16_t L3, int16_t* lsfq)
> +{
> + int i,j,k;
> + int16_t J[2]={10, 5}; //Q13
find a better name, also this should be static const uint8_t
[...]
> + /*
> + Pitch gain of previous subframe.
> +
> + (EE) This does not comply with specification, but reference
> + and Intel decoder uses here minimum sharpen value instead of maximum. */
> + ctx->pitch_sharp = SHARP_MIN;
The comment should be somehow changed so it is more seperate from the code
that is * before the lines or indented.
[...]
> + /* LSP coefficients */
> + for(i=0; i<10; i++)
> + ctx->lq_prev[0][i]=lq_init[i];
[...]
> + for(k=1; k<MA_NP; k++)
> + for(i=0; i<10; i++)
> + ctx->lq_prev[k][i]=ctx->lq_prev[0][i];
This can be done simpler and cleaner.
[...]
> +/**
> + * \brief decode one G.729 frame into PCM samples
> + * \param ctx private data structure
> + * \param out_frame array for output PCM samples
> + * \param out_frame_size maximum number of elements in output array
> + * \param parm decoded parameters of the codec
> + * \param frame_erasure frame erasure flag
> + *
> + * \return 2 * subframe_size
> + */
> +static int g729a_decode_frame_internal(G729A_Context* ctx, int16_t* out_frame, int out_frame_size, G729_parameters *parm, int frame_erasure)
> +{
> + int16_t lp[20]; // Q12
> + int16_t lsp[10]; // Q15
> + int16_t lsf[10]; // Q13
> + int pitch_delay_3x; // pitch delay, multiplied by 3
> + int pitch_delay_int; // pitch delay, integer part
> + int16_t fc[MAX_SUBFRAME_SIZE]; // fixed codebooc vector
> + int i, j;
> +
> + ctx->data_error = frame_erasure;
whichever it is erasure (missing/droped frame) != error
figure it out and name everything consistently
[...]
> + if(!i)
> + {
> + // Decoding of the adaptive-codebook vector delay for first subframe (4.1.3)
> + if(ctx->bad_pitch || ctx->data_error)
> + pitch_delay_3x = 3 * ctx->pitch_delay_int_prev + 1;
> + else
> + {
> + if(parm->ac_index[i] >= 197)
> + pitch_delay_3x = 3 * parm->ac_index[i] - 335;
> + else
> + pitch_delay_3x = parm->ac_index[i] + 59;
not sure but somehow the follow looks nicer to me
pitch_delay_3x = parm->ac_index[i] + 59;
if(pitch_delay_3x > 255)
pitch_delay_3x = 3 * pitch_delay_3x - 512;
> + }
> + }
> + else
> + {
> + // Decoding of the adaptive-codebook vector delay for second subframe (4.1.3)
> + if(ctx->data_error)
> + pitch_delay_3x = 3 * ctx->pitch_delay_int_prev + 1;
> + else
> + pitch_delay_3x = parm->ac_index[i] +
> + 3 * av_clip(ctx->pitch_delay_int_prev-5, PITCH_MIN, PITCH_MAX-9) - 1;
> + }
the error case is the same and can be factored out
> + pitch_delay_int = pitch_delay_3x / 3;
> +
> + g729_decode_ac_vector(pitch_delay_int, (pitch_delay_3x%3)-1,
> + ctx->exc + i*ctx->subframe_size, ctx->subframe_size);
spliting it in int & frac just to then fix the resulting mess in an if()
in the function is not good, pass pitch_delay_3x and correctly split it
[...]
> + g729_update_gain_erasure(ctx->pred_energ_q);
> + }
> + else
> + {
> + // Decoding of the fixed codebook gain (4.1.5 and 3.9.1)
> + ctx->gain_pitch = cb_GA[parm->ga_cb_index[i]][0] + cb_GB[parm->gb_cb_index[i]][0];
> +
> + ctx->gain_code = g729_get_gain_code(parm->ga_cb_index[i],
> + parm->gb_cb_index[i],
besides the very ugly misalignment of the parameters
it woudl be MUCH cleaer to look up all values outside, not look up the first 2
outside and the next 2 inside the function
[...]
> + parm->ma_predictor = get_bits(&gb, L0_BITS); //L0
> + parm->quantizer_1st = get_bits(&gb, L1_BITS); //L1
> + parm->quantizer_2nd_lo = get_bits(&gb, L2_BITS); //L2
> + parm->quantizer_2nd_hi = get_bits(&gb, L3_BITS); //L3
> +
> + parm->ac_index[0] = get_bits(&gb, P1_BITS); //P1
> + parm->parity = get_bits(&gb, P0_BITS); //P0 (parity)
> + parm->fc_indexes[0] = get_bits(&gb, FC_BITS(ctx)); //C1
> + parm->pulses_signs[0] = get_bits(&gb, FC_PULSE_COUNT); //S1
> + parm->ga_cb_index[0] = get_bits(&gb, GA_BITS); //GA1
> + parm->gb_cb_index[0] = get_bits(&gb, GB_BITS); //GB1
> +
> + parm->ac_index[1] = get_bits(&gb, P2_BITS); //P2
> + parm->fc_indexes[1] = get_bits(&gb, FC_BITS(ctx)); //C2
> + parm->pulses_signs[1] = get_bits(&gb, FC_PULSE_COUNT); //S2
> + parm->ga_cb_index[1] = get_bits(&gb, GA_BITS); //GA2
> + parm->gb_cb_index[1] = get_bits(&gb, GB_BITS); //GB2
reaname all the LPFG_1234_BITS to names describing their content!
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080323/4eb171b7/attachment.pgp>
More information about the ffmpeg-devel
mailing list