[FFmpeg-devel] [PATCH] correctly handle MSRLE uncompressed

[Kostya]

On Sun, May 24, 2009 at 12:47:01AM -0700, Baptiste Coudurier wrote:
> Hi
> 
> $subject, fix many files in RLE on mplayehq and issue #1074.
> 
> Decoder needs a bit of cleanup too, I think the context is no longer
> needed. Kostya ?

I'm not a maintainer but I'm pretty sure Mike won't object.
 
> -- 
> Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
> Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer                                  http://www.ffmpeg.org

> Index: libavcodec/msrle.c
> ===================================================================
> --- libavcodec/msrle.c	(revision 18912)
> +++ libavcodec/msrle.c	(working copy)
> @@ -55,7 +55,19 @@
>  
>      s->avctx = avctx;
>  
> -    avctx->pix_fmt = PIX_FMT_PAL8;
> +    switch (avctx->bits_per_coded_sample) {
> +    case 4:
> +    case 8:
> +        avctx->pix_fmt = PIX_FMT_PAL8;
> +        break;
> +    case 24:
> +        avctx->pix_fmt = PIX_FMT_BGR24;
> +        break;
> +    default:
> +        av_log(avctx, AV_LOG_ERROR, "unsupported bits per sample\n");
> +        return -1;
> +    }
> +
>      s->frame.data[0] = NULL;
>  
>      return 0;

Commit anytime.

> @@ -68,6 +80,7 @@
>      const uint8_t *buf = avpkt->data;
>      int buf_size = avpkt->size;
>      MsrleContext *s = avctx->priv_data;
> +    int istride = ((avctx->width * avctx->bits_per_coded_sample + 31) & ~31) / 8;

Maybe you should use FFALIGN() here. Maybe not.

>      s->buf = buf;
>      s->size = buf_size;
> @@ -88,8 +101,31 @@
>          }
>      }
>  
> -    ff_msrle_decode(avctx, (AVPicture*)&s->frame, avctx->bits_per_coded_sample, buf, buf_size);
> +    /* FIXME how to correctly detected RLE ??? */

how to correctly detect RLE
Hint: escape codes at the end

> +    if (avctx->height * istride == avpkt->size) {
> +        int linesize = avctx->width * avctx->bits_per_coded_sample / 8;
> +        uint8_t *ptr = s->frame.data[0];
> +        uint8_t *buf = avpkt->data + (avctx->height-1)*istride;
> +        int i, j;
>  
> +        for (i = 0; i < avctx->height; i++) {
> +            if (avctx->bits_per_coded_sample == 4) {
> +                for (j = 0; j < linesize; j++) {
> +                    ptr[j*2+0] = buf[j] >> 4;
> +                    ptr[j*2+1] = buf[j] & 0xf;
> +                }
> +                if (j*2 < avctx->width)
> +                    ptr[j*2+0] = buf[j] >> 4;

I'd rather write for(j = 0; j < avctx->width; j += 2){unpack byte}
without an additional condition

> +            } else {
> +                memcpy(ptr, buf, linesize);
> +            }
> +            buf -= istride;
> +            ptr += s->frame.linesize[0];
> +        }
> +    } else {
> +        ff_msrle_decode(avctx, (AVPicture*)&s->frame, avctx->bits_per_coded_sample, buf, buf_size);
> +    }
> +
>      *data_size = sizeof(AVFrame);
>      *(AVFrame*)data = s->frame;
  
looks ok