[FFmpeg-devel] [PATCH] Sun Rasterfile decoder

Michael Niedermayer michaelni
Thu Dec 27 01:53:23 CET 2007


On Thu, Dec 27, 2007 at 12:16:24AM +0100, Ivo wrote:
> On Wednesday 26 December 2007 21:50, Michael Niedermayer wrote:
> > > +    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
> [...]
> > > +    *picture = *(AVFrame *)&s->picture;
> > > +    *data_size = sizeof(AVPicture);
> >
> > hmm
> 
> All fixed.
> 
> > > +    if (depth != 1 && depth != 8 && depth != 24) {
> > > +        av_log(avctx, AV_LOG_ERROR, "invalid depth\n");
> > > +        return -1;
> > > +    }
> [...]
> > > +        case 24:
> > > +            avctx->pix_fmt = PIX_FMT_BGR24;
> > > +    }
> >
> > that if() at the top would better fit in a default here
> 
> Done.
> 
> > > +        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 ...)
> 
> This is not exactly the same. The people at Sun's decided it was a good idea 
> to store the palette like: R0R1R2..Rn G0G1G2...Gn B0B1B2...Bn and that's 
> what this code converts to normal R0G0B0 etc..
> 
> > > +            for (; c; c--, a++) {
> >
> > while(c--){
> 
> Done.
> 
> > > +                if (i < stride)
> > > +                    ptr[i] = b;
> >
> > this should be w not stride
> 
> That would not be correct for 1 and 24 bit images. I thought stride was a 
> good idea to be sure it never overflowed the scanline but was guaranteed to 
> be long enough without further calculations. The check is necessary because 
> in sun rasterfiles scanlines are padded again to 16-bit boundaries and, 
> unlike pcx which had decoding stops after every scanline, runs can span 
> multiple lines. I could however use the len variable and sometimes skip one 
> store. Done that in the new patch.
> 
> > > +                if (++i >= alen) {
> > > +                    i = 0;
> > > +                    ptr += stride;
> > > +                }
> > > +            }
> > > +        }
> >
> > s/i/x/
> > s/c/run/
> 
> Done.
> 
> > also i think this is not guranteed to stop at the end of the array
> 
> Yes, you are right. Corrupted image data could cause it to write past the 
> end. Fixed by adding another condition.
> 
> New patch attached.
[...]

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

hmmmmmmmm


[...]
> +    if (type == RT_BYTE_ENCODED) {
> +        int a, value, run;
> +        uint8_t *end = ptr + h*stride;
> +
> +        x = 0;
> +        for (a=0; a < alen*h && ptr < end; ) {

and what if stride is negative?

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20071227/22feac66/attachment.pgp>



More information about the ffmpeg-devel mailing list