[FFmpeg-devel] [PATCH] PCX encoder
Daniel Verkamp
daniel
Tue Mar 17 18:14:50 CET 2009
On Mon, Mar 16, 2009 at 7:05 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Sun, Mar 15, 2009 at 08:28:32PM -0400, Daniel Verkamp wrote:
>> On Sun, Mar 15, 2009 at 7:40 PM, Daniel Verkamp <daniel at drv.nu> wrote:
>> > On Sun, Mar 15, 2009 at 7:38 PM, Daniel Verkamp <daniel at drv.nu> wrote:
>> >> On Sun, Mar 15, 2009 at 4:18 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >>> 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
>> >>>
>> >>
>> >> Done.
>> >>
>> >>>
>> >>>> +
>> >>>> + ? ? ? ? ? ?// 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
>> >>>
>> >>
>> >> Written as I think you mean - the check is still in the loop,
>> >> though... ?The reason I put the check in the loop in the first place
>> >> is to avoid duplicating the output part (deciding whether the run can
>> >> be written in one byte or two) for the final byte.
>> >>
>> >>>
>> >>> [...]
>> >>>
>> >>>> + ? ?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
>> >>>
>> >>
>> >> Changed both of above variable names.
>> >>
>> >>>
>> >>>> + ? ? ? ? ? ?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
>> >>>
>> >>
>> >> Indeed, reworked to use a buf_end.
>> >>
>> >>>
>> >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?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
>> >>>
>> >>
>> >> Good point, added one now.
>> >> Do I need one for the (128-byte) header as well, or can I assume buf
>> >> is always at least some constant size?
>> >>
>> >>> [...]
>> >>> --
>> >>> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>> >>
>> >> Thanks,
>> >> -- Daniel Verkamp
>> >>
>> >
>> > And of course it would help if I attached the patch...
>> >
>>
>> This time without \r\n line endings...
>
>
> [...]
>> +/**
>> + * 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++) {
> ? ? ? ? ? ? ? ? ? ^^^^^^^^^^^^^
> useless
>
Removed.
>
>> + ? ? ? ?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;
>> + ? ? ? ? ? ?} else {
>> + ? ? ? ? ? ? ? ?dst_used += 2;
>> + ? ? ? ? ? ? ? ?if (dst_used > dst_size)
>> + ? ? ? ? ? ? ? ? ? ?return -1;
>> + ? ? ? ? ? ? ? ?*dst++ = 0xC0 | count;
>> + ? ? ? ? ? ?}
>
> i would put a dst_size < 2LL*src_size outside
>
Ah, I see - changed to check this way, plus some other reductions.
> [...]
>> + ? ?filler = 128 - (buf - buf_start);
>> + ? ?memset(buf, 0, filler);
>> + ? ?buf += filler;
>
> while(buf-buf_start < 128)
> ? ?*buf++= 0;
>
Changed.
>> +
>> + ? ?plane = av_mallocz(line_bytes);
>> + ? ?src = pict->data[0];
>> + ? ?src_line_size = (avctx->width * nplanes * bpp + 7) >> 3;
>> +
>> + ? ?for (y = 0; y < avctx->height; y++) {
>> + ? ? ? ?for (p = 0; p < nplanes; p++) {
>> + ? ? ? ? ? ?for (i = p, j = 0; i < src_line_size; i += nplanes, j++) {
>> + ? ? ? ? ? ? ? ?plane[j] = src[i];
>> + ? ? ? ? ? ?}
>> + ? ? ? ? ? ?if ((written = pcx_rle_encode(buf, buf_end - buf,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?plane, line_bytes)) < 0) {
>> + ? ? ? ? ? ? ? ?av_log(avctx, AV_LOG_ERROR, "buffer too small\n");
>> + ? ? ? ? ? ? ? ?return -1;
>
> memleak
>
Fixed.
>
>> + ? ? ? ? ? ?}
>> + ? ? ? ? ? ?buf += written;
>> + ? ? ? ?}
>> + ? ? ? ?src += pict->linesize[0];
>> + ? ?}
>> +
>> + ? ?av_free(plane);
>> +
>> + ? ?if (nplanes == 1 && bpp == 8) {
>> + ? ? ? ?if (buf + 257 > buf_end) {
>
> this has a small chance of overflow
>
> if(buf_end - buf < 257)
>
Changed.
>
> [...]
> --
> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
Also added pcx to image2_muxer; new patch attached.
Thanks,
-- Daniel Verkamp
-------------- next part --------------
>From 5379f56abec36a2f5e5e37d4b2ec894f53320fea Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <daniel at drv.nu>
Date: Sat, 14 Mar 2009 14:41:56 -0500
Subject: [PATCH] Add PCX encoder
---
Changelog | 1 +
doc/general.texi | 2 +-
libavcodec/Makefile | 1 +
libavcodec/allcodecs.c | 2 +-
libavcodec/pcxenc.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++++
libavformat/img2.c | 2 +-
6 files changed, 214 insertions(+), 3 deletions(-)
create mode 100644 libavcodec/pcxenc.c
diff --git a/Changelog b/Changelog
index 7538ea6..0d2a16e 100644
--- a/Changelog
+++ b/Changelog
@@ -4,6 +4,7 @@ version <next>:
- deprecated vhook subsystem removed
- deprecated old scaler removed
- VQF demuxer
+- PCX encoder
diff --git a/doc/general.texi b/doc/general.texi
index 143aef4..7e84a58 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -250,7 +250,7 @@ following image formats are supported:
@tab PAM is a PNM extension with alpha support.
@item PBM @tab X @tab X
@tab Portable BitMap image
- at item PCX @tab @tab X
+ at item PCX @tab X @tab X
@tab PC Paintbrush
@item PGM @tab X @tab X
@tab Portable GrayMap image
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 13415fc..8175fc5 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -159,6 +159,7 @@ OBJS-$(CONFIG_NUV_DECODER) += nuv.o rtjpeg.o
OBJS-$(CONFIG_PAM_ENCODER) += pnmenc.o pnm.o
OBJS-$(CONFIG_PBM_ENCODER) += pnmenc.o pnm.o
OBJS-$(CONFIG_PCX_DECODER) += pcx.o
+OBJS-$(CONFIG_PCX_ENCODER) += pcxenc.o
OBJS-$(CONFIG_PGM_ENCODER) += pnmenc.o pnm.o
OBJS-$(CONFIG_PGMYUV_ENCODER) += pnmenc.o pnm.o
OBJS-$(CONFIG_PNG_DECODER) += png.o pngdec.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index b024e01..bc37401 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -127,7 +127,7 @@ void avcodec_register_all(void)
REGISTER_DECODER (NUV, nuv);
REGISTER_ENCODER (PAM, pam);
REGISTER_ENCODER (PBM, pbm);
- REGISTER_DECODER (PCX, pcx);
+ REGISTER_ENCDEC (PCX, pcx);
REGISTER_ENCODER (PGM, pgm);
REGISTER_ENCODER (PGMYUV, pgmyuv);
REGISTER_ENCDEC (PNG, png);
diff --git a/libavcodec/pcxenc.c b/libavcodec/pcxenc.c
new file mode 100644
index 0000000..67796f1
--- /dev/null
+++ b/libavcodec/pcxenc.c
@@ -0,0 +1,209 @@
+/*
+ * PC Paintbrush PCX (.pcx) image encoder
+ * Copyright (c) 2009 Daniel Verkamp <daniel at drv.nu>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * PCX image encoder
+ * @file libavcodec/pcxenc.c
+ * @author Daniel Verkamp
+ * @sa http://www.qzx.com/pc-gpe/pcx.txt
+ */
+
+#include "avcodec.h"
+#include "bytestream.h"
+
+typedef struct PCXContext {
+ AVFrame picture;
+} PCXContext;
+
+static const uint32_t monoblack_pal[] = { 0x000000, 0xFFFFFF };
+
+static av_cold int pcx_encode_init(AVCodecContext *avctx)
+{
+ PCXContext *s = avctx->priv_data;
+
+ avcodec_get_frame_defaults(&s->picture);
+ avctx->coded_frame = &s->picture;
+
+ return 0;
+}
+
+/**
+ * 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;
+ const uint8_t *dst_start = dst;
+
+ // check worst-case upper bound on dst_size
+ if (dst_size < 2LL * src_size)
+ return -1;
+
+ for (i = 1; ; 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)
+ *dst++ = 0xC0 | count;
+ *dst++ = prev;
+
+ if (i == src_size)
+ return dst - dst_start;
+
+ // start new run
+ count = 1;
+ prev = src[i];
+ }
+ }
+
+ return 0; // gcc warns even though this cannot be reached
+}
+
+static int pcx_encode_frame(AVCodecContext *avctx,
+ unsigned char *buf, int buf_size, void *data)
+{
+ PCXContext *s = avctx->priv_data;
+ AVFrame *const pict = &s->picture;
+ const uint8_t *buf_start = buf;
+ const uint8_t *buf_end = buf + buf_size;
+
+ int bpp, nplanes, i, j, y, p, line_bytes, written, src_line_size;
+ const uint32_t *pal = NULL;
+ const uint8_t *src;
+ uint8_t *plane;
+
+ *pict = *(AVFrame *)data;
+ pict->pict_type = FF_I_TYPE;
+ pict->key_frame = 1;
+
+ if (avctx->width > 65535 || avctx->height > 65535) {
+ av_log(avctx, AV_LOG_ERROR, "image dimensions do not fit in 16 bits\n");
+ return -1;
+ }
+
+ switch (avctx->pix_fmt) {
+ case PIX_FMT_RGB24:
+ bpp = 8;
+ nplanes = 3;
+ break;
+ case PIX_FMT_RGB8:
+ case PIX_FMT_BGR8:
+ case PIX_FMT_RGB4_BYTE:
+ case PIX_FMT_BGR4_BYTE:
+ case PIX_FMT_GRAY8:
+ case PIX_FMT_PAL8:
+ bpp = 8;
+ nplanes = 1;
+ pal = (uint32_t *)pict->data[1];
+ break;
+ case PIX_FMT_MONOBLACK:
+ bpp = 1;
+ nplanes = 1;
+ pal = monoblack_pal;
+ break;
+ default:
+ av_log(avctx, AV_LOG_ERROR, "unsupported pixfmt\n");
+ return -1;
+ }
+
+ line_bytes = (avctx->width * bpp + 7) >> 3;
+ line_bytes = (line_bytes + 1) & ~1;
+
+ bytestream_put_byte(&buf, 10); // manufacturer
+ bytestream_put_byte(&buf, 5); // version
+ bytestream_put_byte(&buf, 1); // encoding
+ bytestream_put_byte(&buf, bpp); // bits per pixel per plane
+ bytestream_put_le16(&buf, 0); // x min
+ bytestream_put_le16(&buf, 0); // y min
+ bytestream_put_le16(&buf, avctx->width - 1); // x max
+ bytestream_put_le16(&buf, avctx->height - 1); // y max
+ bytestream_put_le16(&buf, 0); // horizontal DPI
+ bytestream_put_le16(&buf, 0); // vertical DPI
+ for (i = 0; i < 16; i++)
+ bytestream_put_be24(&buf, pal ? pal[i] : 0);// palette (<= 16 color only)
+ bytestream_put_byte(&buf, 0); // reserved
+ bytestream_put_byte(&buf, nplanes); // number of planes
+ bytestream_put_le16(&buf, line_bytes); // scanline plane size in bytes
+
+ while (buf - buf_start < 128)
+ *buf++= 0;
+
+ plane = av_mallocz(line_bytes);
+ src = pict->data[0];
+ src_line_size = (avctx->width * nplanes * bpp + 7) >> 3;
+
+ for (y = 0; y < avctx->height; y++) {
+ for (p = 0; p < nplanes; p++) {
+ for (i = p, j = 0; i < src_line_size; i += nplanes, j++) {
+ plane[j] = src[i];
+ }
+ if ((written = pcx_rle_encode(buf, buf_end - buf,
+ plane, line_bytes)) < 0) {
+ av_log(avctx, AV_LOG_ERROR, "buffer too small\n");
+ av_free(plane);
+ return -1;
+ }
+ buf += written;
+ }
+ src += pict->linesize[0];
+ }
+
+ av_free(plane);
+
+ if (nplanes == 1 && bpp == 8) {
+ if (buf_end - buf < 257) {
+ av_log(avctx, AV_LOG_ERROR, "buffer too small\n");
+ return -1;
+ }
+ bytestream_put_byte(&buf, 12);
+ for (i = 0; i < 256; i++) {
+ bytestream_put_be24(&buf, pal[i]);
+ }
+ }
+
+ return buf - buf_start;
+}
+
+AVCodec pcx_encoder = {
+ "pcx",
+ CODEC_TYPE_VIDEO,
+ CODEC_ID_PCX,
+ sizeof(PCXContext),
+ pcx_encode_init,
+ pcx_encode_frame,
+ NULL,
+ .pix_fmts = (enum PixelFormat[]){
+ PIX_FMT_RGB24,
+ PIX_FMT_RGB8, PIX_FMT_BGR8, PIX_FMT_RGB4_BYTE, PIX_FMT_BGR4_BYTE, PIX_FMT_GRAY8, PIX_FMT_PAL8,
+ PIX_FMT_MONOBLACK,
+ PIX_FMT_NONE},
+ .long_name = NULL_IF_CONFIG_SMALL("PC Paintbrush PCX image"),
+};
diff --git a/libavformat/img2.c b/libavformat/img2.c
index 113f431..48f3dfc 100644
--- a/libavformat/img2.c
+++ b/libavformat/img2.c
@@ -428,7 +428,7 @@ AVOutputFormat image2_muxer = {
"image2",
NULL_IF_CONFIG_SMALL("image2 sequence"),
"",
- "bmp,jpeg,jpg,ljpg,pam,pbm,pgm,pgmyuv,png,ppm,sgi,tif,tiff,jp2",
+ "bmp,jpeg,jpg,ljpg,pam,pbm,pcx,pgm,pgmyuv,png,ppm,sgi,tif,tiff,jp2",
sizeof(VideoData),
CODEC_ID_NONE,
CODEC_ID_MJPEG,
--
1.6.2
More information about the ffmpeg-devel
mailing list