On Tue, Jul 19, 2011 at 08:48:07AM +0200, Rob wrote:
> On 15 July 2011 23:48, Diego Biurrun <[email protected]> wrote:
> > Somebody please doublecheck that I was not overeager with the unused
> > functions in those *celp*.c files, possibly they were intended to
> > fulfill some sort of purpose at some future point in time.
>
> Have you spoken to Vladimir Voroshilov about this? Maybe the changes
> needed to make it both compile and work are not large.
Yes, if you look through the ml, you fill find a short thread:
Subject: [libav-devel] vestigial G.729 decoder
Let me quickly quote my question and Vladimir's reply here:
> > We have the beginnings of a G.729 decoder scattered over
> >
> > libavcodec/g729dec.c
> > libavcodec/g729data.h
> >
> > and a bunch of #ifdef blocks in various other files.
> >
> > This was added by Vladimir Voroshilov in 2006, but never finished.
> > It did not even compile back then, much less today.
> >
> > Does this thing have any value or should we remove it?
>
> If noone wants to finish g729, i'm ok with removing those parts, since
> i don't have neither free time nor enough skills to done this work in
> visible future.
> If you insist on ripping it out (the code looks good to me) then
> perhaps it could be moved to some holding place rather than just
> removed.
I was going to reference it on the "interesting patches" section of the
multimedia wiki, is that enough for you?
> > --- a/libavcodec/acelp_pitch_delay.c
> > +++ b/libavcodec/acelp_pitch_delay.c
> > @@ -88,39 +88,6 @@ void ff_acelp_update_past_gain(
> > quant_energy[0] = (6165 * ((ff_log2(gain_corr_factor) >> 2) - (13
> > << 13))) >> 13;
> > }
> >
> > -int16_t ff_acelp_decode_gain_code(
> > - DSPContext *dsp,
> > - int gain_corr_factor,
> > - const int16_t* fc_v,
> > - int mr_energy,
> > - const int16_t* quant_energy,
> > - const int16_t* ma_prediction_coeff,
> > - int subframe_size,
> > - int ma_pred_order)
> > -{
> > - int i;
> > -
> > - mr_energy <<= 10;
> > -
> > - for(i=0; i<ma_pred_order; i++)
> > - mr_energy += quant_energy[i] * ma_prediction_coeff[i];
> > -
> > -#ifdef G729_BITEXACT
> > - mr_energy += (((-6165LL * ff_log2(dsp->scalarproduct_int16(fc_v, fc_v,
> > subframe_size, 0))) >> 3) & ~0x3ff);
> > -
> > - mr_energy = (5439 * (mr_energy >> 15)) >> 8; // (0.15) =
> > (0.15) * (7.23)
> > -
> > - return bidir_sal(
> > - ((ff_exp2(mr_energy & 0x7fff) + 16) >> 5) *
> > (gain_corr_factor >> 1),
> > - (mr_energy >> 15) - 25
> > - );
> > -#else
> > - mr_energy = gain_corr_factor * exp(M_LN10 / (20 << 23) * mr_energy) /
> > - sqrt(dsp->scalarproduct_int16(fc_v, fc_v, subframe_size,
> > 0));
> > - return mr_energy >> 12;
> > -#endif
> > -}
>
> Hmm, this function looks very familiar to me for AMR though my
> implementation was float, sipro appears to use a floating point
> version of this function and I know it is a core function of ACELP
> codecs so perhaps it is good to keep around.
>
> We were aiming to build up a collection of the common ACELP functions
> to reuse to quickly write other ACELP decoders. If removed it would be
> in the history but it would be difficult to know about or find for
> someone who didn't know it had been there.
I'll take that as a vote to keep it then.
> > - * @remark The routine is used in G.729 and AMR (all modes).
> > - */
> > -int16_t ff_acelp_decode_gain_code(
> > - DSPContext *dsp,
> > - int gain_corr_factor,
> > - const int16_t* fc_v,
> > - int mr_energy,
> > - const int16_t* quant_energy,
> > - const int16_t* ma_prediction_coeff,
> > - int subframe_size,
> > - int max_pred_order);
>
> Because of the remark, I would suggest leaving this where it is.
But it is not used in AMR, at least currently...
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
