[FFmpeg-devel] [PATCH] Arithmetic decoding in ALS

[Thilo Borgmann]

Am 10.02.10 16:09, schrieb Thilo Borgmann:
> Am 07.02.10 01:16, schrieb Thilo Borgmann:
>> Am 06.02.10 10:03, schrieb Reimar D?ffinger:
>>> On Tue, Jan 26, 2010 at 01:44:46AM +0100, Thilo Borgmann wrote:
>>>> +//#define DEBUG
>>>
>>> That's not used anywhere.
>>>
>>>> +static const unsigned int cf_tables_1[3][129] = {
>>>
>>> All tables seem to fit in 16 bit?
>>>
>>>> +static const unsigned int *cf_table[16] = {
>>>
>>> one const too few.
>>>
>>>> +int bgmc_init(AVCodecContext *avctx, uint8_t **cf_lut, unsigned int **cf_lut_status);
>>>
>>> global symbols all need either a ff_ or av_ prefix, ff_ is appropriate I guess.
>>>
>>>> +static const unsigned int tail_code[16][6] = {
>>>
>>> Fits in 8 bits?
>>
>> All above done.
>>
>>
>>>
>>>> +        unsigned int B = av_clip((av_ceil_log2(bd->block_length) - 3) >> 1, 0, 5);
>>>
>>> B is not a good name, not only looks it a lot like 8 with some fonts, uppercase is also
>>> for macros.
>>
>> Renamed to "b". Yes, bad name but used just once and it is some
>> fictional entity I don't know a suitable name or abbreviation for. Also
>> "b" is close to the specs and therefore easy to relate to the algorithm.
>>
>>>
>>>> +            k    [sb] = s[sb] > B ? s[sb] - B : 0;
>>>
>>> FFMAX(s[sb] - B, 0)
>>> might be faster for some architectures at least.
>>
>> s[] and b are unsigned, so this might reveal unsigned-issues again on
>> some compilers. Also, this is done once per "large" sub-block so a
>> minimal speedup should converge to zero.
>>
>>
>>>
>>>> +                    if (res & 1)
>>>> +                        res = -res - 1;
>>>> +
>>>> +                    res >>= 1;
>>>
>>> Isn't that the usually "level + sign" coding? In that case there should be an
>>> optimized function for MPEG coding, one of these days I have to search for it
>>> and write it down...
>>> Either way, if res has the lowest bit set, so does -res, so your -1 is just
>>> unsetting the bit you shift away in the next step anyway.
>>
>> Indeed, Done!
>>
>> Also, ff_bgmc_end() actually used in alsdec.c.
>>
>> Thanks! Revision 6 attached.
> 
> If no one objects anymore I will apply this in a few days.

Applied, thanks!

-Thilo