[FFmpeg-devel] [PATCH] G.729A (floating-point) decoder
Michael Niedermayer
michaelni
Sat Mar 1 00:12:33 CET 2008
On Fri, Feb 29, 2008 at 01:36:01AM +0600, Vladimir Voroshilov wrote:
> Hi, Michael
>
> Let's start second roundup of patch cleanup procedure.
> I want to collect issues for fixing at weekend :)
>
[...]
> +///Switched MA predictor of LSP quantizer (size in bits)
> +#define L0_BITS 1
> +///First stage vector of quantizer (size in bits)
> +#define L1_BITS 7
> +///First stage lowervector of quantizer (size in bits)
> +#define L2_BITS 5
> +///First stage hihjer vector of quantizer (size in bits)
> +#define L3_BITS 5
> +///Adaptive codebook index for first subframe (size in bits)
> +#define P1_BITS 8
> +///Adaptive codebook index for second subframe (size in bits)
> +#define P2_BITS 5
> +///Parity bit for pitch delay (size in bits)
> +#define P0_BITS 1
> +/// GA codebook index (size in bits)
> +#define GA_BITS 3
> +/// GB codebook index (size in bits)
> +#define GB_BITS 4
> +/// Number of pulses in fixed-codebook vector
> +#define FC_PULSE_COUNT 4
> +
> +///Size of parameters vector
> +#define VECTOR_SIZE 15
> +
> +static const struct
> +{
> + int sample_rate;
> + ///Size (in bytes) of input frame
> + uint8_t input_frame_size;
> + ///Size (in bytes) of output frame
> + uint8_t output_frame_size;
> + ///Size (in bits) of fixed codebook index
> + uint8_t fc_index_bits;
> +} formats[] =
please put the doxygen comments to the right of variables
[...]
> +static const float interp_filter[31] =
> +{
> + 0.898517,
> + 0.769271, 0.448635, 0.095915,
> + -0.134333, -0.178528, -0.084919,
> + 0.036952, 0.095533, 0.068936,
> + -0.000000, -0.050404, -0.050835,
> + -0.014169, 0.023083, 0.033543,
> + 0.016774, -0.007466, -0.019340,
> + -0.013755, 0.000000, 0.009400,
> + 0.009029, 0.002381, -0.003658,
> + -0.005027, -0.002405, 0.001050,
> + 0.002780, 0.002145, 0.000000
> +};
do we loose any PSNR if this is replaced by the integer one?
also it could be a [3][10] array for simpler access
[...]
> +/**
> + * Initial LSP values
> + */
> +static const float lsp_init[10] =
> +{
> + 0.9595, 0.8413, 0.6549, 0.4154, 0.1423, -0.1423, -0.4154, -0.6549, -0.8413, -0.9595,
> +};
fits in an int16_t
[...]
> +static float sum_of_squares(float *speech, int length)
> +{
> + int n;
> + float sum;
> +
> + if(!length)
> + return 0;
> +
> + sum=speech[0]*speech[0];
> + for(n=1; n<length; n++)
> + sum+=speech[n]*speech[n];
> +
> + return sum;
> +}
static float sum_of_squares(float *speech, int length)
{
int n;
float sum=0;
for(n=0; n<length; n++)
sum+=speech[n]*speech[n];
return sum;
}
> +
> +/**
> + * \brief pseudo random number generator
> + */
> +static inline uint16_t g729_random(G729A_Context* ctx)
> +{
> + return ctx->rand_value = 31821 * (uint32_t)ctx->rand_value + 13849;
> +}
remove the cast
[...]
> +/**
> + * \brief Decoding of the adaptive-codebook vector delay for second subframe (4.1.3)
> + * \param ctx private data structure
> + * \param ac_index Adaptive codebook index for second subframe
> + * \param intT1 first subframe's pitch delay integer part
> + *
> + * \return 3*intT+frac+1, where
> + * intT integer part of delay
> + * frac fractional part of delay [-1, 0, 1]
> + */
> +static int g729_decode_ac_delay_subframe2(G729A_Context* ctx, int ac_index, int intT1)
> +{
> + if(ctx->data_error)
> + return 3*intT1+1;
s/intT1/whatever sane name it has elsewhere/
> +
> + return ac_index + 3*FFMIN(FFMAX(intT1-5, PITCH_MIN), PITCH_MAX-9) - 1;
> +}
> +
> +/**
> + * \brief Decoding of the adaptive-codebook vector (4.1.3)
> + * \param ctx private data structure
> + * \param pitch_delay_int pitch delay, integer part
> + * \param pitch_delay_frac pitch delay, fraction part [-1, 0, 1]
> + * \param ac_v buffer to store decoded vector into
> + */
> +static void g729_decode_ac_vector(G729A_Context* ctx, int pitch_delay_int, int pitch_delay_frac, float* ac_v)
> +{
> + int n, i;
> + float v;
> +
> + //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]
t ? k? there are no t and k
> + for(n=0; n<ctx->subframe_size; n++)
> + {
> + /* 3.7.1, Equation 40 */
> + v=0;
> + for(i=0; i<10; i++)
> + {
> + /* R(x):=ac_v[-k+x] */
> + v+=ac_v[n-pitch_delay_int-i]*interp_filter[pitch_delay_frac+3*i]; //R(n-i)*b30(t+3i)
> + v+=ac_v[n-pitch_delay_int+i+1]*interp_filter[3-pitch_delay_frac+3*i]; //R(n+i+1)*b30(3-t+3i)
vertical align
[...]
> +/**
> + * \brief fixed codebook vector modification if delay is less than 40 (4.1.4 and 3.8)
> + * \param pitch_delay integer part of pitch delay
> + * \param fc_v [in/out] fixed codebook vector to change
> + *
> + * \remark if pitch_delay>=subframe_size no changes to vector are made
> + */
> +static void g729_fix_fc_vector(G729A_Context *ctx, int pitch_delay, float* fc_v)
> +{
> + int i;
> +
> + if(pitch_delay>=ctx->subframe_size)
> + return;
> +
> + for(i=pitch_delay; i<ctx->subframe_size;i++)
> + fc_v[i] += fc_v[i-pitch_delay]*ctx->gain_pitch;
> +}
the if() is useless
gain_pitch and subframe_size could be passed as arguments intead of ctx
> +
> +/**
> + * \brief Decoding of the adaptive and fixed codebook gains from previous subframe (4.4.2)
> + * \param ctx private data structure
> + * \param gp pointer to variable receiving quantized fixed-codebook gain (gain pitch)
> + * \param gc pointer to variable receiving quantized adaptive-codebook gain (gain code)
> + */
> +static void g729_get_gain_from_previous(G729A_Context *ctx, float* gp, float* gc)
> +{
> + /* 4.4.2, Equation 93 */
> + *gc = 0.98*ctx->gain_code;
> + ctx->gain_code = *gc;
> +
> + /* 4.4.2, Equation 94 */
> + *gp = FFMIN(0.9*ctx->gain_pitch, 0.9);
> + ctx->gain_pitch = *gp;
> +}
could you please avoid writing into random things of the context in functions.
in this case the 2 independant equations should just be inlined.
> +
> +/**
> + * \brief Attenuation of the memory of the gain predictor (4.4.3)
> + * \param ctx private data structure
> + */
> +static void g729_update_gain(G729A_Context *ctx)
the pointer to the modified array should be passed as argument instead of ctx.
> +{
> + float avg_gain=0;
> + int i;
> +
> + /* 4.4.3. Equation 95 */
> + for(i=0; i<4; i++)
> + avg_gain+=ctx->pred_vect_q[i];
> +
> + avg_gain = FFMAX(avg_gain * 0.25 - 4.0, -14);
> +
> + for(i=3; i>0; i--)
> + ctx->pred_vect_q[i]=ctx->pred_vect_q[i-1];
> + ctx->pred_vect_q[0]=avg_gain;
float avg_gain= pred_vect_q[3];
for(i=3; i>0; i--){
avg_gain += pred_vect_q[i-1];
pred_vect_q[i]= pred_vect_q[i-1];
}
pred_vect_q[0]= FFMAX(avg_gain * 0.25 - 4.0, -14);
> +}
> +
> +/**
> + * \brief Decoding of the adaptive and fixed codebook gains (4.1.5 and 3.9.1)
> + * \param ctx private data structure
> + * \param GA Gain codebook (stage 2)
> + * \param GB Gain codebook (stage 2)
> + * \param fc_v fixed-codebook vector
> + * \param gp pointer to variable receiving quantized fixed-codebook gain (gain pitch)
> + * \param gc pointer to variable receiving quantized adaptive-codebook gain (gain code)
> + */
> +static void g729_get_gain(G729A_Context *ctx, int nGA, int nGB, float* fc_v, float* gp, float* gc)
doxygen and parameters dont match
[...]
> + /* 3.9.1, Equation 74 */
> + *gc = energy*(cb1_sum); //quantized fixed-codebook gain (gain pitch)
useless ()
[...]
> +
> +/**
> + * \brief Calculates coefficients of weighted A(z/GAMMA) filter
> + * \param Az source filter
> + * \param gamma weight coefficients
> + * \param Azg resulted weighted A(z/GAMMA) filter
> + *
> + * Azg[i]=GAMMA^i*Az[i] , i=0..subframe_size
> + *
> + */
> +static void g729a_weighted_filter(float* Az, float gamma, float *Azg)
const float* Az
> +{
> + float gamma_pow;
> + int n;
> +
> + gamma_pow=gamma;
declaration and initialization can be marged
[...]
> +/**
> + * \brief long-term postfilter (4.2.1)
> + * \param ctx private data structure
> + * \param intT1 integer part of the pitch delay T1 in the first subframe
> + * \param residual_filt speech signal with applied A(z/GAMMA_N) filter
> + */
> +static void g729a_long_term_filter(G729A_Context *ctx, int intT1, float *residual_filt)
> +{
> + int k, n, intT0;
> + float gl; ///< gain coefficient for long-term postfilter
> + float corr_t0; ///< correlation of residual signal with delay intT0
> + float corr_0; ///< correlation of residual signal with delay 0
> + float correlation, corr_max;
> + float inv_glgp;///< 1.0/(1+gl*GAMMA_P)
> + float glgp_inv_glgp; ///< gl*GAMMA_P/(1+gl*GAMMA_P);
I already said that doxygen comments are not for local variables!
> +
> + /* A.4.2.1 */
> + int minT0=FFMIN(intT1, PITCH_MAX-3)-3;
> + int maxT0=FFMIN(intT1, PITCH_MAX-3)+3;
> + /* Long-term postfilter start */
> +
> + /*
> + First pass: searching the best T0 (pitch delay)
> + Second pass is not used in G.729A: fractional part is always zero
> + */
> + intT0=minT0;
> + corr_max=INT_MIN;
> + for(k=minT0; k<=maxT0; k++)
> + {
> + correlation=0;
> + /* 4.2.1, Equation 80 */
> + for(n=0; n<ctx->subframe_size; n++)
> + correlation+=ctx->residual[n+PITCH_MAX]*ctx->residual[n+PITCH_MAX-k];
> + if(correlation>corr_max)
> + {
> + corr_max=correlation;
> + intT0=k;
> + }
> + }
> +
> + corr_t0=sum_of_squares(ctx->residual+PITCH_MAX-intT0, ctx->subframe_size);
> + corr_0=sum_of_squares(ctx->residual+PITCH_MAX, ctx->subframe_size);
vertical align, also if you add a int offset as parameter then this could be
used in the loop above as well
> +
> + /* 4.2.1, Equation 82. checking if filter should be disabled */
> + if(corr_max*corr_max < 0.5*corr_0*corr_t0)
> + gl=0;
> + else if(!corr_t0)
> + gl=1;
> + else
> + gl=FFMIN(corr_max/corr_t0, 1);
> +
> + inv_glgp = 1.0 / (1 + gl*GAMMA_P);
> + glgp_inv_glgp = gl * GAMMA_P * inv_glgp;
g1 *= GAMMA_P;
inv_glgp = 1.0 / (1 + gl);
glgp_inv_glgp = g1 / (1 + gl);
> +
> + /* 4.2.1, Equation 78, reconstructing delayed signal */
> + for(n=0; n<ctx->subframe_size; n++)
> + residual_filt[n]=ctx->residual[n+PITCH_MAX]*inv_glgp+ctx->residual[n+PITCH_MAX-intT0]*glgp_inv_glgp;
use spaces, please, and format things in a slightly more readable way and
not onle here ...
residual_filt[n]= ctx->residual[n+PITCH_MAX ]*inv_glgp
+ ctx->residual[n+PITCH_MAX-intT0]*glgp_inv_glgp;
> +
> + //Shift residual for using in next subframe
> + memmove(ctx->residual, ctx->residual+ctx->subframe_size, PITCH_MAX*sizeof(float));
> +}
> +
> +/**
> + * \brief compensates the tilt in the short-term postfilter (4.2.3)
> + * \param ctx private data structure
> + * \param lp_gn coefficients of A(z/GAMMA_N) filter
> + * \param lp_gd coefficients of A(z/GAMMA_D) filter
> + * \param res_pst residual signal (partially filtered)
> +*/
> +static void g729a_tilt_compensation(G729A_Context *ctx,float *lp_gn, float *lp_gd, float* res_pst)
> +{
> + float tmp;
> + float gt,k,rh1,rh0;
> + float hf[22]; // A(Z/GAMMA_N)/A(z/GAMMA_D) filter impulse response
> + float tmp_buf[11+22];
> + float sum;
> + int i, n;
> +
> + hf[0]=1;
> + for(i=0; i<10; i++)
> + hf[i+1]=lp_gn[i];
> +
> + for(i=11; i<22;i++)
> + hf[i]=0;
> +
> + /* Applying 1/A(z/GAMMA_D) to hf */
> + for(i=0; i<10; i++)
> + tmp_buf[i]=hf[i+11];
hf[i+11] == 0 here
> +
> + for(n=0; n<22; n++)
> + {
> + sum=hf[n];
> + for(i=0; i<10; i++)
> + sum -= lp_gd[i]*tmp_buf[n-i-1+10];
> + tmp_buf[n+10]=sum;
> + hf[n]=sum;
> + }
tmp_buf and hf are redundant please remove one
> +
> + /* Now hf contains impulse response of A(z/GAMMA_N)/A(z/GAMMA_D) filter */
> +
> + /* A.4.2.3, Equation A.14, calcuating rh(0) */
> + rh0=0;
> + for(i=0; i<22; i++)
> + rh0+=hf[i]*hf[i];
> +
> + /* A.4.2.3, Equation A.14, calcuating rh(1) */
> + rh1=0;
> + for(i=0; i<22-1; i++)
> + rh1+=hf[i]*hf[i+1];
> +
> + /* A.4.2.3, Equation A.14 */
> + k=-rh1/rh0;
> +
> + if(k>=0)
> + gt=0;
> + else
> + gt=GAMMA_T*k;
gt= -GAMMA_T * FFMAX(rh1 / rh0, 0);
[...]
> +/**
> + * \brief Signal postfiltering (4.2, with A.4.2 simplification)
> + * \param ctx private data structure
> + * \param lp LP filter coefficients
> + * \param intT1 integer part of the pitch delay T1 of the first subframe
why not call it pitch_delay_int?
[...]
> +/**
> + * \brief high-pass filtering and upscaling (4.2.5)
> + * \param ctx private data structure
> + * \param speech reconstructed speech signal for applying filter to
> + *
> + * Filter has cut-off frequency 100Hz
> + */
> +static void g729_high_pass_filter(G729A_Context* ctx, float* speech)
> +{
> + float z_2=0;
> + float f_0=0;
> + int i;
> +
> + for(i=0; i<ctx->subframe_size; i++)
> + {
> + z_2=ctx->hpf_z1;
> + ctx->hpf_z1=ctx->hpf_z0;
> + ctx->hpf_z0=speech[i];
> +
> + f_0 = 1.9330735 * ctx->hpf_f1 - 0.93589199 * ctx->hpf_f2 +
> + 0.93980581 * ctx->hpf_z0 - 1.8795834 * ctx->hpf_z1 + 0.93980581 * z_2;
f_0 = 1.9330735 * ctx->hpf_f1
- 0.93589199 * ctx->hpf_f2
+ 0.93980581 * (ctx->hpf_z0 + z_2)
- 1.8795834 * ctx->hpf_z1;
[...]
> + for(i=0; i<ctx->subframe_size; i++)
> + {
> + tmp_speech[i] = FFMIN(tmp_speech[i], 32767.0);
> + tmp_speech[i] = FFMAX(tmp_speech[i], -32768.0);
> + speech[i]=lrintf(tmp_speech[i]);
the MAX/MIN should be after the lrintf()
> + }
> +}
> +
> +/**
> + * \brief Convert LSF to LSP
> + * \param ctx private data structure
> + * \param lsf LSF coefficients
> + * \param lsp LSP coefficients
> + *
> + * \remark It is safe to pass the same array in lsf and lsp parameters
> + */
> +static void g729_lsf2lsp(G729A_Context *ctx, float *lsf, float *lsp)
const float *lsf;
> +{
> + int i;
> +
> + /* Convert LSF to LSP */
> + for(i=0;i<10; i++)
> + lsp[i]=cos(lsf[i]);
> +}
cos() can be very slow (depending on hardware)
please use the LUT based solution from the integer implementation
[...]
> +/**
> + * \brief Decode LP coefficients from L0-L3 (3.2.4)
LP or LSP ?
> + * \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
> + * \param lsfq Decoded LSP coefficients
> + */
> +static void g729_lsp_decode(G729A_Context* ctx, int16_t L0, int16_t L1, int16_t L2, int16_t L3, float* lsfq)
[...]
> + /* 3.2.4, Equation 20 */
> + for(i=0; i<10; i++)
> + {
> + lsfq[i]=lq[i] * ma_predictor_sum[L0][i] / Q13_BASE;
> + for(k=0; k<MA_NP; k++)
> + lsfq[i] += (ctx->lq_prev[k][i] * ma_predictor[L0][k][i]); //Q15
> + lsfq[i] /= Q15_BASE;
> + //Saving LSF for using when error occured in next frames
> + ctx->lsf_prev[i]=lsfq[i];
> + }
tabs are forbidden in svn
the Q13_BASE downscale is done at the wrong point, see the integer
implementation.
lq_prev and lsf_prev can be changed to integer as well.
[...]
> +
> +static void get_lsp_coefficients(float* q, float* f)
const float *q or even better const float *lsp
> +{
> + int i, j;
> + int qidx=2;
> + float b;
> +
> + f[0]=1.0;
> + f[1]=-2*q[0];
> +
> + for(i=2; i<=5; i++)
> + {
> + b=-2*q[qidx];
> + f[i] = b*f[i-1] + 2*f[i-2];
> +
> + for(j=i-1; j>1; j--)
> + {
> + f[j] += b*f[j-1] + f[j-2];
> + }
f[i] = f[i-2];
for(j=i; j>1; j--)
f[j] += b*f[j-1] + f[j-2];
> + f[1]+=b;
> + qidx+=2;
> + }
as qidx=2*i-2 theres no need for a qidx variable
> +}
> +/**
> + * \brief LSP to LP conversion (3.2.6)
> + * \param ctx private data structure
> + * \param lsp LSP coefficients
> + * \param lp decoded LP coefficients
indicate what is input and output please
> + */
> +static void g729_lsp2lp(G729A_Context* ctx, float* lsp, float* lp)
const float *lsp
[...]
> +/**
> + * \brief interpolate LSP end decode LP for both first and second subframes (3.2.5 and 3.2.6)
Interpolate LSP for the first subframe and convert LSP -> LP for both subframes.
> + * \param ctx private data structure
> + * \param lsp_curr current LSP coefficients
@param lsp_2nd LSP coefficients of the 2nd subframe
> + * \param lp [out] decoded LP coefficients
> + */
> +static void g729_lp_decode(G729A_Context* ctx, float* lsp_curr, float* lp)
> +{
> + float lsp[10];
lsp_1st[10];
[...]
> +/**
> + * \brief G.729A decoder initialization
> + * \param ctx private data structure
> + * \return 0 if success, non-zero otherwise
> + */
> +static int ff_g729a_decoder_init(AVCodecContext * avctx)
param ctx ?
> +{
> + G729A_Context* ctx=avctx->priv_data;
> + int i,k;
> +
> + if(avctx->sample_rate==8000)
> + ctx->format=0;
> +#ifdef G729_SUPPORT_4400
> + else if (avctx->sample_rate==4400)
> + ctx->format=1;
> +#endif
> + else
> + {
> + av_log(avctx, AV_LOG_ERROR, "Sample rate %d is not supported\n", avctx->sample_rate);
> + return AVERROR_NOFMT;
> + }
> + ctx->subframe_size=formats[ctx->format].output_frame_size>>2; // cnumber of 2-byte long samples in one subframe
^^^^^^^
whats a cnumber?
[...]
> + * \param serial array if bits (0x81 - 1, 0x7F -0)
> + * \param serial_size number of items in array
> + * \param out_frame array for output PCM samples
> + * \param out_frame_size maximum number of elements in output array
> + */
> +static int g729a_decode_frame_internal(void* context, int16_t* out_frame, int out_frame_size, int *parm)
The parameters documented are not arguments to this function (serial, ...)
and the context should have a proper type.
> +{
> + G729A_Context* ctx=context;
> + float lp[20];
> + float lsp[10];
> + int pitch_delay; ///< pitch delay
> + float fc[MAX_SUBFRAME_SIZE]; ///< fixed codebooc vector
> + float gp, gc;
> + int intT1;
> +
> + ctx->data_error=0;
> + ctx->bad_pitch=0;
> +
> + if(!g729_parity_check(parm[4], parm[5]))
> + ctx->bad_pitch=1;
ctx->bad_pitch = !g729_parity_check(parm[4], parm[5]);
> +
> + if(ctx->data_error)
> + g729_lsp_restore_from_previous(ctx, lsp);
> + else
> + g729_lsp_decode(ctx, parm[0], parm[1], parm[2], parm[3], lsp);
data_error cannot be != 0 here in your code.
> +
> + g729_lp_decode(ctx, lsp, lp);
> +
> + /* first subframe */
> + pitch_delay=g729_decode_ac_delay_subframe1(ctx, parm[4], ctx->intT2_prev);
> + intT1=pitch_delay/3; //Used in long-term postfilter
There are several occurances of pitch_delay/3 below which could be replaced by
intT1 (and please rename it to something more meaningfull).
> +
> + g729_decode_ac_vector(ctx, pitch_delay/3, (pitch_delay%3)-1, ctx->exc);
> +
> + if(ctx->data_error)
> + {
> + parm[6] = g729_random(ctx) & 0x1fff;
> + parm[7] = g729_random(ctx) & 0x000f;
> + }
> +
> + g729_decode_fc_vector(ctx, parm[6], parm[7], fc);
> + g729_fix_fc_vector(ctx, pitch_delay/3, fc);
> +
> + if(ctx->data_error)
> + {
> + g729_get_gain_from_previous(ctx, &gp, &gc);
> + g729_update_gain(ctx);
> + }
> + else
> + {
> + g729_get_gain(ctx, parm[8], parm[9], fc, &gp, &gc);
> + }
> + g729_mem_update(ctx, fc, gp, gc, ctx->exc);
> + g729_reconstruct_speech(ctx, lp, intT1, ctx->exc, out_frame);
> + ctx->subframe_idx++;
> +
> + /* second subframe */
> + pitch_delay=g729_decode_ac_delay_subframe2(ctx, parm[10], pitch_delay/3);
> +
> + ctx->intT2_prev=pitch_delay/3;
> + if(ctx->data_error)
> + ctx->intT2_prev=FFMIN(ctx->intT2_prev+1, PITCH_MAX);
> +
> + g729_decode_ac_vector(ctx, pitch_delay/3, (pitch_delay%3)-1, ctx->exc+ctx->subframe_size);
> +
> + if(ctx->data_error)
> + {
> + parm[11] = g729_random(ctx) & 0x1fff;
> + parm[12] = g729_random(ctx) & 0x000f;
> + }
> +
> + g729_decode_fc_vector(ctx, parm[11], parm[12], fc);
> + g729_fix_fc_vector(ctx, pitch_delay/3, fc);
> +
> + if(ctx->data_error)
> + {
> + g729_get_gain_from_previous(ctx, &gp, &gc);
> + g729_update_gain(ctx);
> + }
> + else
> + {
> + g729_get_gain(ctx, parm[13], parm[14], fc, &gp, &gc);
> + }
> + g729_mem_update(ctx, fc, gp, gc, ctx->exc+ctx->subframe_size);
> + g729_reconstruct_speech(ctx, lp+10, intT1, ctx->exc+ctx->subframe_size, out_frame+ctx->subframe_size);
> + ctx->subframe_idx++;
This code looks duplicated/unrolled, make a loop out of it please.
> +
> + //Save signal for using in next frame
> + memmove(ctx->exc_base, ctx->exc_base+2*ctx->subframe_size, (PITCH_MAX+INTERPOL_LEN)*sizeof(float));
> +
> + return 2*ctx->subframe_size;
> +}
> +
> +/**
> + * \brief decodes one G.729 frame (10 bytes long) into parameters vector
> + * \param ctx private data structure
s/private data structure/context/
> + * \param buf 10 bytes of decoder parameters
> + * \param buf_size size of input buffer
> + * \param int parm output vector of decoded parameters
^^^^^^^^
?
> + *
> + * \return 0 if success, nonzero - otherwise
> + */
> +static int g729_bytes2parm(G729A_Context *ctx, const uint8_t *buf, int buf_size, int *parm)
> +{
> + GetBitContext gb;
> + int l_frame=formats[ctx->format].input_frame_size;
> +
> + if(buf_size<l_frame)
> + return AVERROR(EIO);
> +
> + init_get_bits(&gb, buf, buf_size);
> +
> + parm[0]=get_bits(&gb, L0_BITS); //L0
> + parm[1]=get_bits(&gb, L1_BITS); //L1
> + parm[2]=get_bits(&gb, L2_BITS); //L2
> + parm[3]=get_bits(&gb, L3_BITS); //L3
> + parm[4]=get_bits(&gb, P1_BITS); //P1
> + parm[5]=get_bits(&gb, P0_BITS); //Parity
> + parm[6]=get_bits(&gb, formats[ctx->format].fc_index_bits*FC_PULSE_COUNT+1); //C1
> + parm[7]=get_bits(&gb, FC_PULSE_COUNT); //S1
> + parm[8]=get_bits(&gb, GA_BITS); //GA1
> + parm[9]=get_bits(&gb, GB_BITS); //GB1
> + parm[10]=get_bits(&gb, P2_BITS); //P2
parm[ 8]=get_bits(&gb, GA_BITS); //GA1
parm[ 9]=get_bits(&gb, GB_BITS); //GB1
parm[10]=get_bits(&gb, P2_BITS); //P2
also parm should be a struct and this should look like
parm->cb_index[0]= get_bits(&gb, GA_CB_INDEX_BITS); //GA1
parm->cb_index[1]= get_bits(&gb, GB_CB_INDEX_BITS); //GB1
...
Or some other appropriate names, 2 letter names are not very understanable.
[...]
> + int in_frame_size=formats[ctx->format].input_frame_size;
> + int out_frame_size=formats[ctx->format].output_frame_size;
vertical align
> + int i, ret;
> + uint8_t *dst=data;
> + const uint8_t *src=buf;
> +
> + if (buf_size<in_frame_size)
> + return AVERROR(EIO);
> +
> + *data_size=0;
You are now checking the input buffer size, but what is really important is
the output buffer size, its in data_size and you just overwrite this, this is
not ok. You must check that te buffer is large enough for what you write in
there.
> + for(i=0; i<buf_size; i+=in_frame_size)
Each packet the decoder receives should be 1 frame.
[...]
> + *data_size=(dst-(uint8_t*)data);
> + return (src-buf);
superflous ()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- 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/20080301/8316ad24/attachment.pgp>
More information about the ffmpeg-devel
mailing list