[Ffmpeg-devel] [PATCH] BMP encoder

Michael Niedermayer michaelni
Mon Oct 30 18:58:42 CET 2006


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 ...
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 ...
and either way the these things are seperate issues

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is




More information about the ffmpeg-devel mailing list