[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