[Ffmpeg-devel] [PATCH]BMP encoder

[Michael Niedermayer]

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?


>  
> -#define read16(bits) bswap_16(get_bits(bits, 16))
> -#define read32(bits) bswap_32(get_bits_long(bits, 32))
> -
> -static int bmp_decode_init(AVCodecContext *avctx){

> +#if defined(CONFIG_BMP_ENCODER)||defined(CONFIG_BMP_DECODER)

the file will not be compiled if this isnt true


> +static int bmp_common_init(AVCodecContext *avctx){
>      BMPContext *s = avctx->priv_data;
>  
>      avcodec_get_frame_defaults((AVFrame*)&s->picture);
> @@ -43,7 +42,13 @@
>  
>      return 0;
>  }
> +#endif
>  
> +#ifdef CONFIG_BMP_DECODER
> +
> +#define read16(bits) bswap_16(get_bits(bits, 16))
> +#define read32(bits) bswap_32(get_bits_long(bits, 32))

moving these around is a cosmetic change, and its unneeded they do no
harm if they stay where the other #defines are


> +
>  static int bmp_decode_frame(AVCodecContext *avctx,
>                              void *data, int *data_size,
>                              uint8_t *buf, int buf_size)
> @@ -242,13 +247,91 @@
>      return buf_size;
>  }
>  
> +#endif // CONFIG_BMP_DECODER
> +
> +#ifdef CONFIG_BMP_ENCODER

please sort functions so that the number of ifdefs get minimized or even
better put the encoder into its own file


> +
> +#define write16(bits, val) put_bits(bits, 16, bswap_16(val))
> +#define write32(bits, val) put_bits(bits, 32, bswap_32(val))
> +
> +static int bmp_encode_frame(AVCodecContext *avctx, unsigned char *buf, int buf_size, void *data){
> +    BMPContext *s = avctx->priv_data;
> +    AVFrame *pict = data;
> +    AVFrame * const p= (AVFrame*)&s->picture;
> +    PutBitContext bits;
> +    int n_bytes_image, n_bytes_per_row, n_bytes, i, n, hsize;
> +    uint8_t *ptr;
> +    *p = *pict;
> +    p->pict_type= FF_I_TYPE;
> +    p->key_frame= 1;
> +    n_bytes_per_row = (avctx->width*3 + 3) & ~3;
> +    n_bytes_image = avctx->height*n_bytes_per_row;
> +    n_bytes = n_bytes_image + 14 /* BITMAPFILEHEADER */ + 40 /* BITMAPINFOHEADER */;

instead of commenting i would rather use named values 
(#define BM_FILE_HEADER_SIZE 14)
but iam fine with the commented numbers too if you prefer


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070124/3afe134e/attachment.pgp>