[FFmpeg-devel] [PATCH] Long-term prediction for ALS

[Thilo Borgmann]

Ronald S. Bultje schrieb:
> Hi,
> 
> On Thu, Nov 12, 2009 at 11:30 AM, Thilo Borgmann
> <thilo.borgmann at googlemail.com> wrote:
>> Ronald S. Bultje schrieb:
>>> ltp_lag_length = 8 + samplerate >= 96000 + samplerate >= 192000;
>>> (might need brackets).
>> I like that one, too. Revision 1 attached.
> [..]
> 
> One more:
> 
>> +            ltp_gain[2]   = get_unary(gb, 0, 4);
>> +            ltp_gain[2] <<= 2;
>> +            ltp_gain[2]  += get_bits(gb, 2);
>> +            ltp_gain[2]   = ltp_gain_values[ltp_gain[2]];
> 
> No need to store the temp. variable in ltp_gain[2]. Might confuse the
> compiler into doing something silly.
> 
> int idx = get_unary(gb, 0, 4) << 2 + get_bits(gb, 2);
> ltp_gain[2] = ltp_gain_values[idx];

I did it that way and also added clipping the index before actually
accessing the array.

> 
> Or even:
> 
> int a = get_unary(gb, 0, 4), b = get_bits(gb, 2);
> ltp_gain[2] = ltp_gain_values[a][b];
> 
> (The compiler will handle these two equivalent if it's not completely
> retarded.) I also think that (if the order / behaviour is defined, I'm
> not sure if it is) you could get rid of the temporary variables then,
> so it fits on a single line:
> 
> ltp_gainp[2] = ltp_gain_values[get_unary(gb, 0, 4)][get_bits(gb, 2)];
> 

Accessing a one-dimensional array via x[][] would compile? Well at least
this might lead to confusion... and with clipping this would also loose
its elegance.

Thanks, revision 2 attached.

-Thilo
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: als_ltp.rev2.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091112/b96583dc/attachment.asc>