[Ffmpeg-devel] [PATCH] BMP encoder

[Michel Bardiaux]

Michael Niedermayer wrote:
> Hi
> 
> On Mon, Oct 30, 2006 at 06:20:50PM +0100, Michel Bardiaux wrote:
> [...]
>>>>>> +    write32(&s->pb, 0);
>>>>>> +    // BMP files are bottom-to-top
>>>>>> +    ptr = p->data[0] + (avctx->height - 1) * p->linesize[0];
>>>>>> +    linesize = -p->linesize[0];
>>>>>> +    for(i = 0; i < avctx->height; i++){
>>>>>> +        uint8_t *src = ptr;
>>>>>> +        int n = 0;
>>>>>> +        for(j=0;j<avctx->width;j++) {
>>>>>> +            put_bits(&s->pb, 8, src[0]);
>>>>>> +            put_bits(&s->pb, 8, src[1]);
>>>>>> +            put_bits(&s->pb, 8, src[2]);
>>>>> using the bitstream reader for a purely byte based format is unacceptable
>>>>> (reason is that its several times slower and more complex)
>> There was 'previous art': the bmp *decoder* used the bitstreams. There 
>> was no 'FIXME: change to byte stream for performance'. You're changing 
>> the rules in the middle of the game. Are you going to reject the patch 
>> if I do change the decoder to bytestram too? Or reject if I *dont*?
> 
> 
> bmp.c:
> ---------
>  switch(depth){
>     case 24:
>         for(i = 0; i < avctx->height; i++){
>             memcpy(ptr, buf, n);
>             buf += n;
>             ptr += linesize;
>         }
>         break;
>     case 16:
>         for(i = 0; i < avctx->height; i++){
>             uint16_t *src = (uint16_t *) buf;
>             uint16_t *dst = (uint16_t *) ptr;
> 
>             for(j = 0; j < avctx->width; j++)
>                 *dst++ = le2me_16(*src++);
> 
>             buf += n;
>             ptr += linesize;
>         }
>         break;
>     case 32:
>         for(i = 0; i < avctx->height; i++){
>             uint8_t *src = buf;
>             uint8_t *dst = ptr;
> 
>             for(j = 0; j < avctx->width; j++){
>                 dst[0] = src[rgb[2]];
>                 dst[1] = src[rgb[1]];
>                 dst[2] = src[rgb[0]];
>                 dst += 3;
>                 src += 4;
>             }
> 
>             buf += n;
>             ptr += linesize;
>         }
>         break;
> ----------
> 
> no bitstream.h stuff in the inner loop ....
> bmp.c uses it just for reading the header, if thats optimal is another 
> question yes but its a differnt matter, and if you look you will see
> that my complaint was below the innermost loop in the encoder not
> the header reading ...

I understand you now.

> and yes i would be happy if the decoder where changed to not use bitstream.h
> in the header reading either but mans is the maintainer of the bmp decoder
> so its his decission ...

Mans? You or me?

> and either way the these things are seperate issues
> 
> [...]
> 


-- 
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/