[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [2/7] - pitch lag decoding

Michael Niedermayer michaelni
Wed Jun 18 03:45:43 CEST 2008


On Fri, Jun 13, 2008 at 06:41:40PM +0700, Vladimir Voroshilov wrote:
> 2008/6/12 Michael Niedermayer <michaelni at gmx.at>:
> > On Thu, Jun 12, 2008 at 12:20:14AM +0700, Vladimir Voroshilov wrote:
[...]
> >> + *
> >> + * Pitch delay is coded:
> >
> > is it pitch delay or pitch lag? you mix the terms randomly ...
> 
> Hm. i thought "lag" is synonym for "delay" (as my dictionary says).
> Specification uses "delay" in most of text, while reference
> code uses "lag" everywhere.
> Tell me if i should replace all instances with "lag" or "delay" only
> (if so, tell also what is better and why, please)

ask diego, he is the master of english consistency.
Iam not a native english speaker and do not really know the difference
(if there is any) between lag and delay.


[...]

> +int ff_acelp_decode_lag3_1st_8_bits(int ac_index)

maybe:

ff_acelp_decode_8bit_to_1st_lag3
and
/* Decodes the 8bit ac_index to the pitch lag of the first frame in 1/3 units.



> +{
> +    ac_index += 58;
> +    if(ac_index > 254)
> +        ac_index = 3 * ac_index - 510;
> +    return ac_index;
> +}
> +
> +int ff_acelp_decode_lag3_2nd_8_4_bits(
> +        int ac_index,
> +        int pitch_lag_min)
> +{

> +    if(ac_index < 4)
> +        return 3*(ac_index + pitch_lag_min);
> +    else if(ac_index < 12)
> +        return 3 * pitch_lag_min + ac_index + 6;

spaces surrounding * are inconsistant


> +    else
> +        return 3 * (ac_index + pitch_lag_min) - 18;
> +}
> +
> +int ff_acelp_decode_lag3_2nd_8_56_bits(
> +        int ac_index,
> +        int pitch_lag_min)
> +{
> +        return 3 * pitch_lag_min + ac_index - 2;
> +}
> +

> +int ff_acelp_decode_lag6_9_6_bits(int ac_index)

shouldnt that have a 1st or 2nd in the name as well?


[...]
> +/**
> + * \brief Decode pitch lag of the first subframe encoded by 8 bits with 1/3
> + *        resolution.
> + * \param ac_index adaptive codebook index (8 bits)
> + *
> + * \return pitch lag in 1/3 units
> + *
> + * Pitch delay is coded:

> + *    with 1/3 resolution, 19 1/3 <= pitch_delay <= 84 2/3
                                            .    .
For some reason i hate int frac notation 19.3 84.6 seem clearer to me
but this is bikeshed, iam fine with int frac as well if you prefer ...
Though personally id either write fractions or decimal not integer + frac


> + *    integers only,           85 <= pitch_delay <= 143
> + */
> +int ff_acelp_decode_lag3_1st_8_bits(int ac_index);
> +
> +/**
> + * \brief Decode pitch lag of the second subframe encoded by 5 or 6 bits
> + *        with 1/3 precision.
> + * \param ac_index adaptive codebook index (5 or 6 bits)
> + * \param pitch_lag_min lower bound (integer) of pitch delay interval
> + *                      for second subframe
> + *
> + * \return pitch lag in 1/3 units
> + *
> + * Pitch delay is coded:
> + *    with 1/3 resolution, int(prev_pitch_delay) - 5 2/3 <= pitch_delay <= int(prev_pitch_delay) + 4 2/3

When i see this iam tempted to simplify it to 
- 5 2/3 <= pitch_delay - int(prev_pitch_delay) <= 4 2/3

not sure if its a good idea though


[...]
> +/**
> + * \brief Decode pitch lag of the second subframe encoded by 6 bits
> + *        with 1/6 precision.
> + * \param ac_index adaptive codebook index (6 bits)
> + * \param pitch_lag_min lower bound (integer) of pitch delay interval for
> + *                      second subframe
> + *
> + * \return pitch lag in 1/6 units
> + *
> + * Pitch delay is coded:
> + *    with 1/6 resolution, int(prev_pitch_delay) - 5 1/2 <= pitch_delay <= int(prev_pitch_delay) + 4 1/2
> + *

> + * This range is adapted for the cases where prev_pitch_delay straddles the boundaries of the
> + * delay range [18; 143].

Iam confused, this function does not do anything special about any boundary
and if its done outside why is it mentioned here ...


> + *
> + * \remark The routine is used in AMR @12.2k for the second and fourth subframes.
> + */
> +int ff_acelp_decode_lag6_2nd_9_6_bits(
> +        int ac_index,
> +        int pitch_lag_min);
> +

> +/**
> + * \brief attenuation of the memory of the gain predictor (4.4.3 of G.729)

that one sounds very odd as well


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20080618/4c976e36/attachment.pgp>



More information about the ffmpeg-devel mailing list