[Ffmpeg-devel] [PATCH]BMP encoder

[Michel Bardiaux]

M?ns Rullg?rd wrote:
> Michael Niedermayer <michaelni at gmx.at> writes:
> 
>> Hi
>>
>> On Tue, Jan 23, 2007 at 01:08:33PM +0100, Michel Bardiaux wrote:
>>> -- 
>> [...]
>>
>>> Index: libavcodec/bmp.c
>>> ===================================================================
>>> --- libavcodec/bmp.c	(revision 7659)
>>> +++ libavcodec/bmp.c	(working copy)
>>> @@ -1,6 +1,7 @@
>>>  /*
>>>   * BMP image format
>>> - * Copyright (c) 2005 Mans Rullgard
>>> + * Copyright (c) 2005 Mans Rullgard (decoder)
>>> + * Copyright (c) 2006, 2007 Michel Bardiaux (encoder)
>>>   *
>>>   * This file is part of FFmpeg.
>>>   *
>>> @@ -32,10 +33,8 @@
>>>  #define BMP_RLE4      2
>>>  #define BMP_BITFIELDS 3
>> these should be an enum IMHO, mans any objections if i change that?
> 
> Change it if you like.

Michael has done it.

> 
> [...]
> 
>> better put the encoder into its own file
> 
> Yes, please.

OK, I will. That implies moving the BMP_RLE4 etc to a bmp.h, and 
BMPContext. What should I do about the _init method? One for enc and one 
for dec, even if that duplicates the code?

> 
>>> +    n_bytes = n_bytes_image + 14 /* BITMAPFILEHEADER */ + 40 /* BITMAPINFOHEADER */;
>> instead of commenting i would rather use named values 
>> (#define BM_FILE_HEADER_SIZE 14)
> 
> I prefer macros.
> 
Actually, the 14 alone is not used, only combined with 40, and *that* 
cant be a define, because some other bmp encodings use a color table, 
which goes just after the BITMAPINFOHEADER, and has variable size. I 
think the best would be to call put_bits_count at the right place (like 
after the header is complete) then go store it in the right place.


-- 
Michel Bardiaux
R&D Director
T +32 [0] 2 790 29 41
F +32 [0] 2 790 29 02
E mailto:mbardiaux at mediaxim.be

Mediaxim NV/SA
Vorstlaan 191 Boulevard du Souverain
Brussel 1160 Bruxelles
http://www.mediaxim.com/