On Fri, Jan 20, 2012 at 11:36:26PM +0000, Paul B Mahol wrote:
> Fixed patch (with additional fixes) attached.

> From e03c2261ae641f7b70b00b69973f79fff54a2271 Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <[email protected]>
> Date: Thu, 19 Jan 2012 20:54:02 +0000
> Subject: [PATCH] XWD encoder and decoder
> 
> ---
>  Changelog              |    1 +
>  doc/general.texi       |    2 +
>  libavcodec/Makefile    |    2 +
>  libavcodec/allcodecs.c |    1 +
>  libavcodec/avcodec.h   |    1 +
>  libavcodec/version.h   |    2 +-
>  libavcodec/xwd.h       |   41 +++++++
>  libavcodec/xwddec.c    |  252 ++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/xwdenc.c    |  273 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/img2.c     |    3 +-
>  10 files changed, 576 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/xwd.h
>  create mode 100644 libavcodec/xwddec.c
>  create mode 100644 libavcodec/xwdenc.c
> 
[...]
> diff --git a/libavcodec/xwddec.c b/libavcodec/xwddec.c
> new file mode 100644
> index 0000000..0c8cacf
> --- /dev/null
> +++ b/libavcodec/xwddec.c
> @@ -0,0 +1,252 @@
> +/*
> + * 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"
> +#include "xwd.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, ncolors;
> +    uint32_t xoffset, be, bpp, lsize, rsize;
> +    uint32_t pixformat, pixdepth, bunit, bitorder, bpad;
> +    uint32_t rgb[3];
> +    uint8_t *ptr;
> +
> +    if (buf_size < XWD_HEADER_SIZE)
> +        return AVERROR_INVALIDDATA;
> +
> +    header_size = bytestream_get_be32(&buf);
> +    if (buf_size < header_size)
> +        return AVERROR_INVALIDDATA;
> +
> +    version = bytestream_get_be32(&buf);
> +    if (version != XWD_VERSION) {
> +        av_log(avctx, AV_LOG_ERROR, "unsupported version\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (header_size < XWD_HEADER_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);
> +    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;
> +    ncolors       = bytestream_get_be32(&buf);
> +    buf          += header_size - (XWD_HEADER_SIZE - 20);
> +
> +    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, ncolors %d, bpp %d, be %d, lsize 
> %d, xoffset %d\n",
> +           vclass, ncolors, 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 > XWD_Z_PIXMAP) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid pixmap format\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (pixdepth == 0 || pixdepth > 32) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid pixmap depth\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (bpad != 8 && bpad != 16 && bpad != 32) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid bitmap scan-line pad\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (bpp == 0 || bpp > 32) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid bits per pixel\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (ncolors > 256) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid number of entries in 
> colormap\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if ((ret = av_image_check_size(avctx->width, avctx->height, 0, NULL)) < 
> 0)
> +        return ret;
> +
> +    rsize = FFALIGN(avctx->width * bpp, bpad) / 8;
> +    if (lsize < rsize) {
> +        av_log(avctx, AV_LOG_ERROR, "invalid bytes per scan-line\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (buf_size < header_size + ncolors * XWD_CMAP_SIZE + avctx->height * 
> lsize) {
> +        av_log(avctx, AV_LOG_ERROR, "input buffer too small\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    if (pixformat != XWD_Z_PIXMAP) {
> +        av_log(avctx, AV_LOG_ERROR, "pixmap format %d unsupported\n", 
> pixformat);
> +        return AVERROR_PATCHWELCOME;

nit: av_log_ask_for_sample ?

[...]
> +    if (avctx->pix_fmt == PIX_FMT_NONE) {
> +        av_log_ask_for_sample(avctx, "unknown file\n");

too late to disown it :) print visual+bpp combination that lead to no pixfmt

[...]
> diff --git a/libavcodec/xwdenc.c b/libavcodec/xwdenc.c
> new file mode 100644
> index 0000000..8f27ab2
> --- /dev/null
> +++ b/libavcodec/xwdenc.c
> @@ -0,0 +1,273 @@
> +/*
> + * 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/pixdesc.h"
> +#include "avcodec.h"
> +#include "bytestream.h"
> +#include "xwd.h"
> +
> +#define WINDOW_NAME         "lavcxwdenc"
> +#define WINDOW_NAME_SIZE    strlen(WINDOW_NAME) + 1
> +
> +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;
> +    enum PixelFormat pix_fmt = avctx->pix_fmt;
> +    uint32_t pixdepth, bpp, bpad, ncolors = 0, lsize, vclass, be = 0;
> +    uint32_t rgb[3] = { 0 };
> +    uint32_t header_size;
> +    int i, out_size;
> +    uint8_t *ptr;
> +
> +    switch (pix_fmt) {
> +    case PIX_FMT_ARGB:
> +    case PIX_FMT_BGRA:
> +    case PIX_FMT_RGBA:
> +    case PIX_FMT_ABGR:
> +        if (pix_fmt == PIX_FMT_ARGB ||
> +            pix_fmt == PIX_FMT_ABGR)
> +            be = 1;
> +        if (pix_fmt == PIX_FMT_ABGR ||
> +            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   = XWD_TRUE_COLOR;
> +        bpad     = 32;
> +        break;
> +    case PIX_FMT_BGR24:
> +    case PIX_FMT_RGB24:
> +        if (pix_fmt == PIX_FMT_RGB24)
> +            be = 1;
> +        bpp      = 24;
> +        pixdepth = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);
> +        lsize    = 3 * avctx->width;
> +        vclass   = XWD_TRUE_COLOR;
> +        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 (av_pix_fmt_descriptors[pix_fmt].flags & PIX_FMT_BE)
> +            be = 1;

just invoke this before the switch{} along with pixdepth, correct be and
pixdepth in cases where it's needed (like ARGB) and calculate lsize after the
switch, it's (bpp * width) >> 3, isn't it?

> +        if (pix_fmt == PIX_FMT_BGR565LE ||
> +            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 = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);
> +        lsize    = 2 * avctx->width;
> +        vclass   = XWD_TRUE_COLOR;
> +        bpad     = 16;
> +        break;
> +    case PIX_FMT_RGB555LE:
> +    case PIX_FMT_RGB555BE:
> +    case PIX_FMT_BGR555LE:
> +    case PIX_FMT_BGR555BE:
> +        if (av_pix_fmt_descriptors[pix_fmt].flags & PIX_FMT_BE)
> +            be = 1;
> +        if (pix_fmt == PIX_FMT_BGR555LE ||
> +            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 = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);
> +        lsize    = 2 * avctx->width;
> +        vclass   = XWD_TRUE_COLOR;
> +        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 = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);
> +        lsize    = avctx->width;
> +        vclass   = XWD_PSEUDO_COLOR;
> +        bpad     = 8;
> +        ncolors  = 256;
> +        break;
> +    case PIX_FMT_GRAY16LE:
> +    case PIX_FMT_GRAY16BE:
> +        if (av_pix_fmt_descriptors[pix_fmt].flags & PIX_FMT_BE)
> +            be = 1;
> +        bpp      = 1;

and this looks suspicious

> +        pixdepth = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);
> +        lsize    = avctx->width * 2;
> +        bpad     = 16;
> +        vclass   = XWD_STATIC_GRAY;
> +        break;
> +    case PIX_FMT_MONOWHITE:
> +        bpp      = 1;
> +        pixdepth = av_get_bits_per_pixel(&av_pix_fmt_descriptors[pix_fmt]);
> +        lsize    = avctx->width / 8;
> +        bpad     = 8;
> +        vclass   = XWD_STATIC_GRAY;
> +        break;
> +    default:
> +        av_log(avctx, AV_LOG_INFO, "unsupported pixel format\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    lsize       = FFALIGN(lsize * 8, bpad) / 8;
> +    header_size = XWD_HEADER_SIZE + WINDOW_NAME_SIZE;
> +    out_size    = header_size + ncolors * XWD_CMAP_SIZE + avctx->height * 
> lsize;
> +
> +    if (buf_size < out_size) {
> +        av_log(avctx, AV_LOG_ERROR, "output buffer too small\n");
> +        return AVERROR(ENOMEM);
> +    }
> +
> +    avctx->coded_frame->key_frame = 1;
> +    avctx->coded_frame->pict_type = AV_PICTURE_TYPE_I;
> +
> +    bytestream_put_be32(&buf, header_size);
> +    bytestream_put_be32(&buf, XWD_VERSION);   // file version
> +    bytestream_put_be32(&buf, XWD_Z_PIXMAP);  // pixmap format
> +    bytestream_put_be32(&buf, pixdepth);      // pixmap depth in pixels
> +    bytestream_put_be32(&buf, avctx->width);  // pixmap width in pixels
> +    bytestream_put_be32(&buf, avctx->height); // pixmap height in pixels
> +    bytestream_put_be32(&buf, 0);             // bitmap x offset
> +    bytestream_put_be32(&buf, be);            // byte order
> +    bytestream_put_be32(&buf, 32);            // bitmap unit
> +    bytestream_put_be32(&buf, be);            // bit-order of image data
> +    bytestream_put_be32(&buf, bpad);          // bitmap scan-line pad in bits
> +    bytestream_put_be32(&buf, bpp);           // bits per pixel
> +    bytestream_put_be32(&buf, lsize);         // bytes per scan-line
> +    bytestream_put_be32(&buf, vclass);        // visual class
> +    bytestream_put_be32(&buf, rgb[0]);        // red mask
> +    bytestream_put_be32(&buf, rgb[1]);        // green mask
> +    bytestream_put_be32(&buf, rgb[2]);        // blue mask
> +    bytestream_put_be32(&buf, 8);             // size of each bitmask in bits
> +    bytestream_put_be32(&buf, ncolors);       // number of colors
> +    bytestream_put_be32(&buf, ncolors);       // number of entries in color 
> map
> +    bytestream_put_be32(&buf, avctx->width);  // window width
> +    bytestream_put_be32(&buf, avctx->height); // window height
> +    bytestream_put_be32(&buf, 0);             // window upper left X 
> coordinate
> +    bytestream_put_be32(&buf, 0);             // window upper left Y 
> coordinate
> +    bytestream_put_be32(&buf, 0);             // window border width
> +    bytestream_put_buffer(&buf, WINDOW_NAME, WINDOW_NAME_SIZE);
> +
> +    for (i = 0; i < ncolors; i++) {
> +        bytestream_put_be32(&buf, i);  // colormap entry number
> +#if HAVE_BIGENDIAN
> +        *(buf++) = p->data[1][i * 4 + 1];
> +        *(buf++) = p->data[1][i * 4 + 1];
> +        *(buf++) = p->data[1][i * 4 + 2];
> +        *(buf++) = p->data[1][i * 4 + 2];
> +        *(buf++) = p->data[1][i * 4 + 3];
> +        *(buf++) = p->data[1][i * 4 + 3];
> +#else
> +        *(buf++) = p->data[1][i * 4 + 2];
> +        *(buf++) = p->data[1][i * 4 + 2];
> +        *(buf++) = p->data[1][i * 4 + 1];
> +        *(buf++) = p->data[1][i * 4 + 1];
> +        *(buf++) = p->data[1][i * 4 + 0];
> +        *(buf++) = p->data[1][i * 4 + 0];
> +#endif

Why do you think this approach is better?
Also *buf++ is enough, save parentheses for other situations.

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to