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

[Michael Niedermayer]

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

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

It is not what we do, but why we do it that matters.
-------------- 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/20090303/e52694f0/attachment.pgp>