[FFmpeg-devel] [PATCH v3 1/2] libavcodec/pgxdec: Add PGX decoder

Michael Niedermayer michael at niedermayer.cc
Sun Jun 28 23:44:02 EEST 2020


On Sun, Jun 28, 2020 at 08:13:28PM +0530, gautamramk at gmail.com wrote:
> From: Gautam Ramakrishnan <gautamramk at gmail.com>
> 
> This patch adds a pgx decoder.
> ---
>  Changelog               |   1 +
>  doc/general.texi        |   2 +
>  libavcodec/Makefile     |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/codec_desc.c |   7 ++
>  libavcodec/codec_id.h   |   1 +
>  libavcodec/pgxdec.c     | 205 ++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h    |   2 +-
>  8 files changed, 219 insertions(+), 1 deletion(-)
>  create mode 100644 libavcodec/pgxdec.c
> 
> diff --git a/Changelog b/Changelog
> index a60e7d2eb8..1bb9931c0d 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -4,6 +4,7 @@ releases are sorted from youngest to oldest.
>  version <next>:
>  - AudioToolbox output device
>  - MacCaption demuxer
> +- PGX decoder
>  
>  
>  version 4.3:
> diff --git a/doc/general.texi b/doc/general.texi
> index ea34b963b5..53d556c56c 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -751,6 +751,8 @@ following image formats are supported:
>      @tab Portable GrayMap image
>  @item PGMYUV       @tab X @tab X
>      @tab PGM with U and V components in YUV 4:2:0
> + at item PGX          @tab   @tab X
> +    @tab PGX file decoder
>  @item PIC          @tab @tab X
>      @tab Pictor/PC Paint
>  @item PNG          @tab X @tab X
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 5a6ea59715..12aa43fe51 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -536,6 +536,7 @@ OBJS-$(CONFIG_PGM_ENCODER)             += pnmenc.o
>  OBJS-$(CONFIG_PGMYUV_DECODER)          += pnmdec.o pnm.o
>  OBJS-$(CONFIG_PGMYUV_ENCODER)          += pnmenc.o
>  OBJS-$(CONFIG_PGSSUB_DECODER)          += pgssubdec.o
> +OBJS-$(CONFIG_PGX_DECODER)             += pgxdec.o
>  OBJS-$(CONFIG_PICTOR_DECODER)          += pictordec.o cga_data.o
>  OBJS-$(CONFIG_PIXLET_DECODER)          += pixlet.o
>  OBJS-$(CONFIG_PJS_DECODER)             += textdec.o ass.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index fa0c08d42e..a5048290f7 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -238,6 +238,7 @@ extern AVCodec ff_pgm_encoder;
>  extern AVCodec ff_pgm_decoder;
>  extern AVCodec ff_pgmyuv_encoder;
>  extern AVCodec ff_pgmyuv_decoder;
> +extern AVCodec ff_pgx_decoder;
>  extern AVCodec ff_pictor_decoder;
>  extern AVCodec ff_pixlet_decoder;
>  extern AVCodec ff_png_encoder;
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 9f8847544f..67e0a3055c 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -1405,6 +1405,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .long_name = NULL_IF_CONFIG_SMALL("AVS2-P2/IEEE1857.4"),
>          .props     = AV_CODEC_PROP_LOSSY,
>      },
> +    {
> +        .id        = AV_CODEC_ID_PGX,
> +        .type      = AVMEDIA_TYPE_VIDEO,
> +        .name      = "pgx",
> +        .long_name = NULL_IF_CONFIG_SMALL("PGX (JPEG2000 Test Format)"),
> +        .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> +    },
>      {
>          .id        = AV_CODEC_ID_Y41P,
>          .type      = AVMEDIA_TYPE_VIDEO,
> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
> index d885962c9c..896ecb0ce0 100644
> --- a/libavcodec/codec_id.h
> +++ b/libavcodec/codec_id.h
> @@ -241,6 +241,7 @@ enum AVCodecID {
>      AV_CODEC_ID_SCREENPRESSO,
>      AV_CODEC_ID_RSCC,
>      AV_CODEC_ID_AVS2,
> +    AV_CODEC_ID_PGX,
>  
>      AV_CODEC_ID_Y41P = 0x8000,
>      AV_CODEC_ID_AVRP,
> diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c
> new file mode 100644
> index 0000000000..688846f797
> --- /dev/null
> +++ b/libavcodec/pgxdec.c
> @@ -0,0 +1,205 @@
> +/*
> + * PGX image format
> + * Copyright (c) 2020 Gautam Ramakrishnan
> + *
> + * 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 "internal.h"
> +#include "bytestream.h"
> +#include "libavutil/imgutils.h"
> +
> +typedef struct PGXContext {
> +    GetByteContext  g;
> +} PGXContext;
> +
> +static int pgx_get_number(AVCodecContext *avctx, PGXContext * const s) {
> +    int ret = -1;
> +    char digit;
> +    int numdigits = 0;
> +
> +    while (1) {
> +        if (numdigits > 10) {
> +            return -1;
> +        }

If iam not mistaken this will still overflow the 32bit int 

signed integer overflow is undefined behavior so this must be fixed
using 64bit in the intermediary and then checking for INT_MAX is
an easy way to fix it but there are many other equivalent ones


> +        if (!bytestream2_get_bytes_left(&s->g))
> +            return -1;
> +        digit = bytestream2_get_byte(&s->g);
> +        if (digit == ' ' || digit == 0xA || digit == 0xD)
> +            break;
> +        else if (digit < '0' || digit > '9')
> +            return -1;
> +
> +        if (ret < 0)
> +            ret = 0;
> +        ret = 10 * ret + (digit - '0');
> +        numdigits++;
> +    }
> +
> +    if (ret >= 0)
> +        return ret;
> +    return -1;
> +}
> +
> +static int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * s,
> +                                int *depth, int *width, int *height,
> +                                int *sign)
> +{
> +    int byte;
> +
> +    if (bytestream2_get_bytes_left(&s->g) < 6) {
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    bytestream2_skip(&s->g, 6);
> +
> +    // Is the component signed?
> +    byte = bytestream2_peek_byte(&s->g);
> +    if (byte == '+') {
> +        *sign = 0;
> +        bytestream2_skip(&s->g, 1);
> +    } else if (byte == '-') {
> +        *sign = 1;
> +        bytestream2_skip(&s->g, 1);
> +    } else if (byte == 0)
> +        goto error;
> +
> +    byte = bytestream2_peek_byte(&s->g);
> +    if (byte == ' ')
> +        bytestream2_skip(&s->g, 1);
> +    else if (byte == 0)
> +        goto error;
> +
> +    *depth = pgx_get_number(avctx, s);
> +    *width = pgx_get_number(avctx, s);
> +    *height = pgx_get_number(avctx, s);
> +    if (*depth <= 0 || *width <= 0 || *height <= 0)
> +        goto error;
> +
> +    if (bytestream2_peek_byte(&s->g) == 0xA)
> +        bytestream2_skip(&s->g, 1);
> +    return 0;
> +
> +error:
> +    av_log(avctx, AV_LOG_ERROR, "Error in decoding header.\n");
> +    return AVERROR_INVALIDDATA;
> +}
> +
> +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s,
> +                                int width, int height, int sign, int depth)
> +{
> +    int i, j;
> +    for (i = 0; i < height; i++) {
> +        uint8_t *line = frame->data[0] + i * frame->linesize[0];
> +        for (j = 0; j < width; j++) {
> +            int val;
> +            if (bytestream2_get_bytes_left(&s->g) < 1)
> +                return AVERROR_INVALIDDATA;
> +            if (sign) {
> +                val = bytestream2_get_byte(&s->g) + (1 << (depth - 1));
> +                val <<= (8 - depth);
> +                *(line + j) = val;
> +            } else {
> +                val = bytestream2_get_byteu(&s->g);
> +                val <<= (8 - depth);
> +                *(line + j) = val;
> +            }
> +        }
> +        line += frame->linesize[0];
> +    }
> +    return 0;
> +}
> +
> +static inline int write_frame_16(AVPacket *avpkt, AVFrame *frame, PGXContext * const s,
> +                                 int width, int height, int sign, int depth)
> +{
> +    int i, j;
> +    for (i = 0; i < height; i++) {
> +        uint16_t *line = (uint16_t*)frame->data[0] + i*frame->linesize[0]/2;
> +        for (j = 0; j < width; j++) {
> +            int val;
> +            if (bytestream2_get_bytes_left(&s->g) < 2)
> +                return AVERROR_INVALIDDATA;
> +            if (sign) {
> +                val = (int16_t)bytestream2_get_be16(&s->g) + (1 << (depth - 1));
> +                val <<= (16 - depth);
> +                *(line + j) = val;
> +            } else {
> +                val = bytestream2_get_be16u(&s->g);
> +                val <<= (16 - depth);
> +                *(line + j) = val;
> +            }
> +        }
> +        line += frame->linesize[0]/2;
> +    }
> +    return 0;
> +}
> +
> +static int pgx_decode_frame(AVCodecContext *avctx, void *data,
> +                            int *got_frame, AVPacket *avpkt)
> +{
> +    PGXContext *s = avctx->priv_data;
> +    AVFrame *p    = data;
> +    int ret;
> +    int bpp;
> +    int width, height, depth;
> +    int sign = 0;
> +    bytestream2_init(&s->g, avpkt->data, avpkt->size);
> +
> +    if ((ret = ff_pgx_decode_header(avctx, s, &depth, &width, &height, &sign)) < 0)
> +        return ret;

> +    if (av_image_check_size(width, height, 0, avctx)) {

this could forward the returned error code


> +        av_log(avctx, AV_LOG_ERROR, "Image too large.\n");
> +        return AVERROR_INVALIDDATA;
> +    }

> +    if (ret = ff_set_dimensions(avctx, width, height) < 0)

this will misbehave without some more ()

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200628/603513c7/attachment.sig>


More information about the ffmpeg-devel mailing list