[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