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

Måns Rullgård mans
Thu Nov 12 18:04:11 CET 2009


"Ronald S. Bultje" <rsbultje at gmail.com> writes:

> 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];
>
> Or even:
>
> int a = get_unary(gb, 0, 4), b = get_bits(gb, 2);
> ltp_gain[2] = ltp_gain_values[a][b];

Is that a 1D or 2D array.  Please try to index arrays as they are
declared, not in some other random fashion.  It *DOES* break
sometimes.

> (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)];

I don't recall seeing any promise of evaluation order for such an
expression.  Unless you can point at a paragraph in the C99 spec
defining it, don't rely on it.

-- 
M?ns Rullg?rd
mans at mansr.com



More information about the ffmpeg-devel mailing list