[FFmpeg-devel] [PATCH] One more shared func for SIPR and AMR

[Michael Niedermayer]

On Sat, Nov 14, 2009 at 10:43:59PM +0100, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> On Wed, Nov 11, 2009 at 06:03:40PM +0100, Vitor Sessak wrote:
>>> $subj.
>>>
>>> -Vitor
>>>  acelp_pitch_delay.c |   48 
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  acelp_pitch_delay.h |   18 ++++++++++++++++++
>>>  2 files changed, 66 insertions(+)
>>> e0460b2f9e7167632ea6c730a18eb19f8b71e2f9  pitch_lag.diff
>>> Index: libavcodec/acelp_pitch_delay.c
>>> ===================================================================
>>> --- libavcodec/acelp_pitch_delay.c	(revision 20431)
>>> +++ libavcodec/acelp_pitch_delay.c	(working copy)
>>> @@ -140,3 +140,51 @@
>>>       return val;
>>>  }
>>> +
>>> +void ff_decode_pitch_lag(int *lag_int, int *lag_frac, int pitch_index,
>>> +                         const int prev_lag_int, const int subframe,
>>> +                         int third_as_first, int resolution)
>>> +{
>>> +    /* Note n * 10923 >> 15 is floor(x/3) for 0 <= n <= 32767 */
>>> +    if (subframe == 0 ||
>>> +        (subframe == 2 && third_as_first)) {
>>> +        if (pitch_index < 197) {
>>> +            *lag_int  = (pitch_index + 59) * 10923 >> 15;
>>> +            *lag_frac = pitch_index - *lag_int * 3 + 58;
>>> +        } else {
>>> +            *lag_int  = pitch_index - 112;
>>> +            *lag_frac = 0;
>>> +        }
>> maybe this function could be clearer if the int/frac split was factored 
>> out
>> and done at the end?
>> above could be something like:
>> if(pitch_index < 197) pitch_index += 59
>> else                  pitch_index = 3*pitch_index - 335
>> *lag_int  = pitch_index * 10923 >> 15;
>> *lag_frac = pitch_index - *lag_int * 3 - 1;
>> but this of course makes only sense if its not meassureable slower in
>> reality and actually clearer
>
> The function is not speed critical and this indeed makes the function less 
> obfuscated. New patch attached.
>
> -Vitor

>  acelp_pitch_delay.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  acelp_pitch_delay.h |   18 ++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 91e9914dac0744d77cd5877aba49d83a6271254e  pitch_lag2.diff

looks fine to me, maybe though some of out *CELP experts have comments too

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- 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/20091115/40b1d1f6/attachment.pgp>