[FFmpeg-devel] [PATCH] IFF: Add the HAM stuff

[Sebastian Vater]

Ronald S. Bultje a ?crit :
> Hi,
>
> On Fri, May 14, 2010 at 10:06 PM, Sebastian Vater
> <cdgs.basty at googlemail.com> wrote:
>   
>> +    uint8_t * ham_buf; // Temporary buffer for planar to chunky conversation
>> +    uint32_t *ham_palbuf; // HAM decode table
>> +    unsigned  compression; // Delta compression method used
>> +    unsigned  bpp; // Bits per plane to decode
>> +    unsigned  ham; // 0 if non-HAM or number of hold bits (6 for bpp > 6, 4 otherwise)
>> +    unsigned  flags; // 1 for EHB, 0 is no extra half darkening
>> +    unsigned  masking; // TODO: Masking method used
>>     
>
> Please vertically align, and use doxy-compatible comments here where
> appropriate (///< bla, not // bla). See "doxygen" output to get an
> idea of what we're looking for in these kind of docs.
>   

Fixed.

>> +#define DECODE_HAM_PLANE32(x)       \
>>     
>
> You can probably just make this a for(n=0;n<4;n++), gcc should unroll
> that automatically. Then you don't need the macro.
>   

I tried it out, it doesn't unroll the loop but insteads makes the
function inline instead.

Anyway, I spent a lots of time to get this function really fast.

>> +static void decodehamplane32(uint32_t *dst,
>> +                             const uint8_t  *buf,
>> +                             const uint32_t *const pal,
>> +                             unsigned buf_size)
>>     
>
> You can merge the args on 2 lines, no need for 4. Also, please use
> underscores, yourfunctionnamesaregettingalittlebittoolong.
>   

Fixed.

> Why?
>   

Number of planes can change between IFF-ANIM frames, that's whay. On the
other hand, it's also used by HAM decoder s->bpp contains original
number of planes (bits per coded sample is always set to 24 when
encountering HAM).

>> +#define GET_EXTRA_DATA(x) ((uint8_t *) (x)->data + AV_RB16((x)->data))
>>     
> [..]
>   
>> +#define GET_EXTRA_SIZE(x) ((x)->size - AV_RB16((x)->data))
>>     
>
> This is only used in libavcodec/iff.c and thus does not belong in iff.h.
>   

Fixed.

Besides this I fixed an error with the HAM palette initializing, it has
to always initialize 1 << hbits, but it didn't when the CMAP chunk size
/ 3 was smaller than this.

Also this patch is based upon newest duplicate code merge patch, again.

-- 

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: iff-ham-support.patch
Type: text/x-patch
Size: 18910 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100515/995e9c93/attachment.bin>