[FFmpeg-devel] [PATCH] Indeo5 decoder

Maxim max_pole
Fri Mar 27 16:49:35 CET 2009


Reimar D?ffinger schrieb:
> On Fri, Mar 27, 2009 at 04:22:40PM +0100, Maxim wrote:
>   
>>>> +            switch (get_bits(&ctx->gb,2)) {
>>>> +                case 0:
>>>> +                    mb_size = 16;
>>>> +                    blk_size = 8;
>>>> +                    break;
>>>> +                case 1:
>>>> +                    mb_size = 8;
>>>> +                    blk_size = 8;
>>>> +                    break;
>>>> +                case 2:
>>>> +                    mb_size = 8;
>>>> +                    blk_size = 4;
>>>> +                    break;
>>>> +                case 3:
>>>> +                    mb_size = 4;
>>>> +                    blk_size = 4;
>>>> +                    break;
>>>> +            }
>>>>     
>>>>         
>>> mb_size = blk_size =  4;
>>> if (!get_bits1(...)) {
>>>     mb_size  <<= 1;
>>>     blk_size <<= 1;
>>> }
>>> if (!get_bits1(...))
>>>     mb_size  <<= 1;
>>>
>>> Maybe?
>>>   
>>>       
>> this make the code less readable IMHO. Therefore leaved as is...
>>     
>
> How about e.g.
> blk_size = get_bits1(...) ? 4 : 8;
> mb_size  = get_bits1(...) ? blk_size : 2*blksize;
> or
> blk_size = get_bits1(...) ? 4 : 8;
> mb_size  = blk_size << !get_bits1(...);
> or
> blk_size = 4        << !get_bits1(...);
> mb_size  = blk_size << !get_bits1(...);
>
> The switch IMO simply makes it look too much like "magic numbers" when
> it could be much more nicely interpreted as two independent flags - not
> to mention 2 vs. 18 lines of code.
>   

Ok, I'll replace it in the next update. I think I'll add a comment line
for a clear description of the block sizes as well...

Regards
Maxim



More information about the ffmpeg-devel mailing list