[FFmpeg-devel] [PATCH] PCX encoder

[Michael Niedermayer]

On Sat, Mar 14, 2009 at 04:09:54PM -0400, Daniel Verkamp wrote:
> On Sat, Mar 14, 2009 at 3:50 PM, Daniel Verkamp <daniel at drv.nu> wrote:
> > Hi,
> >
> > Attached is a patch for a PCX image encoder that handles 1-, 8-, and
> > 24-bpp pixfmts. ?1 and 8 bpp tested with pbrush from Win3.11, 24 bpp
> > tested with IrfanView; if anyone has a version of PC Paintbrush that
> > supports 24 bpp, please test, but I am fairly sure it is correct.
> >
> > Thanks,
> > -- Daniel Verkamp
> >
> 
> Silly last-minute change broke compilation - fixed patch attached.
[...]
> +/**
> + * PCX run-length encoder
> + * @param dst output buffer
> + * @param dst_size size of output buffer
> + * @param src input buffer
> + * @param src_size size of input buffer
> + * @return number of bytes written to dst or -1 on error
> + */
> +static int pcx_rle_encode(      uint8_t *dst, int dst_size,
> +                          const uint8_t *src, int src_size)
> +{
> +    int i;
> +    uint8_t prev = src[0];
> +    int count = 1;
> +    int dst_used = 0;
> +
> +    for (i = 1; i <= src_size; i++) {
> +        if (i != src_size && src[i] == prev && count < 0x3F) {
> +            // current byte is same as prev
> +            ++count;
> +        } else {
> +            // output prev * count
> +            if (count == 1 && prev < 0xC0) {
> +                if (++dst_used > dst_size)
> +                    return -1;
> +                *dst++ = prev;
> +            } else {
> +                dst_used += 2;
> +                if (dst_used > dst_size)
> +                    return -1;
> +                *dst++ = 0xC0 | count;
> +                *dst++ = prev;
> +            }

*dst++ = prev can be factored out


> +
> +            // get new prev
> +            if (i != src_size) {
> +                count = 1;
> +                prev = src[i];
> +            }

if(i == src_size)
    return dst_used;
before this seems to make more sense and would also make the
check in the loop unneeded


[...]

> +    for (i = 0; i < avctx->height; i++) {

y is better than i for the vertical coordinate


> +        for (j = 0; j < nplanes; j++) {

p is better than j for plane


> +            for (k = j, l = 0; k < src_line_size; k += nplanes, l++) {
> +                plane[l] = src[k];
> +            }
> +            if ((written = pcx_rle_encode(buf, buf_size - (buf - bufstart),

i think a buf_end pointer could simplify this


> +                                          plane, line_bytes)) < 0) {
> +                av_log(avctx, AV_LOG_ERROR, "buffer too small\n");
> +                return -1;
> +            }
> +            buf += written;
> +        }
> +        src += p->linesize[0];
> +    }
> +
> +    av_free(plane);
> +
> +    if (nplanes == 1 && bpp == 8) {
> +        bytestream_put_byte(&buf, 12);
> +        for (i = 0; i < 256; i++) {
> +            bytestream_put_be24(&buf, pal[i]);
> +        }
> +    }

this seems to be missing checks for buf_size

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

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- 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/20090315/a33f20fd/attachment.pgp>