[FFmpeg-soc] [soc]: r5119 - als/alsdec.c

Thilo Borgmann thilo.borgmann at googlemail.com
Sat Aug 15 14:14:11 CEST 2009


>> -            if (sconf->resolution == 2 || sconf->floating) {
>> +            if (sconf->resolution == 2 || sconf->floating)
>>                  const_val_bits = 24;
>> -            } else {
>> +            else
>>                  const_val_bits = avctx->bits_per_raw_sample;
>> -            }
> 
> Note that the {} of the if part do not add an extra line of code, so
> e.g. Michael prefers to keep them.
> Also this would IMO be simpler as
> const_val_bits = sconf->resolution == 2 || sconf->floating ? 24 : avctx->bits_per_raw_sample;
> (mostly because it is obvious that in both paths const_val_bits is set.

The else part is one line less. I don't want to mix the style to have
the if part including braces and the else part not.

This part is not about to be changed thus to make follow-up patches of
this part smaller does also not apply IMO.

The tertiary operator would even make it a bit faster I think but for
the cost of being unreadable because of its length and need to be split
to two lines IMO.

As long as it is suggested and not requested I would like to keep it as is.

Thanks for your reviewing!

Thilo


More information about the FFmpeg-soc mailing list