[FFmpeg-devel] [PATCH] H.264: cleanup decode_residual()

[Alex Converse]

On Mon, Mar 2, 2009 at 11:08 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Mon, Mar 02, 2009 at 10:18:34PM -0500, Alex Converse wrote:
>> On Mon, Mar 2, 2009 at 10:06 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Mon, Mar 02, 2009 at 09:11:24PM -0500, Alex Converse wrote:
>> >> easier to follow and a very very minor speedup on gcc-4.3.2
>> >
>> >> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
>> >> index 91f20c9..560f964 100644
>> >> --- a/libavcodec/h264.c
>> >> +++ b/libavcodec/h264.c
>> >> @@ -4187,17 +4187,16 @@ static int decode_residual(H264Context *h, GetBitContext *gb, DCTELEM *block, in
>> >> ? ? ? ? ? ? ?//first coefficient has suffix_length equal to 0 or 1
>> >> ? ? ? ? ? ? ?if(prefix<14){ //FIXME try to build a large unified VLC table for all this
>> >> ? ? ? ? ? ? ? ? ?if(suffix_length)
>> >> - ? ? ? ? ? ? ? ? ? ?level_code= (prefix<<suffix_length) + get_bits(gb, suffix_length); //part
>> >> + ? ? ? ? ? ? ? ? ? ?level_code= (prefix<<1) + get_bits(gb, 1); //part
>> >> ? ? ? ? ? ? ? ? ?else
>> >> - ? ? ? ? ? ? ? ? ? ?level_code= (prefix<<suffix_length); //part
>> >> + ? ? ? ? ? ? ? ? ? ?level_code= prefix; //part
>> >> ? ? ? ? ? ? ?}else if(prefix==14){
>> >> ? ? ? ? ? ? ? ? ?if(suffix_length)
>> >> - ? ? ? ? ? ? ? ? ? ?level_code= (prefix<<suffix_length) + get_bits(gb, suffix_length); //part
>> >> + ? ? ? ? ? ? ? ? ? ?level_code= (prefix<<1) + get_bits(gb, 1); //part
>> >> ? ? ? ? ? ? ? ? ?else
>> >> ? ? ? ? ? ? ? ? ? ? ?level_code= prefix + get_bits(gb, 4); //part
>> >
>> > above is ok
>> >
>>
>> I missed an obvious use of get_bits1() up there. Would that be preferable?
>
> yes
>
>
>>
>> >
>> >> ? ? ? ? ? ? ?}else{
>> >> - ? ? ? ? ? ? ? ?level_code= (15<<suffix_length) + get_bits(gb, prefix-3); //part
>> >> - ? ? ? ? ? ? ? ?if(suffix_length==0) level_code+=15; //FIXME doesn't make (much)sense
>> >> + ? ? ? ? ? ? ? ?level_code= 30 + get_bits(gb, prefix-3); //part
>> >> ? ? ? ? ? ? ? ? ?if(prefix>=16)
>> >> ? ? ? ? ? ? ? ? ? ? ?level_code += (1<<(prefix-3))-4096;
>> >> ? ? ? ? ? ? ?}
>> >
>> > can you explain why suffix_length here has to be 1 ?
>> >
>>
>> It doesn't have to be 1. It has to be 0 or 1.
>> If it's 0 we have: level_code= (15<<0) + get_bits(gb, prefix-3) + 15;
>> If it's 1 we have: level_code= (15<<1) + get_bits(gb, prefix-3);
>> Both are equivalent to: level_code= 30 + get_bits(gb, prefix-3);
>
> ahh right, makes sense, patch ok
>
> [...]

Applied.

--Alex