[FFmpeg-devel] [PATCH] IFF: Add grayscale support to decoder

Sebastian Vater cdgs.basty
Thu May 13 20:55:40 CEST 2010


Ronald S. Bultje a ?crit :
> Hi,
>
> On Thu, May 13, 2010 at 1:25 PM, Sebastian Vater
> <cdgs.basty at googlemail.com> wrote:
>   
>> So this patch fixes this issue by setting PIX_FMT_GRAY8 only if bpp == 8
>> and initializes a custom RGB color palette for bpp < 8 simulating
>> correct behaviour (pictures are way too dark otherwise).
>>
>> Please note that the GRAY2RGB macro will later be used by HAM, too.
>>     
> [..]
>   
>> +#define GRAY2RGB(x) (((x) << 16) | ((x) <<  8) | (x))
>>     
>
> So, this is OK in principle, but you're calculating x 3 times here.
> Just look at the disasembly. A static inline function might be better.
>   

Fixed.

>   
>> -    return avctx->bits_per_coded_sample <= 8 ?
>> +    return (avctx->bits_per_coded_sample <= 8 &&
>> +            avctx->pix_fmt != PIX_FMT_GRAY8) ?
>>     
>
> avctx->pix_fmt == PAL8 ? .. : .. should achieve the same thing in 1
> line less code.
>   

I think we agreed that we use this so this function is at least called
for HAM (in which case bits_per_coded_sample is <= 8).
Otherwise I have to change this line again, when adding HAM support.

> Rest of the patch is OK, good work.

Thanks!

-- 

Best regards,
                   :-) Basty/CDGS (-:

-------------- next part --------------
A non-text attachment was scrubbed...
Name: iff-decoder-grayscale-support.patch
Type: text/x-patch
Size: 3371 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100513/d207a280/attachment.bin>



More information about the ffmpeg-devel mailing list