On Thu, Jan 19, 2012 at 09:22:12PM +0000, Paul B Mahol wrote:
> ---
>  Changelog              |    1 +
>  doc/general.texi       |    2 +
>  libavcodec/Makefile    |    2 +
>  libavcodec/allcodecs.c |    1 +
>  libavcodec/avcodec.h   |    1 +
>  libavcodec/version.h   |    2 +-
>  libavcodec/xwddec.c    |  220 ++++++++++++++++++++++++++++++++++++++++
>  libavcodec/xwdenc.c    |  260 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/img2.c     |    3 +-
>  9 files changed, 490 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/xwddec.c
>  create mode 100644 libavcodec/xwdenc.c
> 
> diff --git a/Changelog b/Changelog
> index 3515ba5..559fad3 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -15,6 +15,7 @@ version 0.8_beta2:
>    enable it but it may be removed in a later Libav release.
>  - rv34: frame-level multi-threading
>  - optimized iMDCT transform on x86 using SSE for for mpegaudiodec
> +- XWD decoder and encoder
  
Why not group it with SMJPEG muxer above?
  
>  version 0.8_beta1:
> diff --git a/doc/general.texi b/doc/general.texi
> index 79af887..cda79c0 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -379,6 +379,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
> +@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 1e8d09b..3468f66 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -435,6 +435,8 @@ OBJS-$(CONFIG_XAN_WC4_DECODER)         += xxan.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_YOP_DECODER)             += yop.o
>  OBJS-$(CONFIG_ZLIB_DECODER)            += lcldec.o
>  OBJS-$(CONFIG_ZLIB_ENCODER)            += lclenc.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 5289692..d00bd0b 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -230,6 +230,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_DECODER (YOP, yop);
>      REGISTER_ENCDEC  (ZLIB, zlib);
>      REGISTER_ENCDEC  (ZMBV, zmbv);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index be1b202..967a134 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -254,6 +254,7 @@ enum CodecID {
>      CODEC_ID_VBLE,
>      CODEC_ID_DXTORY,
>      CODEC_ID_V410,
> +    CODEC_ID_XWD,
>  
>      /* various PCM "codecs" */
>      CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at the 
> start of audio codecs
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index c7b4c15..77e1682 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -21,7 +21,7 @@
>  #define AVCODEC_VERSION_H
>  
>  #define LIBAVCODEC_VERSION_MAJOR 53
> -#define LIBAVCODEC_VERSION_MINOR 34
> +#define LIBAVCODEC_VERSION_MINOR 35
>  #define LIBAVCODEC_VERSION_MICRO  0
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> diff --git a/libavcodec/xwddec.c b/libavcodec/xwddec.c
> new file mode 100644
> index 0000000..9615959
> --- /dev/null
> +++ b/libavcodec/xwddec.c
> @@ -0,0 +1,220 @@
> +/*
> + * XWD image format
> + *
> + * Copyright (c) 2012 Paul B Mahol
> + *
> + * This file is part of Libav.
> + *
> + * Libav 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.
> + *
> + * Libav 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 Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +#include "libavutil/imgutils.h"
> +#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, ret, buf_size = avpkt->size;
> +    uint32_t version, header_size, vclass, cmap;
> +    uint32_t xoffset, be, bpp, lsize;
> +    uint32_t pixformat, pixdepth, bunit, bitorder, bpad;
> +    uint32_t rgb[3];
> +    uint8_t *ptr;
> +
> +    if (buf_size < 100)
> +        return AVERROR_INVALIDDATA;
> +
> +    header_size = bytestream_get_be32(&buf);
> +    if (buf_size < header_size)
> +        return AVERROR_INVALIDDATA;
> +
> +    version = bytestream_get_be32(&buf);
> +    if (version != 7) {
> +        av_log(avctx, AV_LOG_ERROR, "unsupported version\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (header_size < 100 || header_size > buf_size) {
> +        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);

looks like you'd better realign (vertically) this chunk

> +    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);
> +    buf      += 8;
> +    cmap      = bytestream_get_be32(&buf);
> +    buf      += header_size - 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);
> +    av_log(avctx, AV_LOG_DEBUG, "red %0x, green %0x, blue %0x\n", rgb[0], 
> rgb[1], rgb[2]);
> +
> +    if (pixformat != 2) {
> +        av_log_missing_feature(avctx, "pixformat <2", 0);

why <2 if it's checked !=2 ?

> +        return AVERROR_PATCHWELCOME;
> +    }
> +
> +    if (bpad % 8)
> +        return AVERROR_INVALIDDATA;
> +    if (cmap > 256)
> +        return AVERROR_INVALIDDATA;

nit: some error messages won't hurt

> +
> +    if ((ret = av_image_check_size(avctx->width, avctx->height, 0, NULL)) < 
> 0)
> +        return ret;
> +
> +    if (buf_size < header_size + cmap * 12 + avctx->height * lsize)
> +        return AVERROR_INVALIDDATA;

you forgot to check lsize for being sane

> +
> +    switch (vclass) {
> +    case 0:
> +    case 1:
> +        if (pixdepth == 1)
> +            avctx->pix_fmt = PIX_FMT_MONOBLACK;
> +        else if (pixdepth == 8)
> +            avctx->pix_fmt = PIX_FMT_GRAY8;
> +        else if (pixdepth == 16)
> +            avctx->pix_fmt = be ? PIX_FMT_GRAY16BE : PIX_FMT_GRAY16LE;
> +        break;
> +    case 2:
> +    case 3:
> +        avctx->pix_fmt = PIX_FMT_PAL8;
> +        break;
> +    case 4:
> +    case 5:
> +        if (bpp == 16 && pixdepth == 15) {
> +            if (rgb[0] == 0x7C00 && rgb[1] == 0x3E0 && rgb[2] == 0x1F)
> +                avctx->pix_fmt = be ? PIX_FMT_RGB555BE : PIX_FMT_RGB555LE;
> +            else if (rgb[0] == 0x1F && rgb[1] == 0x3E0 && rgb[2] == 0x7C00)
> +                avctx->pix_fmt = be ? PIX_FMT_BGR555BE : PIX_FMT_BGR555LE;
> +        } else if (bpp == 16 && pixdepth == 16) {
> +            if (rgb[0] == 0xF800 && rgb[1] == 0x7E0 && rgb[2] == 0x1F)
> +                avctx->pix_fmt = be ? PIX_FMT_RGB565BE : PIX_FMT_RGB565LE;
> +            else if (rgb[0] == 0x1F && rgb[1] == 0x7E0 && rgb[2] == 0xF800)
> +                avctx->pix_fmt = be ? PIX_FMT_BGR565BE : PIX_FMT_BGR565LE;
> +        } 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 masks %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 masks %0X %0X %0X\n", 
> rgb[0], rgb[1], rgb[2]);
> +                return AVERROR(EINVAL);
> +            }
> +        }

This looks monstruous. I suggest making a structure
{ bpp, rmask, gmask, bmask, pixfmt_be, pixfmt_le }
and check data against it in loop (so we can catch wrong pixdepths too).

> +        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);
> +    }

nit: it's better to return error value from get_buffer() here

> +
> +    p->key_frame = 1;
> +    p->pict_type = AV_PICTURE_TYPE_I;
> +
> +    if (avctx->pix_fmt == PIX_FMT_PAL8) {
> +        for (i = 0; i < cmap; i++) {
> +            buf  += 4;
> +#if HAVE_BIGENDIAN
> +            p->data[1][i * 4    ] = 0xFF;
> +            p->data[1][i * 4 + 1] = bytestream_get_be16(&buf) >> 8;
> +            p->data[1][i * 4 + 2] = bytestream_get_be16(&buf) >> 8;
> +            p->data[1][i * 4 + 3] = bytestream_get_be16(&buf) >> 8;
> +#else
> +            p->data[1][i * 4 + 3] = 0xFF;
> +            p->data[1][i * 4 + 2] = bytestream_get_be16(&buf) >> 8;
> +            p->data[1][i * 4 + 1] = bytestream_get_be16(&buf) >> 8;
> +            p->data[1][i * 4    ] = bytestream_get_be16(&buf) >> 8;
> +#endif

I agree with those who proposed to read components into separate variables and
write it once, with AV_WN32A() for example.

> +            buf  += 2;
> +        }
> +    }
> +
> +    ptr = p->data[0];
> +    for (i = 0; i < avctx->height; i++) {
> +        bytestream_get_buffer(&buf, ptr, lsize);
> +        ptr += p->linesize[0];
> +    }

what if lsize happens to be greater than p->linesize[0], we'll get write past
buffer bounds at the last line

> +
> +    *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..db44df2
> --- /dev/null
> +++ b/libavcodec/xwdenc.c
> @@ -0,0 +1,260 @@
> +/*
> + * XWD image format
> + *
> + * Copyright (c) 2012 Paul B Mahol
> + *
> + * This file is part of Libav.
> + *
> + * Libav 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.
> + *
> + * Libav 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 Libav; 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, bpad, cmap = 0, lsize, vclass, be = 0;
> +    uint32_t rgb[3] = { 0 };
> +    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;
> +        } else {
> +            rgb[0] = 0xFF0000;
> +            rgb[1] = 0xFF00;
> +            rgb[2] = 0xFF;
> +        }
> +        bpp = 32;
> +        pixdepth = 24;
> +        lsize = 4 * avctx->width;
> +        vclass = 4;
> +        bpad = 32;

nit: vertical alignment here won't hurt

also I think it might be better to check

 av_pix_fmt_descriptors[pix_fmt].flags & PIX_FMT_BE
for big-endianness (except for ARGB/ABGR of course)

And there's av_get_bits_per_pixel() for actual pixdepth too
(again, except this first case it should work fine)

> +        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;
> +        bpad = 32;
> +        rgb[0] = 0xFF0000;
> +        rgb[1] = 0xFF00;
> +        rgb[2] = 0xFF;
> +        break;
> +    case PIX_FMT_RGB565LE:
> +    case PIX_FMT_RGB565BE:
> +    case PIX_FMT_BGR565LE:
> +    case PIX_FMT_BGR565BE:
> +        if (avctx->pix_fmt == PIX_FMT_RGB565BE ||
> +            avctx->pix_fmt == PIX_FMT_BGR565BE)
> +            be = 1;
> +        if (avctx->pix_fmt == PIX_FMT_BGR565LE ||
> +            avctx->pix_fmt == PIX_FMT_BGR565BE) {
> +            rgb[0] = 0x1F;
> +            rgb[1] = 0x7E0;
> +            rgb[2] = 0xF800;
> +        } else {
> +            rgb[0] = 0xF800;
> +            rgb[1] = 0x7E0;
> +            rgb[2] = 0x1F;
> +        }
> +        bpp = 16;
> +        pixdepth = 16;
> +        lsize = 2 * avctx->width;
> +        vclass = 4;
> +        bpad = 16;
> +        break;
> +    case PIX_FMT_RGB555LE:
> +    case PIX_FMT_RGB555BE:
> +    case PIX_FMT_BGR555LE:
> +    case PIX_FMT_BGR555BE:
> +        if (avctx->pix_fmt == PIX_FMT_RGB555BE ||
> +            avctx->pix_fmt == PIX_FMT_BGR555BE)
> +            be = 1;
> +        if (avctx->pix_fmt == PIX_FMT_BGR555LE ||
> +            avctx->pix_fmt == PIX_FMT_BGR555BE) {
> +            rgb[0] = 0x1F;
> +            rgb[1] = 0x3E0;
> +            rgb[2] = 0x7C00;
> +        } else {
> +            rgb[0] = 0x7C00;
> +            rgb[1] = 0x3E0;
> +            rgb[2] = 0x1F;
> +        }
> +        bpp = 16;
> +        pixdepth = 15;
> +        lsize = 2 * avctx->width;
> +        vclass = 4;
> +        bpad = 16;
> +        break;
> +    case PIX_FMT_RGB8:
> +    case PIX_FMT_BGR8:
> +    case PIX_FMT_RGB4_BYTE:
> +    case PIX_FMT_BGR4_BYTE:
> +    case PIX_FMT_PAL8:
> +        bpp = 8;
> +        pixdepth = 8;
> +        lsize = avctx->width;
> +        vclass = 3;
> +        bpad = 8;
> +        cmap = 256;
> +        break;
> +    case PIX_FMT_GRAY16LE:
> +    case PIX_FMT_GRAY16BE:
> +        if (avctx->pix_fmt == PIX_FMT_GRAY16BE)
> +            be = 1;
> +        bpp = 1;
> +        pixdepth = 16;
> +        lsize = avctx->width * 2;
> +        bpad = 16;
> +        vclass = 0;
> +        break;
> +    case PIX_FMT_MONOBLACK:
> +        bpp = 1;
> +        pixdepth = 1;
> +        lsize = avctx->width / 8;
> +        bpad = 8;
> +        vclass = 0;
> +        break;
> +    default:
> +        av_log(avctx, AV_LOG_INFO, "unsupported pixel format\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    lsize = FFALIGN(lsize, bpad);
> +
> +    if (buf_size < 107 + cmap * 12 + avctx->height * lsize) {
> +        av_log(avctx, AV_LOG_ERROR, "out buffer too small\n");

"output buffer is too small\n"

> +        return AVERROR(ENOMEM);
> +    }
> +

nit: commenting magic values you put here would help somebody looking into
source.

> +    bytestream_put_be32(&buf, 107);
> +    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, bpad);
> +    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);

can it be lavcxwdenc so right tool will be blamed?

> +
> +    for (i = 0; i < cmap; i++) {
> +        bytestream_put_be32(&buf, i);
> +#if HAVE_BIGENDIAN
> +        bytestream_put_be16(&buf, p->data[1][i * 4 + 1] << 8);
> +        bytestream_put_be16(&buf, p->data[1][i * 4 + 2] << 8);
> +        bytestream_put_be16(&buf, p->data[1][i * 4 + 3] << 8);
> +#else
> +        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);
> +#endif

again, should be better to retrieve pal entry as single 32-bit int,
extract RGB and output it

> +        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;

calculate it once at the beginning, store to some variable and check buffer
size and return its value, so if you ever change data size you'll need to
change only one place to calculate it

> +}
> +
> +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_ARGB,
> +                                                 PIX_FMT_ABGR,
> +                                                 PIX_FMT_RGB24,
> +                                                 PIX_FMT_BGR24,
> +                                                 PIX_FMT_RGB565BE,
> +                                                 PIX_FMT_RGB565LE,
> +                                                 PIX_FMT_BGR565BE,
> +                                                 PIX_FMT_BGR565LE,
> +                                                 PIX_FMT_RGB555BE,
> +                                                 PIX_FMT_RGB555LE,
> +                                                 PIX_FMT_BGR555BE,
> +                                                 PIX_FMT_BGR555LE,
> +                                                 PIX_FMT_RGB8,
> +                                                 PIX_FMT_BGR8,
> +                                                 PIX_FMT_RGB4_BYTE,
> +                                                 PIX_FMT_BGR4_BYTE,
> +                                                 PIX_FMT_PAL8,
> +                                                 PIX_FMT_GRAY16BE,
> +                                                 PIX_FMT_GRAY16LE,
> +                                                 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 2af561c..e3e7d11 100644
> --- a/libavformat/img2.c
> +++ b/libavformat/img2.c
> @@ -85,6 +85,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}
>  };
>  
> @@ -501,7 +502,7 @@ AVOutputFormat ff_image2_muxer = {
>      .name           = "image2",
>      .long_name      = NULL_IF_CONFIG_SMALL("image2 sequence"),
>      .extensions     = "bmp,dpx,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,
> -- 
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to