[FFmpeg-devel] [PATCH] Sun Rasterfile decoder

[Michael Niedermayer]

On Wed, Dec 26, 2007 at 02:00:50PM +0100, Ivo wrote:
> Hi,
> 
> Here's a Sun Rasterfile decoder.
> 
> http://wiki.multimedia.cx/index.php?title=Sun_rasterfile
> 
> Documentation on the format is sparse, but it can decode all files I was 
> able to find and generate with other tools.
> 
> I uploaded all sample files to:
> 
> ftp://samples.mplayerhq.hu/MPlayer/incoming/sunrast
[...]
> +static int sunrast_init(AVCodecContext *avctx) {
> +    SUNRASTContext *s = avctx->priv_data;
> +
> +    avcodec_get_frame_defaults((AVFrame*)&s->picture);
> +    avctx->coded_frame= (AVFrame*)&s->picture;
> +    s->picture.data[0] = NULL;

hmm


[...]
> +    AVFrame * const p = (AVFrame *)&s->picture;

hmm


[...]
> +    if (depth != 1 && depth != 8 && depth != 24) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid depth\n");
> +        return -1;
> +    }
> +    if (type > RT_BYTE_ENCODED && type <= RT_FORMAT_IFF) {
> +        av_log(avctx, AV_LOG_ERROR, "unsupported (compression) type\n");
> +        return -1;
> +    }
> +    if (type > RT_FORMAT_IFF) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid (compression) type\n");
> +        return -1;
> +    }
> +    if (maptype & ~1) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid colormap type\n");
> +        return -1;
> +    }
> +
> +    buf += 32;
> +
> +    switch (depth) {
> +        case 1:
> +            avctx->pix_fmt = PIX_FMT_MONOWHITE;
> +            break;
> +        case 8:
> +            avctx->pix_fmt = PIX_FMT_PAL8;
> +            break;
> +        case 24:
> +            avctx->pix_fmt = PIX_FMT_BGR24;
> +    }

that if() at the top would better fit in a default here


[...]
> +        ptr = p->data[1];
> +        for (i=0; i<len; i++, ptr+=4)
> +            *(uint32_t *)ptr = (buf[i]<<16) + (buf[len+i]<<8) + buf[len+len+i];

duplicate relative to your previous decoder (and i somehow think more
decoders use this ...)


[...]
> +        i = 0;
> +        for (a=0; a<alen*h; ) {
> +            c = 1;
> +            if ((b = *buf++) == 0x80) {
> +                c = *buf++ + 1;
> +                if (c != 1)
> +                    b = *buf++;
> +            }
> +            for (; c; c--, a++) {

while(c--){


> +                if (i < stride)
> +                    ptr[i] = b;

this should be w not stride


> +                if (++i >= alen) {
> +                    i = 0;
> +                    ptr += stride;
> +                }
> +            }
> +        }

s/i/x/
s/c/run/

also i think this is not guranteed to stop at the end of the array



[...]
> +    *picture = *(AVFrame *)&s->picture;
> +    *data_size = sizeof(AVPicture);

hmm


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071226/cdf823bc/attachment.pgp>