[FFmpeg-devel] [PATCH] XWD decoder and encoder
Nicolas George
nicolas.george at normalesup.org
Thu Jan 19 09:31:12 CET 2012
Le decadi 30 nivôse, an CCXX, Paul B Mahol a écrit :
> ---
> Changelog | 1 +
> doc/general.texi | 2 +
> libavcodec/Makefile | 2 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/xwddec.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++++
> libavcodec/xwdenc.c | 183 ++++++++++++++++++++++++++++++++++++++++++++
> libavformat/img2.c | 3 +-
> 8 files changed, 390 insertions(+), 1 deletions(-)
> create mode 100644 libavcodec/xwddec.c
> create mode 100644 libavcodec/xwdenc.c
>
> diff --git a/Changelog b/Changelog
> index d6c5457..abe8ffc 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -26,6 +26,7 @@ version next:
> - ffprobe -show_program_version, -show_library_versions, -show_versions options
> - rv34: frame-level multi-threading
> - optimized iMDCT transform on x86 using SSE for for mpegaudiodec
> +- XWD decoder and encoder
>
>
> version 0.9:
> diff --git a/doc/general.texi b/doc/general.texi
> index b5847c9..b1b0e4b 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -399,6 +399,8 @@ following image formats are supported:
> @tab YUV, JPEG and some extension is not supported yet.
> @item Truevision Targa @tab X @tab X
> @tab Targa (.TGA) image format
> + at item XWD @tab X @tab X
> + @tab X Window Dump image format
> @end multitable
>
> @code{X} means that encoding (resp. decoding) is supported.
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index f094d16..5942f67 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -470,6 +470,8 @@ OBJS-$(CONFIG_XBIN_DECODER) += bintext.o cga_data.o
> OBJS-$(CONFIG_XL_DECODER) += xl.o
> OBJS-$(CONFIG_XSUB_DECODER) += xsubdec.o
> OBJS-$(CONFIG_XSUB_ENCODER) += xsubenc.o
> +OBJS-$(CONFIG_XWD_DECODER) += xwddec.o
> +OBJS-$(CONFIG_XWD_ENCODER) += xwdenc.o
> OBJS-$(CONFIG_Y41P_DECODER) += y41pdec.o
> OBJS-$(CONFIG_Y41P_ENCODER) += y41penc.o
> OBJS-$(CONFIG_YOP_DECODER) += yop.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 1e36235..9ce50d2 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -245,6 +245,7 @@ void avcodec_register_all(void)
> REGISTER_DECODER (XAN_WC3, xan_wc3);
> REGISTER_DECODER (XAN_WC4, xan_wc4);
> REGISTER_DECODER (XL, xl);
> + REGISTER_ENCDEC (XWD, xwd);
> REGISTER_ENCDEC (Y41P, y41p);
> REGISTER_DECODER (YOP, yop);
> REGISTER_ENCDEC (YUV4, yuv4);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 215bfc9..cbcf4e3 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -255,6 +255,7 @@ enum CodecID {
> CODEC_ID_VBLE,
> CODEC_ID_DXTORY,
> CODEC_ID_V410,
> + CODEC_ID_XWD,
We need to use MKBETAG, or to sneak it into libav before applying, or we
break compatibility.
> CODEC_ID_Y41P = MKBETAG('Y','4','1','P'),
> CODEC_ID_UTVIDEO = 0x800,
> CODEC_ID_ESCAPE130 = MKBETAG('E','1','3','0'),
> diff --git a/libavcodec/xwddec.c b/libavcodec/xwddec.c
> new file mode 100644
> index 0000000..36a649e
> --- /dev/null
> +++ b/libavcodec/xwddec.c
> @@ -0,0 +1,198 @@
> +/*
> + * XWD image format
> + *
> + * Copyright (c) 2012 Paul B Mahol
> + *
> + * 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
> + */
> +
> +#include "avcodec.h"
> +#include "bytestream.h"
> +
> +static av_cold int xwd_decode_init(AVCodecContext *avctx)
> +{
> + avctx->coded_frame = avcodec_alloc_frame();
> + if (!avctx->coded_frame)
> + return AVERROR(ENOMEM);
> +
> + return 0;
> +}
> +
> +static int xwd_decode_frame(AVCodecContext *avctx, void *data,
> + int *data_size, AVPacket *avpkt)
> +{
> + AVFrame *p = avctx->coded_frame;
> + const uint8_t *buf = avpkt->data;
> + int i, buf_size = avpkt->size;
> + int version, hsize, vclass, cmap;
> + int xoffset, be, bpp, lsize;
> + int pixformat, pixdepth, bunit, bitorder, bpad;
> + uint32_t rgb[3], pal[256];
> + uint8_t *ptr;
> +
> + hsize = bytestream_get_be32(&buf);
Before that:
if (buf_size < 8)
return AVERROR_INVALIDDATA;
Also, it took me some time to realize that hsize does not mean
horizontal_size. I blame TeX, but header_size would still probably be more
reader-friendly.
> + if (buf_size < hsize)
> + return AVERROR_INVALIDDATA;
> +
> + version = bytestream_get_be32(&buf);
> + if (version != 7) {
> + av_log(avctx, AV_LOG_ERROR, "unsupported version\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if (hsize < 100) {
> + av_log(avctx, AV_LOG_ERROR, "invalid header size\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + pixformat = bytestream_get_be32(&buf);
> + pixdepth = bytestream_get_be32(&buf);
> + avctx->width = bytestream_get_be32(&buf);
> + avctx->height = bytestream_get_be32(&buf);
> + xoffset = bytestream_get_be32(&buf);
> + be = bytestream_get_be32(&buf);
> + bunit = bytestream_get_be32(&buf);
> + bitorder = bytestream_get_be32(&buf);
> + bpad = bytestream_get_be32(&buf);
> + bpp = bytestream_get_be32(&buf);
> + lsize = bytestream_get_be32(&buf);
> + vclass = bytestream_get_be32(&buf);
> + rgb[0] = bytestream_get_be32(&buf);
> + rgb[1] = bytestream_get_be32(&buf);
> + rgb[2] = bytestream_get_be32(&buf);
Some alignment would probably be nice.
> + buf += 8;
> + cmap = bytestream_get_be32(&buf);
> + buf += hsize - 80;
> +
> + av_log(avctx, AV_LOG_DEBUG, "pixformat %d, pixdepth %d, bunit %d, bitorder %d, bpad %d\n",
> + pixformat, pixdepth, bunit, bitorder, bpad);
> + av_log(avctx, AV_LOG_DEBUG, "vclass %d, cmap %d, bpp %d, be %d, lsize %d, xoffset %d\n",
> + vclass, cmap, bpp, be, lsize, xoffset);
> +
> + if (pixformat != 2) {
> + av_log_missing_feature(avctx, "pixformat <2", 0);
> + return AVERROR_PATCHWELCOME;
> + }
> +
> + switch (vclass) {
> + case 0:
> + case 1:
> + avctx->pix_fmt = PIX_FMT_GRAY8;
> + break;
> + case 2:
> + case 3:
> + {
> + uint8_t red, green, blue;
> +
> + if (cmap > 256)
> + return AVERROR(EINVAL);
> +
> + if (buf_size < avctx->height * avctx->width + cmap * 12 + hsize)
Possible arithmetic overflow with untrusted data.
> + return AVERROR_INVALIDDATA;
> +
> + for (i = 0; i < cmap; i++) {
> + buf += 4;
> + red = bytestream_get_be16(&buf) >> 8;
> + green = bytestream_get_be16(&buf) >> 8;
> + blue = bytestream_get_be16(&buf) >> 8;
It looks like each entry is 16-bits. Are there no palleted format with more
dept in the palette entries?
> + buf += 2;
> + pal[i] = (0xFF << 24) | (red << 16) | (green << 8) | blue;
> + }
> + avctx->pix_fmt = PIX_FMT_PAL8;
> + }
> + break;
> + case 4:
> + case 5:
> + if (buf_size < bpp * avctx->height * avctx->width / 8 + cmap * 12 + hsize)
> + return AVERROR_INVALIDDATA;
Again, possible arithmetic overflow with untrusted data.
> +
> + if (bpp == 16)
> + avctx->pix_fmt = be ? PIX_FMT_RGB555BE : PIX_FMT_RGB555LE;
> + else if (bpp == 24) {
> + if (rgb[0] == 0x00FF0000 && rgb[1] == 0x0000FF00 && rgb[2] == 0x000000FF)
> + avctx->pix_fmt = be ? PIX_FMT_RGB24 : PIX_FMT_BGR24;
> + else if (rgb[0] == 0x000000FF && rgb[1] == 0x0000FF00 && rgb[2] == 0x00FF0000)
> + avctx->pix_fmt = be ? PIX_FMT_BGR24 : PIX_FMT_RGB24;
> + else {
> + av_log(avctx, AV_LOG_ERROR, "Unknown bitfields %0X %0X %0X\n", rgb[0], rgb[1], rgb[2]);
> + return AVERROR(EINVAL);
> + }
> + } else if (bpp == 32) {
> + if (rgb[0] == 0x00FF0000 && rgb[1] == 0x0000FF00 && rgb[2] == 0x000000FF)
> + avctx->pix_fmt = be ? PIX_FMT_ARGB : PIX_FMT_BGRA;
> + else if (rgb[0] == 0x000000FF && rgb[1] == 0x0000FF00 && rgb[2] == 0x00FF0000)
> + avctx->pix_fmt = be ? PIX_FMT_ABGR : PIX_FMT_RGBA;
> + else {
> + av_log(avctx, AV_LOG_ERROR, "Unknown bitfields %0X %0X %0X\n", rgb[0], rgb[1], rgb[2]);
> + return AVERROR(EINVAL);
> + }
> + }
I believe it could be made simpler:
int rgb = (rgb[0] == 0x00FF0000 && rgb[1] == 0x0000FF00 && rgb[2] == 0x000000FF) -
(rgb[0] == 0x000000FF && rgb[1] == 0x0000FF00 && rgb[2] == 0x00FF0000);
/* rgb == 1 -> RGB; rgb == -1 -> BGR, rgb == 0 -> unknown */
...
else if (bpp == 24 || bpp == 32) {
static const enum PixelFormat pix_fmt_map[2][2][2] = {
/* 24 bpp */
{ { PIX_FMT_RGB24, PIX_FMT_BGR24 }, /* RGB */
{ PIX_FMT_BGR24, PIX_FMT_RGB24 } }, /* BGR */
/* 32 bpp */
{ { PIX_FMT_ARGB, PIX_FMT_BGRA }, /* RGB */
{ PIX_FMT_ABGR, PIX_FMT_RGBA } } /* BGR */
};
if (!rgb) {
av_log(avctx, AV_LOG_ERROR, "Unknown bitfields %0X %0X %0X\n", rgb[0], rgb[1], rgb[2]);
return AVERROR(EINVAL);
}
avctx->pix_fmt = pix_fmt_map[bpp == 32][rgb < 0][!be];
}
> + buf += cmap * 12;
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "invalid image format\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if (p->data[0])
> + avctx->release_buffer(avctx, p);
> +
> + p->reference = 0;
> + if (avctx->get_buffer(avctx, p) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> + return AVERROR(ENOMEM);
> + }
> +
> + p->key_frame = 1;
> + p->pict_type = AV_PICTURE_TYPE_I;
> +
> + if (avctx->pix_fmt == PIX_FMT_PAL8)
> + memcpy(p->data[1], pal, AVPALETTE_SIZE);
> +
> + ptr = p->data[0];
> + for (i = 0; i < avctx->height; i++) {
> + memcpy(ptr, buf, p->linesize[0]);
If the stride if the AVFrame is larger than the stride of the encoded data,
it will lead to reads beyond the end of the packet. Copying width*bpp/8
seems more reasonable.
> + buf += xoffset + lsize;
> + ptr += p->linesize[0];
> + }
> +
> + *data_size = sizeof(AVFrame);
> + *(AVFrame *)data = *p;
> +
> + return buf_size;
> +}
> +
> +static av_cold int xwd_decode_close(AVCodecContext *avctx)
> +{
> + if (avctx->coded_frame->data[0])
> + avctx->release_buffer(avctx, avctx->coded_frame);
> +
> + av_freep(&avctx->coded_frame);
> +
> + return 0;
> +}
> +
> +AVCodec ff_xwd_decoder = {
> + .name = "xwd",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = CODEC_ID_XWD,
> + .init = xwd_decode_init,
> + .close = xwd_decode_close,
> + .decode = xwd_decode_frame,
> + .capabilities = CODEC_CAP_DR1,
> + .long_name = NULL_IF_CONFIG_SMALL("XWD (X Window Dump) image"),
> +};
> diff --git a/libavcodec/xwdenc.c b/libavcodec/xwdenc.c
> new file mode 100644
> index 0000000..d87b6c8
> --- /dev/null
> +++ b/libavcodec/xwdenc.c
> @@ -0,0 +1,183 @@
> +/*
> + * XWD image format
> + *
> + * Copyright (c) 2012 Paul B Mahol
> + *
> + * 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
> + */
> +
> +#include "avcodec.h"
> +#include "bytestream.h"
> +
> +static av_cold int xwd_encode_init(AVCodecContext *avctx)
> +{
> + avctx->coded_frame = avcodec_alloc_frame();
> + if (!avctx->coded_frame)
> + return AVERROR(ENOMEM);
> +
> + return 0;
> +}
> +
> +static int xwd_encode_frame(AVCodecContext *avctx, uint8_t *buf,
> + int buf_size, void *data)
> +{
> + AVFrame *p = data;
> + uint32_t pixdepth, bpp, cmap = 0, lsize, vclass, be = 0;
> + uint32_t rgb[3] = { 0xFF0000, 0xFF00, 0xFF };
> + int i;
> + uint8_t *ptr;
> +
> + avctx->coded_frame->key_frame = 1;
> + avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I;
> +
> + switch (avctx->pix_fmt) {
> + case PIX_FMT_ARGB:
> + case PIX_FMT_BGRA:
> + case PIX_FMT_RGBA:
> + case PIX_FMT_ABGR:
> + if (avctx->pix_fmt == PIX_FMT_ARGB ||
> + avctx->pix_fmt == PIX_FMT_ABGR)
> + be = 1;
> + if (avctx->pix_fmt == PIX_FMT_ABGR ||
> + avctx->pix_fmt == PIX_FMT_RGBA) {
> + rgb[0] = 0xFF;
> + rgb[1] = 0xFF00;
> + rgb[2] = 0xFF0000;
> + }
> + bpp = 32;
> + pixdepth = 24;
> + lsize = 4 * avctx->width;
> + vclass = 4;
> + break;
> + case PIX_FMT_BGR24:
> + case PIX_FMT_RGB24:
> + if (avctx->pix_fmt == PIX_FMT_RGB24)
> + be = 1;
> + bpp = 24;
> + pixdepth = 24;
> + lsize = 3 * avctx->width;
> + vclass = 4;
> + break;
> + case PIX_FMT_RGB555LE:
> + case PIX_FMT_RGB555BE:
> + if (avctx->pix_fmt == PIX_FMT_RGB555BE)
> + be = 1;
> + bpp = 16;
> + pixdepth = 16;
> + lsize = 2 * avctx->width;
> + vclass = 4;
> + 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;
> + pixdepth = 8;
> + lsize = avctx->width;
> + vclass = 3;
> + cmap = 256;
> + break;
> + case PIX_FMT_MONOBLACK:
> + bpp = 1;
> + pixdepth = 1;
> + lsize = avctx->width / 8;
> + vclass = 0;
> + break;
> + default:
> + av_log(avctx, AV_LOG_INFO, "unsupported pixel format\n");
> + return AVERROR(EINVAL);
> + }
> +
> + lsize = FFALIGN(lsize, 32);
> +
> + bytestream_put_be32(&buf, 107);
Is it normal that you never validated buf_size?
> + bytestream_put_be32(&buf, 7);
> + bytestream_put_be32(&buf, 2);
> + bytestream_put_be32(&buf, pixdepth);
> + bytestream_put_be32(&buf, avctx->width);
> + bytestream_put_be32(&buf, avctx->height);
> + bytestream_put_be32(&buf, 0);
> + bytestream_put_be32(&buf, be);
> + bytestream_put_be32(&buf, 32);
> + bytestream_put_be32(&buf, be);
> + bytestream_put_be32(&buf, 32);
> + bytestream_put_be32(&buf, bpp);
> + bytestream_put_be32(&buf, lsize);
> + bytestream_put_be32(&buf, vclass);
> + bytestream_put_be32(&buf, rgb[0]);
> + bytestream_put_be32(&buf, rgb[1]);
> + bytestream_put_be32(&buf, rgb[2]);
> + bytestream_put_be32(&buf, 8);
> + bytestream_put_be32(&buf, cmap);
> + bytestream_put_be32(&buf, cmap);
> + bytestream_put_be32(&buf, avctx->width);
> + bytestream_put_be32(&buf, avctx->height);
> + bytestream_put_be32(&buf, 0);
> + bytestream_put_be32(&buf, 0);
> + bytestream_put_be32(&buf, 0);
> + bytestream_put_buffer(&buf, "xwdenc", 7);
> +
> + for (i = 0; i < cmap; i++) {
> + bytestream_put_be32(&buf, i);
> + bytestream_put_be16(&buf, p->data[1][i * 4 + 2] << 8);
> + bytestream_put_be16(&buf, p->data[1][i * 4 + 1] << 8);
> + bytestream_put_be16(&buf, p->data[1][i * 4 + 0] << 8);
"* 0x101" would be more accurate.
> + bytestream_put_byte(&buf, 0x7);
> + bytestream_put_byte(&buf, 0x28);
> + }
> +
> + ptr = p->data[0];
> + for (i = 0; i < avctx->height; i++) {
> + bytestream_put_buffer(&buf, ptr, lsize);
> + ptr += p->linesize[0];
> + }
> +
> + return 107 + cmap * 12 + avctx->height * lsize;
> +}
> +
> +static av_cold int xwd_encode_close(AVCodecContext *avctx)
> +{
> + av_freep(&avctx->coded_frame);
> +
> + return 0;
> +}
> +
> +AVCodec ff_xwd_encoder = {
> + .name = "xwd",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = CODEC_ID_XWD,
> + .init = xwd_encode_init,
> + .encode = xwd_encode_frame,
> + .close = xwd_encode_close,
> + .pix_fmts = (const enum PixelFormat[]) { PIX_FMT_BGRA,
> + PIX_FMT_RGBA,
> + PIX_FMT_RGB24,
> + PIX_FMT_BGR24,
> + PIX_FMT_RGB555BE,
> + PIX_FMT_RGB555LE,
> + 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("XWD (X Window Dump) image"),
> +};
> diff --git a/libavformat/img2.c b/libavformat/img2.c
> index bc35591..80859c3 100644
> --- a/libavformat/img2.c
> +++ b/libavformat/img2.c
> @@ -90,6 +90,7 @@ static const IdStrMap img_tags[] = {
> { CODEC_ID_JPEG2000 , "jpc"},
> { CODEC_ID_DPX , "dpx"},
> { CODEC_ID_PICTOR , "pic"},
> + { CODEC_ID_XWD , "xwd"},
> { CODEC_ID_NONE , NULL}
> };
>
> @@ -527,7 +528,7 @@ AVOutputFormat ff_image2_muxer = {
> .name = "image2",
> .long_name = NULL_IF_CONFIG_SMALL("image2 sequence"),
> .extensions = "bmp,dpx,jls,jpeg,jpg,ljpg,pam,pbm,pcx,pgm,pgmyuv,png,"
> - "ppm,sgi,tga,tif,tiff,jp2",
> + "ppm,sgi,tga,tif,tiff,jp2,xwd",
> .priv_data_size = sizeof(VideoData),
> .video_codec = CODEC_ID_MJPEG,
> .write_header = write_header,
> --
> 1.7.7
Hope this helps, thanks for the good work.
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120119/dfbfa195/attachment.asc>
More information about the ffmpeg-devel
mailing list