[FFmpeg-devel] [PATCH] Sun Rasterfile decoder

[Ivo]

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.

--Ivo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch_sunrast
Type: text/x-diff
Size: 7830 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071227/538b5ee6/attachment.diff>