On 03/17/2014 12:08 AM, Vittorio Giovara wrote:
> From: Aleksi Nurmi <[email protected]>
> 
> Further enhancements by Paul B Mahol <[email protected]>.
> 
> Signed-off-by: Vittorio Giovara <[email protected]>
> ---
> In the original code there is support for 0RGB format, which we don't have.
> However I am not sure of the usefulness of that format, so for now I preferred
> disabling it here instead of supporting yet another pixel format.

This is a decoder, not an encoder. We should support the format because
it exists and should be trivial to support. Just convert the 4-byte
"0RGB" to 3-byte RGB. Or if you prefer, do another pass after copying to
change the alpha bytes from 0 to 255.

> Vittorio
> 
>  Changelog               |   1 +
>  doc/general.texi        |   2 +
>  libavcodec/Makefile     |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/avcodec.h    |   1 +
>  libavcodec/codec_desc.c |   7 ++
>  libavcodec/pix.c        | 206 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h    |   4 +-
>  libavformat/img2.c      |   1 +
>  9 files changed, 222 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/pix.c
> 
> diff --git a/Changelog b/Changelog
> index 279c0d8..cd56ac3 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -4,6 +4,7 @@ releases are sorted from youngest to oldest.
>  version <next>:
>  - compand audio filter
>  - shuffleplanes filter
> +- BRender PIX image decoder
>  
>  
>  version 10:
> diff --git a/doc/general.texi b/doc/general.texi
> index 8c0cb1b..b8ecea1 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -449,6 +449,8 @@ following image formats are supported:
>      @tab PGM with U and V components in YUV 4:2:0
>  @item PIC          @tab @tab X
>      @tab Pictor/PC Paint
> +@item PIX          @tab   @tab X
> +    @tab PIX is an image format used in the Argonaut BRender 3D engine.
>  @item PNG          @tab X @tab X
>      @tab 2/4 bpp not supported yet
>  @item PPM          @tab X @tab X
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index c04b3f1..fa5319c 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -282,6 +282,7 @@ OBJS-$(CONFIG_PGMYUV_DECODER)          += pnmdec.o pnm.o
>  OBJS-$(CONFIG_PGMYUV_ENCODER)          += pnmenc.o
>  OBJS-$(CONFIG_PGSSUB_DECODER)          += pgssubdec.o
>  OBJS-$(CONFIG_PICTOR_DECODER)          += pictordec.o cga_data.o
> +OBJS-$(CONFIG_PIX_DECODER)             += pix.o
>  OBJS-$(CONFIG_PNG_DECODER)             += png.o pngdec.o pngdsp.o
>  OBJS-$(CONFIG_PNG_ENCODER)             += png.o pngenc.o
>  OBJS-$(CONFIG_PPM_DECODER)             += pnmdec.o pnm.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index ed6d7ff..74eb253 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -204,6 +204,7 @@ void avcodec_register_all(void)
>      REGISTER_ENCDEC (PGM,               pgm);
>      REGISTER_ENCDEC (PGMYUV,            pgmyuv);
>      REGISTER_DECODER(PICTOR,            pictor);
> +    REGISTER_DECODER(PIX,               pix);
>      REGISTER_ENCDEC (PNG,               png);
>      REGISTER_ENCDEC (PPM,               ppm);
>      REGISTER_ENCDEC (PRORES,            prores);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 7beb277..fe525e6 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -284,6 +284,7 @@ enum AVCodecID {
>      AV_CODEC_ID_HNM4_VIDEO,
>      AV_CODEC_ID_HEVC,
>      AV_CODEC_ID_FIC,
> +    AV_CODEC_ID_PIX,
>  
>      /* various PCM "codecs" */
>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at the 
> start of audio codecs
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 2ad5326..878e37a 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -1166,6 +1166,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>      },
>      {
> +        .id        = AV_CODEC_ID_PIX,
> +        .type      = AVMEDIA_TYPE_VIDEO,
> +        .name      = "pix",
> +        .long_name = NULL_IF_CONFIG_SMALL("BRender PIX image"),
> +        .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> +    },
> +    {
>          .id        = AV_CODEC_ID_PNG,
>          .type      = AVMEDIA_TYPE_VIDEO,
>          .name      = "png",
> diff --git a/libavcodec/pix.c b/libavcodec/pix.c
> new file mode 100644
> index 0000000..5b42ce2
> --- /dev/null
> +++ b/libavcodec/pix.c
> @@ -0,0 +1,206 @@
> +/*
> + * BRender PIX (.pix) image decoder
> + * Copyright (c) 2012 Aleksi Nurmi
> + *
> + * 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
> + */
> +
> +/*
> + * Tested against samples from I-War / Independence War and Defiance.
> + * If the PIX file does not contain a palette, the
> + * palette_has_changed property of the AVFrame is set to 0.
> + */

So is it an image format or a video format? What is the user supposed to
do with a paletted image without a palette?

> +
> +#include "libavutil/imgutils.h"
> +
> +#include "avcodec.h"
> +#include "bytestream.h"
> +#include "internal.h"
> +
> +typedef struct PixHeader {
> +    int format;
> +    unsigned int width, height;
> +} PixHeader;
> +
> +static int pix_decode_header(PixHeader *out, GetByteContext *pgb)
> +{
> +    unsigned int header_len = bytestream2_get_be32(pgb);
> +
> +    out->format = bytestream2_get_byte(pgb);
> +    bytestream2_skip(pgb, 2);
> +    out->width  = bytestream2_get_be16(pgb);
> +    out->height = bytestream2_get_be16(pgb);
> +
> +    // the header is at least 11 bytes long; we read the first 7
> +    if (header_len < 11) {
> +        return -1;
> +    }
> +
> +    // skip the rest of the header
> +    bytestream2_skip(pgb, header_len - 7);
> +
> +    return 0;
> +}
> +
> +static int pix_decode_frame(AVCodecContext *avctx,
> +                              void *data, int *got_frame,
> +                              AVPacket *avpkt)

vertical alignment

> +{
> +    AVFrame *frame = data;
> +
> +    int ret;
> +    GetByteContext gb;
> +
> +    unsigned int bytes_pp;
> +    unsigned int magic[4];
> +    unsigned int chunk_type;
> +    unsigned int data_len;
> +    PixHeader hdr;
> +
> +    bytestream2_init(&gb, avpkt->data, avpkt->size);
> +
> +    magic[0] = bytestream2_get_be32(&gb);
> +    magic[1] = bytestream2_get_be32(&gb);
> +    magic[2] = bytestream2_get_be32(&gb);
> +    magic[3] = bytestream2_get_be32(&gb);
> +
> +    if (magic[0] != 0x12 ||
> +        magic[1] != 0x08 ||
> +        magic[2] != 0x02 ||
> +        magic[3] != 0x02) {
> +        av_log(avctx, AV_LOG_ERROR, "Not a BRender PIX file\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    chunk_type = bytestream2_get_be32(&gb);
> +    if (chunk_type != 0x03 && chunk_type != 0x3d) {
> +        av_log(avctx, AV_LOG_ERROR, "Invalid chunk type %d\n", chunk_type);
> +        return AVERROR_INVALIDDATA;
> +    }

How about giving those magic numbers for chunk_type some useful names?

> +
> +    ret = pix_decode_header(&hdr, &gb);
> +    if (ret < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "Invalid header length\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +    switch (hdr.format) {
> +    case 3:
> +        avctx->pix_fmt = AV_PIX_FMT_PAL8;
> +        bytes_pp = 1;
> +        break;
> +    case 4:
> +        avctx->pix_fmt = AV_PIX_FMT_RGB555BE;
> +        bytes_pp = 2;
> +        break;
> +    case 5:
> +        avctx->pix_fmt = AV_PIX_FMT_RGB565BE;
> +        bytes_pp = 2;
> +        break;
> +    case 6:
> +        avctx->pix_fmt = AV_PIX_FMT_RGB24;
> +        bytes_pp = 3;
> +        break;
> +    case 18:
> +        avctx->pix_fmt = AV_PIX_FMT_Y400A;
> +        bytes_pp = 2;
> +        break;
> +    default:
> +        avpriv_request_sample(avctx, "Format %d", hdr.format);
> +        return AVERROR_PATCHWELCOME;
> +    }
> +
> +    if ((ret = ff_set_dimensions(avctx, hdr.width, hdr.height)) < 0)
> +        return ret;
> +
> +    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
> +        return ret;
> +
> +    chunk_type = bytestream2_get_be32(&gb);
> +
> +    if (avctx->pix_fmt == AV_PIX_FMT_PAL8 &&
> +        (chunk_type == 0x03 || chunk_type == 0x3d)) {
> +        PixHeader palhdr;
> +        uint32_t *pal_out = (uint32_t *)frame->data[1];
> +        int i;
> +
> +        ret = pix_decode_header(&palhdr, &gb);
> +        if (ret < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "Invalid palette header length\n");
> +            return AVERROR_INVALIDDATA;
> +        }
> +        if (palhdr.format != 7) {
> +            av_log(avctx, AV_LOG_ERROR, "Palette is not in 0RGB format\n");
> +            return AVERROR_INVALIDDATA;
> +        }
> +
> +        chunk_type = bytestream2_get_be32(&gb);
> +        data_len = bytestream2_get_be32(&gb);
> +        bytestream2_skip(&gb, 8);
> +        if (chunk_type != 0x21 || data_len != 1032 ||
> +            bytestream2_get_bytes_left(&gb) < 1032) {
> +            av_log(avctx, AV_LOG_ERROR, "Invalid palette data\n");
> +            return AVERROR_INVALIDDATA;
> +        }

This is confusing. Does data_len include the skipped 8 bytes before the
palette? or the 8 bytes after the palette? You seem to be checking 2
different things.

> +        // convert 0RGB to machine endian format (ARGB32)
> +        for (i = 0; i < 256; ++i) {
> +            bytestream2_skipu(&gb, 1);
> +            *pal_out++ = (0xFFU << 24) | bytestream2_get_be24u(&gb);
> +        }

You don't need the skip. Just do:
*pal_out++ = (0xFFU << 24) | bytestream2_get_be32u(&gb);

> +        bytestream2_skip(&gb, 8);
> +
> +        frame->palette_has_changed = 1;
> +
> +        chunk_type = bytestream2_get_be32(&gb);
> +    } else if (avctx->pix_fmt == AV_PIX_FMT_PAL8) {
> +        uint32_t *pal_out = (uint32_t *)frame->data[1];
> +        int i;
> +
> +        for (i = 0; i < 256; i++)
> +            *pal_out++ = (0xFFU << 24) | (i * 0x010101);
> +        frame->palette_has_changed = 1;

Um, this looks like grayscale to me. Not PAL8.

> +    }
> +
> +    data_len = bytestream2_get_be32(&gb);
> +    bytestream2_skip(&gb, 8);
> +
> +    // read the image data to the buffer
> +    unsigned int bytes_per_scanline = bytes_pp * hdr.width;
> +    unsigned int bytes_left = bytestream2_get_bytes_left(&gb);
> +
> +    if (chunk_type != 0x21 || data_len != bytes_left ||
> +        bytes_left / bytes_per_scanline < hdr.height) {
> +        av_log(avctx, AV_LOG_ERROR, "Invalid image data\n");
> +        return AVERROR_INVALIDDATA;
> +    }

Seems a little strict. How about data_len <= bytes_left?

> +
> +    av_image_copy_plane(frame->data[0], frame->linesize[0],
> +                        avpkt->data + bytestream2_tell(&gb),
> +                        bytes_per_scanline,
> +                        bytes_per_scanline, hdr.height);
> +    *got_frame = 1;

frame->pict_type = AV_PICTURE_TYPE_I;
frame->key_frame = 1;

> +
> +    return avpkt->size;
> +}
> +
> +AVCodec ff_pix_decoder = {
> +    .name         = "pix",
> +    .long_name    = NULL_IF_CONFIG_SMALL("BRender PIX image"),
> +    .type         = AVMEDIA_TYPE_VIDEO,
> +    .id           = AV_CODEC_ID_PIX,
> +    .decode       = pix_decode_frame,
> +    .capabilities = CODEC_CAP_DR1,
> +};
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 5ab49d5..d2f80ad 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,8 +29,8 @@
>  #include "libavutil/version.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR 55
> -#define LIBAVCODEC_VERSION_MINOR 34
> -#define LIBAVCODEC_VERSION_MICRO  1
> +#define LIBAVCODEC_VERSION_MINOR 35
> +#define LIBAVCODEC_VERSION_MICRO  0
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> diff --git a/libavformat/img2.c b/libavformat/img2.c
> index ed59281..68eef8c 100644
> --- a/libavformat/img2.c
> +++ b/libavformat/img2.c
> @@ -55,6 +55,7 @@ static const IdStrMap img_tags[] = {
>      { AV_CODEC_ID_TIFF,       "tif"      },
>      { AV_CODEC_ID_SGI,        "sgi"      },
>      { AV_CODEC_ID_PTX,        "ptx"      },
> +    { AV_CODEC_ID_PIX,        "pix"      },
>      { AV_CODEC_ID_PCX,        "pcx"      },
>      { AV_CODEC_ID_SUNRAST,    "sun"      },
>      { AV_CODEC_ID_SUNRAST,    "ras"      },

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

Reply via email to