| Message ID | 20200624182710.12549-1-gautamramk@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavcodec/pgx: Added pgx decoder | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make_warn | warning | New warnings during build |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Thanks for the patch. Here are a few preliminary remarks. gautamramk@gmail.com (12020-06-24): > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > This patch support to read and decode > pgx files. > --- > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/codec_id.h | 1 + > libavcodec/pgx.h | 38 +++++++++ > libavcodec/pgxdec.c | 176 +++++++++++++++++++++++++++++++++++++++ > libavformat/allformats.c | 1 + > libavformat/img2dec.c | 11 +++ > 7 files changed, 229 insertions(+) > create mode 100644 libavcodec/pgx.h > create mode 100644 libavcodec/pgxdec.c Documentation? > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index 5a6ea59715..0198c244e0 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -533,6 +533,7 @@ OBJS-$(CONFIG_PCX_ENCODER) += pcxenc.o > OBJS-$(CONFIG_PFM_DECODER) += pnmdec.o pnm.o > OBJS-$(CONFIG_PGM_DECODER) += pnmdec.o pnm.o > OBJS-$(CONFIG_PGM_ENCODER) += pnmenc.o > +OBJS-$(CONFIG_PGX_DECODER) += pgxdec.o > OBJS-$(CONFIG_PGMYUV_DECODER) += pnmdec.o pnm.o > OBJS-$(CONFIG_PGMYUV_ENCODER) += pnmenc.o > OBJS-$(CONFIG_PGSSUB_DECODER) += pgssubdec.o > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > index fa0c08d42e..b0217e6d6a 100644 > --- a/libavcodec/allcodecs.c > +++ b/libavcodec/allcodecs.c > @@ -236,6 +236,7 @@ extern AVCodec ff_pcx_decoder; > extern AVCodec ff_pfm_decoder; > extern AVCodec ff_pgm_encoder; > extern AVCodec ff_pgm_decoder; > +extern AVCodec ff_pgx_decoder; > extern AVCodec ff_pgmyuv_encoder; > extern AVCodec ff_pgmyuv_decoder; > extern AVCodec ff_pictor_decoder; > diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h > index d885962c9c..027ef21c62 100644 > --- a/libavcodec/codec_id.h > +++ b/libavcodec/codec_id.h > @@ -111,6 +111,7 @@ enum AVCodecID { > AV_CODEC_ID_PPM, > AV_CODEC_ID_PBM, > AV_CODEC_ID_PGM, > + AV_CODEC_ID_PGX, > AV_CODEC_ID_PGMYUV, > AV_CODEC_ID_PAM, > AV_CODEC_ID_FFVHUFF, > diff --git a/libavcodec/pgx.h b/libavcodec/pgx.h > new file mode 100644 > index 0000000000..bbe93fafe7 > --- /dev/null > +++ b/libavcodec/pgx.h > @@ -0,0 +1,38 @@ > +/* > + * 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 > + */ > + > +#ifndef AVCODEC_PGX_H > +#define AVCODEC_PGX_H > + > +#include "avcodec.h" > +#include "bytestream.h" > + > +typedef struct PGXContext { > + GetByteContext g; > + int depth; > + int sgnd; Is the micro-economy of letters rly ncssry? > + int width; > + int height; > +} PGXContext; > + > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * const s); This is not compatible with the function definition, how did you not get a warning from the compiler? Nit: no space after pointer mark. Will this be used elsewhere? Probably not. Then put it directly in the source file. > + > +#endif /* AVCODEC_PNM_H */ > \ No newline at end of file You need to fix the config of your text editor. > diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c > new file mode 100644 > index 0000000000..233bf34717 > --- /dev/null > +++ b/libavcodec/pgxdec.c > @@ -0,0 +1,176 @@ > +/* > + * 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 "pgx.h" > + > +static int pgx_get_number(AVCodecContext *avctx, PGXContext * const s) { > + char number[10]; > + int i; > + int ret; > + number[9] = '\0'; > + for (i = 0; i < 9; i++) { > + if (!bytestream2_get_bytes_left(&s->g)) > + return AVERROR_INVALIDDATA; > + number[i] = (char)bytestream2_get_byteu(&s->g); The cast is useless and harmful. Same in other places. > + if (number[i] == ' ' || number[i] == 0xA || number[i] == 0xD) { > + number[i] = '\0'; > + break; > + } else if (number[i] < '0' || number[i] > '9') { > + return AVERROR_INVALIDDATA; > + } > + } > + if (i > 9) > + return AVERROR_INVALIDDATA; > + ret = (int)strtol(number, NULL, 10); Looks extra complicated just to use strtol(). Just convert from decimal to number in the loop. > + if (ret >= 0) > + return ret; > + return AVERROR_INVALIDDATA; > +} > + > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * s) > +{ > + const char *header_start = "PG ML "; > + int i; > + int byte; > + if (bytestream2_get_bytes_left(&s->g) < 13) { > + return AVERROR_INVALIDDATA; > + } > + > + for (i = 0; i < 6; i++) { > + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { > + return AVERROR_INVALIDDATA; > + } > + } > + byte = bytestream2_peek_byte(&s->g); > + if (byte == '+') { > + s->sgnd = 0; > + bytestream2_skip(&s->g, 1); > + } else if (byte == '-') { > + s->sgnd = 1; > + bytestream2_skip(&s->g, 1); > + } > + > + if (bytestream2_peek_byte(&s->g) == ' ') > + bytestream2_skip(&s->g, 1); > + s->depth = pgx_get_number(avctx, s); > + s->width = pgx_get_number(avctx, s); > + s->height = pgx_get_number(avctx, s); > + if (bytestream2_peek_byte(&s->g) == 0xA) > + bytestream2_skip(&s->g, 1); > + return 0; > +} > + > +static inline int write_frame_16(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > +{ > + int i, j; > + for (i = 0; i < s->height; i++) { > + uint16_t* line = (uint16_t*)frame->data[0] + i*frame->linesize[0]/2; Pointer mark belongs with the variable, not with the type. > + for (j = 0; j < s->width; j++) { > + int val; > + if (s->sgnd) { > + val = (int16_t)bytestream2_get_be16(&s->g) + (1 << (s->depth - 1)); > + val <<= (16 - s->depth); > + *(line + j) = val; > + } else { > + val = bytestream2_get_be16u(&s->g); > + val <<= (16 - s->depth); > + *(line + j) = val; > + } > + } > + line += frame->linesize[0]/2; > + } > + return 0; > +} > + > +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > +{ > + int i, j; > + for (i = 0; i < s->height; i++) { > + uint8_t* line = frame->data[0] + i*frame->linesize[0]; > + for (j = 0; j < s->width; j++) { > + int val; > + if (s->sgnd) { > + val = bytestream2_get_byte(&s->g) + (1 << (s->depth - 1)); > + val <<= (8 - s->depth); > + *(line + j) = val; > + } else { > + val = bytestream2_get_byteu(&s->g); > + val <<= (8 - s->depth); > + *(line + j) = val; > + } > + } > + line += frame->linesize[0]; > + } > + return 0; > +} > + > +static int pgx_decode_frame(AVCodecContext *avctx, void *data, > + int *got_frame, AVPacket *avpkt) > +{ > + PGXContext * s = avctx->priv_data; > + AVFrame * const p = data; > + int ret; > + int num_vals; > + int bpp; > + bytestream2_init(&s->g, avpkt->data, avpkt->size); > + if ((ret = ff_pgx_decode_header(avctx, s)) < 0) > + return ret; Since you decode the header each time (normal for an image format), most of the fields do not need to be in the persistent context. > + > + num_vals = s->width * s->height; > + > + if (ret = ff_set_dimensions(avctx, s->width, s->height) < 0) > + return ret; > + > + if (s->depth <= 8) { > + avctx->pix_fmt = AV_PIX_FMT_GRAY8; > + bpp = 8; > + } else if (s->depth <= 16) { > + avctx->pix_fmt = AV_PIX_FMT_GRAY16BE; > + bpp = 16; > + } else { > + av_log(avctx, AV_LOG_ERROR, "Maximum depth of 16 bits supported.\n"); > + return AVERROR_PATCHWELCOME; > + } > + > + if ((ret = ff_get_buffer(avctx, p, 0)) < 0) > + return ret; > + *got_frame = 1; > + p->pict_type = AV_PICTURE_TYPE_I; > + p->key_frame = 1; > + avctx->bits_per_raw_sample = s->depth; > + if (bpp == 8) > + write_frame_8(avpkt, p, s); > + else if (bpp == 16) > + write_frame_16(avpkt, p, s); > + return 0; > +} > + > +AVCodec ff_pgx_decoder = { > + .name = "pgx", > + .long_name = NULL_IF_CONFIG_SMALL("PGX image"), > + .type = AVMEDIA_TYPE_VIDEO, > + .id = AV_CODEC_ID_PGX, > + .priv_data_size = sizeof(PGXContext), > + .decode = pgx_decode_frame, > + .capabilities = AV_CODEC_CAP_DR1, > +}; > \ No newline at end of file > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > index 97fd06debb..f8527b1fd4 100644 > --- a/libavformat/allformats.c > +++ b/libavformat/allformats.c > @@ -488,6 +488,7 @@ extern AVInputFormat ff_image_pbm_pipe_demuxer; > extern AVInputFormat ff_image_pcx_pipe_demuxer; > extern AVInputFormat ff_image_pgmyuv_pipe_demuxer; > extern AVInputFormat ff_image_pgm_pipe_demuxer; > +extern AVInputFormat ff_image_pgx_pipe_demuxer; > extern AVInputFormat ff_image_pictor_pipe_demuxer; > extern AVInputFormat ff_image_png_pipe_demuxer; > extern AVInputFormat ff_image_ppm_pipe_demuxer; > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c > index ee7ceed08f..43270901ff 100644 > --- a/libavformat/img2dec.c > +++ b/libavformat/img2dec.c > @@ -1000,6 +1000,16 @@ static int pgmyuv_probe(const AVProbeData *p) // custom FFmpeg format recognized > return ret && av_match_ext(p->filename, "pgmyuv") ? ret : 0; > } > > +static int pgx_probe(const AVProbeData *p) > +{ > + const uint8_t *b = p->buf; > + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; > + ret = ret && av_match_ext(p->filename, "pgx"); > + if (ret) > + return AVPROBE_SCORE_EXTENSION + 1; > + return 0; > +} > + > static int ppm_probe(const AVProbeData *p) > { > return pnm_magic_check(p, 3) || pnm_magic_check(p, 6) ? pnm_probe(p) : 0; > @@ -1094,6 +1104,7 @@ IMAGEAUTO_DEMUXER(pbm, AV_CODEC_ID_PBM) > IMAGEAUTO_DEMUXER(pcx, AV_CODEC_ID_PCX) > IMAGEAUTO_DEMUXER(pgm, AV_CODEC_ID_PGM) > IMAGEAUTO_DEMUXER(pgmyuv, AV_CODEC_ID_PGMYUV) > +IMAGEAUTO_DEMUXER(pgx, AV_CODEC_ID_PGX) > IMAGEAUTO_DEMUXER(pictor, AV_CODEC_ID_PICTOR) > IMAGEAUTO_DEMUXER(png, AV_CODEC_ID_PNG) > IMAGEAUTO_DEMUXER(ppm, AV_CODEC_ID_PPM) Regards,
Am Mi., 24. Juni 2020 um 20:55 Uhr schrieb <gautamramk@gmail.com>: > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > This patch support to read and decode > pgx files. > --- > libavcodec/Makefile | 1 + > libavcodec/allcodecs.c | 1 + > libavcodec/codec_id.h | 1 + > libavcodec/pgx.h | 38 +++++++++ > libavcodec/pgxdec.c | 176 +++++++++++++++++++++++++++++++++++++++ > libavformat/allformats.c | 1 + > libavformat/img2dec.c | 11 +++ > 7 files changed, 229 insertions(+) > create mode 100644 libavcodec/pgx.h > create mode 100644 libavcodec/pgxdec.c > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > index 5a6ea59715..0198c244e0 100644 > --- a/libavcodec/Makefile > +++ b/libavcodec/Makefile > @@ -533,6 +533,7 @@ OBJS-$(CONFIG_PCX_ENCODER) += pcxenc.o > OBJS-$(CONFIG_PFM_DECODER) += pnmdec.o pnm.o > OBJS-$(CONFIG_PGM_DECODER) += pnmdec.o pnm.o > OBJS-$(CONFIG_PGM_ENCODER) += pnmenc.o > +OBJS-$(CONFIG_PGX_DECODER) += pgxdec.o > OBJS-$(CONFIG_PGMYUV_DECODER) += pnmdec.o pnm.o > OBJS-$(CONFIG_PGMYUV_ENCODER) += pnmenc.o > OBJS-$(CONFIG_PGSSUB_DECODER) += pgssubdec.o > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > index fa0c08d42e..b0217e6d6a 100644 > --- a/libavcodec/allcodecs.c > +++ b/libavcodec/allcodecs.c > @@ -236,6 +236,7 @@ extern AVCodec ff_pcx_decoder; > extern AVCodec ff_pfm_decoder; > extern AVCodec ff_pgm_encoder; > extern AVCodec ff_pgm_decoder; > +extern AVCodec ff_pgx_decoder; > extern AVCodec ff_pgmyuv_encoder; > extern AVCodec ff_pgmyuv_decoder; > extern AVCodec ff_pictor_decoder; > diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h > index d885962c9c..027ef21c62 100644 > --- a/libavcodec/codec_id.h > +++ b/libavcodec/codec_id.h > @@ -111,6 +111,7 @@ enum AVCodecID { > AV_CODEC_ID_PPM, > AV_CODEC_ID_PBM, > AV_CODEC_ID_PGM, > + AV_CODEC_ID_PGX, You cannot put this in the middle of an enum, it breaks abi. > AV_CODEC_ID_PGMYUV, > AV_CODEC_ID_PAM, > AV_CODEC_ID_FFVHUFF, > diff --git a/libavcodec/pgx.h b/libavcodec/pgx.h > new file mode 100644 > index 0000000000..bbe93fafe7 > --- /dev/null > +++ b/libavcodec/pgx.h > @@ -0,0 +1,38 @@ > +/* > + * 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 > + */ > + > +#ifndef AVCODEC_PGX_H > +#define AVCODEC_PGX_H > + > +#include "avcodec.h" > +#include "bytestream.h" > + > +typedef struct PGXContext { > + GetByteContext g; > + int depth; > + int sgnd; > + int width; > + int height; > +} PGXContext; > + > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * const s); > + > +#endif /* AVCODEC_PNM_H */ > \ No newline at end of file Apart from this: Why is the header file needed at all? > diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c > new file mode 100644 > index 0000000000..233bf34717 > --- /dev/null > +++ b/libavcodec/pgxdec.c > @@ -0,0 +1,176 @@ > +/* > + * 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 "pgx.h" > + > +static int pgx_get_number(AVCodecContext *avctx, PGXContext * const s) { > + char number[10]; > + int i; > + int ret; > + number[9] = '\0'; > + for (i = 0; i < 9; i++) { > + if (!bytestream2_get_bytes_left(&s->g)) > + return AVERROR_INVALIDDATA; > + number[i] = (char)bytestream2_get_byteu(&s->g); > + if (number[i] == ' ' || number[i] == 0xA || number[i] == 0xD) { > + number[i] = '\0'; > + break; > + } else if (number[i] < '0' || number[i] > '9') { > + return AVERROR_INVALIDDATA; > + } > + } > + if (i > 9) > + return AVERROR_INVALIDDATA; > + ret = (int)strtol(number, NULL, 10); > + if (ret >= 0) > + return ret; > + return AVERROR_INVALIDDATA; > +} > + > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * s) > +{ > + const char *header_start = "PG ML "; > + int i; > + int byte; > + if (bytestream2_get_bytes_left(&s->g) < 13) { > + return AVERROR_INVALIDDATA; > + } Is this needed, are other decoders testing this? > + > + for (i = 0; i < 6; i++) { > + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { > + return AVERROR_INVALIDDATA; > + } Use memcmp() or consider to drop this check: If a user forces this decoder, it should not be necessary to depend on this. > + } > + byte = bytestream2_peek_byte(&s->g); > + if (byte == '+') { > + s->sgnd = 0; > + bytestream2_skip(&s->g, 1); > + } else if (byte == '-') { > + s->sgnd = 1; > + bytestream2_skip(&s->g, 1); > + } > + > + if (bytestream2_peek_byte(&s->g) == ' ') > + bytestream2_skip(&s->g, 1); > + s->depth = pgx_get_number(avctx, s); > + s->width = pgx_get_number(avctx, s); > + s->height = pgx_get_number(avctx, s); > + if (bytestream2_peek_byte(&s->g) == 0xA) > + bytestream2_skip(&s->g, 1); > + return 0; > +} > + > +static inline int write_frame_16(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > +{ > + int i, j; > + for (i = 0; i < s->height; i++) { > + uint16_t* line = (uint16_t*)frame->data[0] + i*frame->linesize[0]/2; > + for (j = 0; j < s->width; j++) { > + int val; > + if (s->sgnd) { > + val = (int16_t)bytestream2_get_be16(&s->g) + (1 << (s->depth - 1)); > + val <<= (16 - s->depth); > + *(line + j) = val; > + } else { > + val = bytestream2_get_be16u(&s->g); > + val <<= (16 - s->depth); > + *(line + j) = val; > + } > + } > + line += frame->linesize[0]/2; > + } > + return 0; > +} > + > +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > +{ > + int i, j; > + for (i = 0; i < s->height; i++) { > + uint8_t* line = frame->data[0] + i*frame->linesize[0]; > + for (j = 0; j < s->width; j++) { > + int val; > + if (s->sgnd) { > + val = bytestream2_get_byte(&s->g) + (1 << (s->depth - 1)); > + val <<= (8 - s->depth); > + *(line + j) = val; > + } else { > + val = bytestream2_get_byteu(&s->g); > + val <<= (8 - s->depth); > + *(line + j) = val; > + } > + } > + line += frame->linesize[0]; > + } > + return 0; > +} > + > +static int pgx_decode_frame(AVCodecContext *avctx, void *data, > + int *got_frame, AVPacket *avpkt) > +{ > + PGXContext * s = avctx->priv_data; > + AVFrame * const p = data; PGXContext *s = ... > + int ret; > + int num_vals; > + int bpp; > + bytestream2_init(&s->g, avpkt->data, avpkt->size); > + if ((ret = ff_pgx_decode_header(avctx, s)) < 0) > + return ret; > + > + num_vals = s->width * s->height; > + > + if (ret = ff_set_dimensions(avctx, s->width, s->height) < 0) > + return ret; > + > + if (s->depth <= 8) { > + avctx->pix_fmt = AV_PIX_FMT_GRAY8; > + bpp = 8; > + } else if (s->depth <= 16) { > + avctx->pix_fmt = AV_PIX_FMT_GRAY16BE; > + bpp = 16; > + } else { > + av_log(avctx, AV_LOG_ERROR, "Maximum depth of 16 bits supported.\n"); > + return AVERROR_PATCHWELCOME; > + } > + > + if ((ret = ff_get_buffer(avctx, p, 0)) < 0) > + return ret; > + *got_frame = 1; > + p->pict_type = AV_PICTURE_TYPE_I; > + p->key_frame = 1; > + avctx->bits_per_raw_sample = s->depth; > + if (bpp == 8) > + write_frame_8(avpkt, p, s); > + else if (bpp == 16) > + write_frame_16(avpkt, p, s); > + return 0; > +} > + > +AVCodec ff_pgx_decoder = { > + .name = "pgx", > + .long_name = NULL_IF_CONFIG_SMALL("PGX image"), > + .type = AVMEDIA_TYPE_VIDEO, > + .id = AV_CODEC_ID_PGX, > + .priv_data_size = sizeof(PGXContext), > + .decode = pgx_decode_frame, > + .capabilities = AV_CODEC_CAP_DR1, > +}; > \ No newline at end of file Please fix this. > diff --git a/libavformat/allformats.c b/libavformat/allformats.c I believe other developers will request that you split the demuxer and the decoder patch. > index 97fd06debb..f8527b1fd4 100644 > --- a/libavformat/allformats.c > +++ b/libavformat/allformats.c > @@ -488,6 +488,7 @@ extern AVInputFormat ff_image_pbm_pipe_demuxer; > extern AVInputFormat ff_image_pcx_pipe_demuxer; > extern AVInputFormat ff_image_pgmyuv_pipe_demuxer; > extern AVInputFormat ff_image_pgm_pipe_demuxer; > +extern AVInputFormat ff_image_pgx_pipe_demuxer; > extern AVInputFormat ff_image_pictor_pipe_demuxer; > extern AVInputFormat ff_image_png_pipe_demuxer; > extern AVInputFormat ff_image_ppm_pipe_demuxer; > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c > index ee7ceed08f..43270901ff 100644 > --- a/libavformat/img2dec.c > +++ b/libavformat/img2dec.c > @@ -1000,6 +1000,16 @@ static int pgmyuv_probe(const AVProbeData *p) // custom FFmpeg format recognized > return ret && av_match_ext(p->filename, "pgmyuv") ? ret : 0; > } > > +static int pgx_probe(const AVProbeData *p) > +{ > + const uint8_t *b = p->buf; > + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; if (AV_RB24(b) != ...) return 0; > + ret = ret && av_match_ext(p->filename, "pgx"); > + if (ret) > + return AVPROBE_SCORE_EXTENSION + 1; You should instead check if the file internally looks like pgx, checking the extension is a final possibility for things that are impossible to detect (which I think is not the case here). Carl Eugen
On Wed, Jun 24, 2020 at 23:57:10 +0530, gautamramk@gmail.com wrote: > + const char *header_start = "PG ML "; What about LM? Or do little-endian samples not exist? > + if (bytestream2_peek_byte(&s->g) == ' ') > + bytestream2_skip(&s->g, 1); I understand the separators can also be tabs, but that might also not be realistic. (I'm looking at the reference parser.) Same in pgx_get_number(). > +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) This is a bit similar to write_frame_16(). If a write_frame_16le() also needs to be added, the three could perhaps be formed with macros. Moritz
Am Do., 25. Juni 2020 um 00:12 Uhr schrieb Moritz Barsnick <barsnick@gmx.net>: > > On Wed, Jun 24, 2020 at 23:57:10 +0530, gautamramk@gmail.com wrote: > > + const char *header_start = "PG ML "; > > What about LM? Or do little-endian samples not exist? There are only 78 known samples, all from the jpeg2000 test suite. Carl Eugen
On Thu, Jun 25, 2020 at 00:52:57 +0200, Carl Eugen Hoyos wrote: > Am Do., 25. Juni 2020 um 00:12 Uhr schrieb Moritz Barsnick <barsnick@gmx.net>: > > What about LM? Or do little-endian samples not exist? > There are only 78 known samples, all from the jpeg2000 test suite. Okay, that makes this a fringe feature. I found 138 samples, 115 of them unique, and none of them uses little endianness, so case closed. Funnily, the reference decoder supports LE. For the record, all samples have either 4, 8 or 12 bit depth. Moritz
On Thu, Jun 25, 2020 at 12:43 AM Nicolas George <george@nsup.org> wrote: > > Thanks for the patch. Here are a few preliminary remarks. > > gautamramk@gmail.com (12020-06-24): > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > This patch support to read and decode > > pgx files. > > --- > > libavcodec/Makefile | 1 + > > libavcodec/allcodecs.c | 1 + > > libavcodec/codec_id.h | 1 + > > libavcodec/pgx.h | 38 +++++++++ > > libavcodec/pgxdec.c | 176 +++++++++++++++++++++++++++++++++++++++ > > libavformat/allformats.c | 1 + > > libavformat/img2dec.c | 11 +++ > > 7 files changed, 229 insertions(+) > > create mode 100644 libavcodec/pgx.h > > create mode 100644 libavcodec/pgxdec.c > > Documentation? > > > > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > > index 5a6ea59715..0198c244e0 100644 > > --- a/libavcodec/Makefile > > +++ b/libavcodec/Makefile > > @@ -533,6 +533,7 @@ OBJS-$(CONFIG_PCX_ENCODER) += pcxenc.o > > OBJS-$(CONFIG_PFM_DECODER) += pnmdec.o pnm.o > > OBJS-$(CONFIG_PGM_DECODER) += pnmdec.o pnm.o > > OBJS-$(CONFIG_PGM_ENCODER) += pnmenc.o > > +OBJS-$(CONFIG_PGX_DECODER) += pgxdec.o > > OBJS-$(CONFIG_PGMYUV_DECODER) += pnmdec.o pnm.o > > OBJS-$(CONFIG_PGMYUV_ENCODER) += pnmenc.o > > OBJS-$(CONFIG_PGSSUB_DECODER) += pgssubdec.o > > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > > index fa0c08d42e..b0217e6d6a 100644 > > --- a/libavcodec/allcodecs.c > > +++ b/libavcodec/allcodecs.c > > @@ -236,6 +236,7 @@ extern AVCodec ff_pcx_decoder; > > extern AVCodec ff_pfm_decoder; > > extern AVCodec ff_pgm_encoder; > > extern AVCodec ff_pgm_decoder; > > +extern AVCodec ff_pgx_decoder; > > extern AVCodec ff_pgmyuv_encoder; > > extern AVCodec ff_pgmyuv_decoder; > > extern AVCodec ff_pictor_decoder; > > diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h > > index d885962c9c..027ef21c62 100644 > > --- a/libavcodec/codec_id.h > > +++ b/libavcodec/codec_id.h > > @@ -111,6 +111,7 @@ enum AVCodecID { > > AV_CODEC_ID_PPM, > > AV_CODEC_ID_PBM, > > AV_CODEC_ID_PGM, > > + AV_CODEC_ID_PGX, > > AV_CODEC_ID_PGMYUV, > > AV_CODEC_ID_PAM, > > AV_CODEC_ID_FFVHUFF, > > diff --git a/libavcodec/pgx.h b/libavcodec/pgx.h > > new file mode 100644 > > index 0000000000..bbe93fafe7 > > --- /dev/null > > +++ b/libavcodec/pgx.h > > @@ -0,0 +1,38 @@ > > +/* > > + * 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 > > + */ > > + > > +#ifndef AVCODEC_PGX_H > > +#define AVCODEC_PGX_H > > + > > +#include "avcodec.h" > > +#include "bytestream.h" > > + > > > +typedef struct PGXContext { > > + GetByteContext g; > > + int depth; > > > + int sgnd; > > Is the micro-economy of letters rly ncssry? > > > + int width; > > + int height; > > +} PGXContext; > > + > > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * const s); > > This is not compatible with the function definition, how did you not get > a warning from the compiler? > > Nit: no space after pointer mark. > > Will this be used elsewhere? Probably not. Then put it directly in the > source file. > > > + > > +#endif /* AVCODEC_PNM_H */ > > > \ No newline at end of file > > You need to fix the config of your text editor. > > > diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c > > new file mode 100644 > > index 0000000000..233bf34717 > > --- /dev/null > > +++ b/libavcodec/pgxdec.c > > @@ -0,0 +1,176 @@ > > +/* > > + * 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 "pgx.h" > > + > > +static int pgx_get_number(AVCodecContext *avctx, PGXContext * const s) { > > + char number[10]; > > + int i; > > + int ret; > > + number[9] = '\0'; > > + for (i = 0; i < 9; i++) { > > + if (!bytestream2_get_bytes_left(&s->g)) > > + return AVERROR_INVALIDDATA; > > > + number[i] = (char)bytestream2_get_byteu(&s->g); > > The cast is useless and harmful. Same in other places. The type conversions confused me a bit. I will remove them. I guess changing from bytestream2_get_byteu to bytestrea2_get_byte should do the trick here? > > > + if (number[i] == ' ' || number[i] == 0xA || number[i] == 0xD) { > > + number[i] = '\0'; > > + break; > > + } else if (number[i] < '0' || number[i] > '9') { > > + return AVERROR_INVALIDDATA; > > + } > > + } > > + if (i > 9) > > + return AVERROR_INVALIDDATA; > > > + ret = (int)strtol(number, NULL, 10); > > Looks extra complicated just to use strtol(). Just convert from decimal > to number in the loop. > Yep, I shall change this. Skipped my mind while writing code. > > + if (ret >= 0) > > + return ret; > > + return AVERROR_INVALIDDATA; > > +} > > + > > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * s) > > +{ > > + const char *header_start = "PG ML "; > > + int i; > > + int byte; > > + if (bytestream2_get_bytes_left(&s->g) < 13) { > > + return AVERROR_INVALIDDATA; > > + } > > + > > + for (i = 0; i < 6; i++) { > > + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { > > + return AVERROR_INVALIDDATA; > > + } > > + } > > + byte = bytestream2_peek_byte(&s->g); > > + if (byte == '+') { > > + s->sgnd = 0; > > + bytestream2_skip(&s->g, 1); > > + } else if (byte == '-') { > > + s->sgnd = 1; > > + bytestream2_skip(&s->g, 1); > > + } > > + > > + if (bytestream2_peek_byte(&s->g) == ' ') > > + bytestream2_skip(&s->g, 1); > > + s->depth = pgx_get_number(avctx, s); > > + s->width = pgx_get_number(avctx, s); > > + s->height = pgx_get_number(avctx, s); > > + if (bytestream2_peek_byte(&s->g) == 0xA) > > + bytestream2_skip(&s->g, 1); > > + return 0; > > +} > > + > > +static inline int write_frame_16(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > > +{ > > + int i, j; > > + for (i = 0; i < s->height; i++) { > > > + uint16_t* line = (uint16_t*)frame->data[0] + i*frame->linesize[0]/2; > > Pointer mark belongs with the variable, not with the type. > > > + for (j = 0; j < s->width; j++) { > > + int val; > > + if (s->sgnd) { > > + val = (int16_t)bytestream2_get_be16(&s->g) + (1 << (s->depth - 1)); > > + val <<= (16 - s->depth); > > + *(line + j) = val; > > + } else { > > + val = bytestream2_get_be16u(&s->g); > > + val <<= (16 - s->depth); > > + *(line + j) = val; > > + } > > + } > > + line += frame->linesize[0]/2; > > + } > > + return 0; > > +} > > + > > +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > > +{ > > + int i, j; > > + for (i = 0; i < s->height; i++) { > > + uint8_t* line = frame->data[0] + i*frame->linesize[0]; > > + for (j = 0; j < s->width; j++) { > > + int val; > > + if (s->sgnd) { > > + val = bytestream2_get_byte(&s->g) + (1 << (s->depth - 1)); > > + val <<= (8 - s->depth); > > + *(line + j) = val; > > + } else { > > + val = bytestream2_get_byteu(&s->g); > > + val <<= (8 - s->depth); > > + *(line + j) = val; > > + } > > + } > > + line += frame->linesize[0]; > > + } > > + return 0; > > +} > > + > > +static int pgx_decode_frame(AVCodecContext *avctx, void *data, > > + int *got_frame, AVPacket *avpkt) > > +{ > > + PGXContext * s = avctx->priv_data; > > + AVFrame * const p = data; > > + int ret; > > + int num_vals; > > + int bpp; > > + bytestream2_init(&s->g, avpkt->data, avpkt->size); > > > + if ((ret = ff_pgx_decode_header(avctx, s)) < 0) > > + return ret; > > Since you decode the header each time (normal for an image format), most > of the fields do not need to be in the persistent context. > So you mean to say no need to store height and width in the PGX context? > > + > > + num_vals = s->width * s->height; > > + > > + if (ret = ff_set_dimensions(avctx, s->width, s->height) < 0) > > + return ret; > > + > > + if (s->depth <= 8) { > > + avctx->pix_fmt = AV_PIX_FMT_GRAY8; > > + bpp = 8; > > + } else if (s->depth <= 16) { > > + avctx->pix_fmt = AV_PIX_FMT_GRAY16BE; > > + bpp = 16; > > + } else { > > + av_log(avctx, AV_LOG_ERROR, "Maximum depth of 16 bits supported.\n"); > > + return AVERROR_PATCHWELCOME; > > + } > > + > > + if ((ret = ff_get_buffer(avctx, p, 0)) < 0) > > + return ret; > > + *got_frame = 1; > > + p->pict_type = AV_PICTURE_TYPE_I; > > + p->key_frame = 1; > > + avctx->bits_per_raw_sample = s->depth; > > + if (bpp == 8) > > + write_frame_8(avpkt, p, s); > > + else if (bpp == 16) > > + write_frame_16(avpkt, p, s); > > + return 0; > > +} > > + > > +AVCodec ff_pgx_decoder = { > > + .name = "pgx", > > + .long_name = NULL_IF_CONFIG_SMALL("PGX image"), > > + .type = AVMEDIA_TYPE_VIDEO, > > + .id = AV_CODEC_ID_PGX, > > + .priv_data_size = sizeof(PGXContext), > > + .decode = pgx_decode_frame, > > + .capabilities = AV_CODEC_CAP_DR1, > > +}; > > \ No newline at end of file > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > > index 97fd06debb..f8527b1fd4 100644 > > --- a/libavformat/allformats.c > > +++ b/libavformat/allformats.c > > @@ -488,6 +488,7 @@ extern AVInputFormat ff_image_pbm_pipe_demuxer; > > extern AVInputFormat ff_image_pcx_pipe_demuxer; > > extern AVInputFormat ff_image_pgmyuv_pipe_demuxer; > > extern AVInputFormat ff_image_pgm_pipe_demuxer; > > +extern AVInputFormat ff_image_pgx_pipe_demuxer; > > extern AVInputFormat ff_image_pictor_pipe_demuxer; > > extern AVInputFormat ff_image_png_pipe_demuxer; > > extern AVInputFormat ff_image_ppm_pipe_demuxer; > > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c > > index ee7ceed08f..43270901ff 100644 > > --- a/libavformat/img2dec.c > > +++ b/libavformat/img2dec.c > > @@ -1000,6 +1000,16 @@ static int pgmyuv_probe(const AVProbeData *p) // custom FFmpeg format recognized > > return ret && av_match_ext(p->filename, "pgmyuv") ? ret : 0; > > } > > > > +static int pgx_probe(const AVProbeData *p) > > +{ > > + const uint8_t *b = p->buf; > > + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; > > + ret = ret && av_match_ext(p->filename, "pgx"); > > + if (ret) > > + return AVPROBE_SCORE_EXTENSION + 1; > > + return 0; > > +} > > + > > static int ppm_probe(const AVProbeData *p) > > { > > return pnm_magic_check(p, 3) || pnm_magic_check(p, 6) ? pnm_probe(p) : 0; > > @@ -1094,6 +1104,7 @@ IMAGEAUTO_DEMUXER(pbm, AV_CODEC_ID_PBM) > > IMAGEAUTO_DEMUXER(pcx, AV_CODEC_ID_PCX) > > IMAGEAUTO_DEMUXER(pgm, AV_CODEC_ID_PGM) > > IMAGEAUTO_DEMUXER(pgmyuv, AV_CODEC_ID_PGMYUV) > > +IMAGEAUTO_DEMUXER(pgx, AV_CODEC_ID_PGX) > > IMAGEAUTO_DEMUXER(pictor, AV_CODEC_ID_PICTOR) > > IMAGEAUTO_DEMUXER(png, AV_CODEC_ID_PNG) > > IMAGEAUTO_DEMUXER(ppm, AV_CODEC_ID_PPM) > > Regards, > > -- > Nicolas George Thanks for the review Nicolas, I shall make the changes
On Thu, Jun 25, 2020 at 12:50 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Mi., 24. Juni 2020 um 20:55 Uhr schrieb <gautamramk@gmail.com>: > > > > From: Gautam Ramakrishnan <gautamramk@gmail.com> > > > > This patch support to read and decode > > pgx files. > > --- > > libavcodec/Makefile | 1 + > > libavcodec/allcodecs.c | 1 + > > libavcodec/codec_id.h | 1 + > > libavcodec/pgx.h | 38 +++++++++ > > libavcodec/pgxdec.c | 176 +++++++++++++++++++++++++++++++++++++++ > > libavformat/allformats.c | 1 + > > libavformat/img2dec.c | 11 +++ > > 7 files changed, 229 insertions(+) > > create mode 100644 libavcodec/pgx.h > > create mode 100644 libavcodec/pgxdec.c > > > > diff --git a/libavcodec/Makefile b/libavcodec/Makefile > > index 5a6ea59715..0198c244e0 100644 > > --- a/libavcodec/Makefile > > +++ b/libavcodec/Makefile > > @@ -533,6 +533,7 @@ OBJS-$(CONFIG_PCX_ENCODER) += pcxenc.o > > OBJS-$(CONFIG_PFM_DECODER) += pnmdec.o pnm.o > > OBJS-$(CONFIG_PGM_DECODER) += pnmdec.o pnm.o > > OBJS-$(CONFIG_PGM_ENCODER) += pnmenc.o > > +OBJS-$(CONFIG_PGX_DECODER) += pgxdec.o > > OBJS-$(CONFIG_PGMYUV_DECODER) += pnmdec.o pnm.o > > OBJS-$(CONFIG_PGMYUV_ENCODER) += pnmenc.o > > OBJS-$(CONFIG_PGSSUB_DECODER) += pgssubdec.o > > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c > > index fa0c08d42e..b0217e6d6a 100644 > > --- a/libavcodec/allcodecs.c > > +++ b/libavcodec/allcodecs.c > > @@ -236,6 +236,7 @@ extern AVCodec ff_pcx_decoder; > > extern AVCodec ff_pfm_decoder; > > extern AVCodec ff_pgm_encoder; > > extern AVCodec ff_pgm_decoder; > > +extern AVCodec ff_pgx_decoder; > > extern AVCodec ff_pgmyuv_encoder; > > extern AVCodec ff_pgmyuv_decoder; > > extern AVCodec ff_pictor_decoder; > > diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h > > index d885962c9c..027ef21c62 100644 > > --- a/libavcodec/codec_id.h > > +++ b/libavcodec/codec_id.h > > @@ -111,6 +111,7 @@ enum AVCodecID { > > AV_CODEC_ID_PPM, > > AV_CODEC_ID_PBM, > > AV_CODEC_ID_PGM, > > > + AV_CODEC_ID_PGX, > > You cannot put this in the middle of an enum, it breaks abi. > Ok, I shall move it to the bottom > > AV_CODEC_ID_PGMYUV, > > AV_CODEC_ID_PAM, > > AV_CODEC_ID_FFVHUFF, > > diff --git a/libavcodec/pgx.h b/libavcodec/pgx.h > > new file mode 100644 > > index 0000000000..bbe93fafe7 > > --- /dev/null > > +++ b/libavcodec/pgx.h > > @@ -0,0 +1,38 @@ > > +/* > > + * 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 > > + */ > > + > > +#ifndef AVCODEC_PGX_H > > +#define AVCODEC_PGX_H > > + > > +#include "avcodec.h" > > +#include "bytestream.h" > > + > > +typedef struct PGXContext { > > + GetByteContext g; > > + int depth; > > + int sgnd; > > + int width; > > + int height; > > +} PGXContext; > > + > > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * const s); > > + > > +#endif /* AVCODEC_PNM_H */ > > > \ No newline at end of file > > Apart from this: > Why is the header file needed at all? > In case we add an encoder. We'll probably make the header file later then? > > diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c > > new file mode 100644 > > index 0000000000..233bf34717 > > --- /dev/null > > +++ b/libavcodec/pgxdec.c > > @@ -0,0 +1,176 @@ > > +/* > > + * 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 "pgx.h" > > + > > +static int pgx_get_number(AVCodecContext *avctx, PGXContext * const s) { > > + char number[10]; > > + int i; > > + int ret; > > + number[9] = '\0'; > > + for (i = 0; i < 9; i++) { > > + if (!bytestream2_get_bytes_left(&s->g)) > > + return AVERROR_INVALIDDATA; > > + number[i] = (char)bytestream2_get_byteu(&s->g); > > + if (number[i] == ' ' || number[i] == 0xA || number[i] == 0xD) { > > + number[i] = '\0'; > > + break; > > + } else if (number[i] < '0' || number[i] > '9') { > > + return AVERROR_INVALIDDATA; > > + } > > + } > > + if (i > 9) > > + return AVERROR_INVALIDDATA; > > + ret = (int)strtol(number, NULL, 10); > > + if (ret >= 0) > > + return ret; > > + return AVERROR_INVALIDDATA; > > +} > > + > > +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * s) > > +{ > > + const char *header_start = "PG ML "; > > + int i; > > + int byte; > > > + if (bytestream2_get_bytes_left(&s->g) < 13) { > > + return AVERROR_INVALIDDATA; > > + } > > Is this needed, are other decoders testing this? > I shall change it. > > + > > + for (i = 0; i < 6; i++) { > > + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { > > + return AVERROR_INVALIDDATA; > > + } > > Use memcmp() or consider to drop this check: > If a user forces this decoder, it should not be necessary to depend on this. > I did not understand this > > + } > > + byte = bytestream2_peek_byte(&s->g); > > + if (byte == '+') { > > + s->sgnd = 0; > > + bytestream2_skip(&s->g, 1); > > + } else if (byte == '-') { > > + s->sgnd = 1; > > + bytestream2_skip(&s->g, 1); > > + } > > > + > > + if (bytestream2_peek_byte(&s->g) == ' ') > > + bytestream2_skip(&s->g, 1); > > + s->depth = pgx_get_number(avctx, s); > > + s->width = pgx_get_number(avctx, s); > > + s->height = pgx_get_number(avctx, s); > > + if (bytestream2_peek_byte(&s->g) == 0xA) > > + bytestream2_skip(&s->g, 1); > > + return 0; > > +} > > + > > +static inline int write_frame_16(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > > +{ > > + int i, j; > > + for (i = 0; i < s->height; i++) { > > + uint16_t* line = (uint16_t*)frame->data[0] + i*frame->linesize[0]/2; > > + for (j = 0; j < s->width; j++) { > > + int val; > > + if (s->sgnd) { > > + val = (int16_t)bytestream2_get_be16(&s->g) + (1 << (s->depth - 1)); > > + val <<= (16 - s->depth); > > + *(line + j) = val; > > + } else { > > + val = bytestream2_get_be16u(&s->g); > > + val <<= (16 - s->depth); > > + *(line + j) = val; > > + } > > + } > > + line += frame->linesize[0]/2; > > + } > > + return 0; > > +} > > + > > +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) > > +{ > > + int i, j; > > + for (i = 0; i < s->height; i++) { > > + uint8_t* line = frame->data[0] + i*frame->linesize[0]; > > + for (j = 0; j < s->width; j++) { > > + int val; > > + if (s->sgnd) { > > + val = bytestream2_get_byte(&s->g) + (1 << (s->depth - 1)); > > + val <<= (8 - s->depth); > > + *(line + j) = val; > > + } else { > > + val = bytestream2_get_byteu(&s->g); > > + val <<= (8 - s->depth); > > + *(line + j) = val; > > + } > > + } > > + line += frame->linesize[0]; > > + } > > + return 0; > > +} > > + > > +static int pgx_decode_frame(AVCodecContext *avctx, void *data, > > + int *got_frame, AVPacket *avpkt) > > +{ > > + PGXContext * s = avctx->priv_data; > > + AVFrame * const p = data; > > PGXContext *s = ... > > > + int ret; > > + int num_vals; > > + int bpp; > > + bytestream2_init(&s->g, avpkt->data, avpkt->size); > > + if ((ret = ff_pgx_decode_header(avctx, s)) < 0) > > + return ret; > > + > > + num_vals = s->width * s->height; > > + > > + if (ret = ff_set_dimensions(avctx, s->width, s->height) < 0) > > + return ret; > > + > > + if (s->depth <= 8) { > > + avctx->pix_fmt = AV_PIX_FMT_GRAY8; > > + bpp = 8; > > + } else if (s->depth <= 16) { > > + avctx->pix_fmt = AV_PIX_FMT_GRAY16BE; > > + bpp = 16; > > + } else { > > + av_log(avctx, AV_LOG_ERROR, "Maximum depth of 16 bits supported.\n"); > > + return AVERROR_PATCHWELCOME; > > + } > > + > > + if ((ret = ff_get_buffer(avctx, p, 0)) < 0) > > + return ret; > > + *got_frame = 1; > > + p->pict_type = AV_PICTURE_TYPE_I; > > + p->key_frame = 1; > > + avctx->bits_per_raw_sample = s->depth; > > + if (bpp == 8) > > + write_frame_8(avpkt, p, s); > > + else if (bpp == 16) > > + write_frame_16(avpkt, p, s); > > + return 0; > > +} > > + > > +AVCodec ff_pgx_decoder = { > > + .name = "pgx", > > + .long_name = NULL_IF_CONFIG_SMALL("PGX image"), > > + .type = AVMEDIA_TYPE_VIDEO, > > + .id = AV_CODEC_ID_PGX, > > + .priv_data_size = sizeof(PGXContext), > > + .decode = pgx_decode_frame, > > + .capabilities = AV_CODEC_CAP_DR1, > > +}; > > > \ No newline at end of file > > Please fix this. > > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > > I believe other developers will request that you split the demuxer > and the decoder patch. > The only issue is the demuxer refers to the decoder. How do I do this? > > index 97fd06debb..f8527b1fd4 100644 > > --- a/libavformat/allformats.c > > +++ b/libavformat/allformats.c > > @@ -488,6 +488,7 @@ extern AVInputFormat ff_image_pbm_pipe_demuxer; > > extern AVInputFormat ff_image_pcx_pipe_demuxer; > > extern AVInputFormat ff_image_pgmyuv_pipe_demuxer; > > extern AVInputFormat ff_image_pgm_pipe_demuxer; > > +extern AVInputFormat ff_image_pgx_pipe_demuxer; > > extern AVInputFormat ff_image_pictor_pipe_demuxer; > > extern AVInputFormat ff_image_png_pipe_demuxer; > > extern AVInputFormat ff_image_ppm_pipe_demuxer; > > diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c > > index ee7ceed08f..43270901ff 100644 > > --- a/libavformat/img2dec.c > > +++ b/libavformat/img2dec.c > > @@ -1000,6 +1000,16 @@ static int pgmyuv_probe(const AVProbeData *p) // custom FFmpeg format recognized > > return ret && av_match_ext(p->filename, "pgmyuv") ? ret : 0; > > } > > > > +static int pgx_probe(const AVProbeData *p) > > +{ > > + const uint8_t *b = p->buf; > > + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; > > if (AV_RB24(b) != ...) > return 0; > > > + ret = ret && av_match_ext(p->filename, "pgx"); > > + if (ret) > > + return AVPROBE_SCORE_EXTENSION + 1; > > You should instead check if the file internally looks like pgx, > checking the extension is a final possibility for things that > are impossible to detect (which I think is not the case here). > The previous check does that. It checks if the first 3 bytes are PG and space. > Carl Eugen
Am Do., 25. Juni 2020 um 09:23 Uhr schrieb Gautam Ramakrishnan <gautamramk@gmail.com>: > > On Thu, Jun 25, 2020 at 12:50 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > + for (i = 0; i < 6; i++) { > > > + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { > > > + return AVERROR_INVALIDDATA; > > > + } > > > > Use memcmp() or consider to drop this check: > > If a user forces this decoder, it should not be necessary to depend on this. > > > I did not understand this My personal believe is - although some decoders are doing this - that this check is not useful. If you keep the check, it should be done with memcmp(). [...] > > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > > > > I believe other developers will request that you split the demuxer > > and the decoder patch. > > > The only issue is the demuxer refers to the decoder. How do I do this? If you decide to split the patch, the first patch adds the decoder, the second the demuxer. Please increase MINOR versions for both libraries in the appropriate patches. [...] > > > +static int pgx_probe(const AVProbeData *p) > > > +{ > > > + const uint8_t *b = p->buf; > > > + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; > > > > if (AV_RB24(b) != ...) > > return 0; > > > > > + ret = ret && av_match_ext(p->filename, "pgx"); > > > + if (ret) > > > + return AVPROBE_SCORE_EXTENSION + 1; > > > > You should instead check if the file internally looks like pgx, > > checking the extension is a final possibility for things that > > are impossible to detect (which I think is not the case here). > > > The previous check does that. It checks if the first 3 bytes are PG and space. Please check more than three bytes - use memcmp() - and please remove the usage of av_match_ext(), it should only be used in rare cases, this is not one of them. Carl Eugen
On Fri, Jun 26, 2020 at 3:50 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > Am Do., 25. Juni 2020 um 09:23 Uhr schrieb Gautam Ramakrishnan > <gautamramk@gmail.com>: > > > > On Thu, Jun 25, 2020 at 12:50 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > > + for (i = 0; i < 6; i++) { > > > > + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { > > > > + return AVERROR_INVALIDDATA; > > > > + } > > > > > > Use memcmp() or consider to drop this check: > > > If a user forces this decoder, it should not be necessary to depend on this. > > > > > I did not understand this > > My personal believe is - although some decoders are doing this - that > this check is not useful. > If you keep the check, it should be done with memcmp(). > > [...] > > > > > diff --git a/libavformat/allformats.c b/libavformat/allformats.c > > > > > > I believe other developers will request that you split the demuxer > > > and the decoder patch. > > > > > The only issue is the demuxer refers to the decoder. How do I do this? > > If you decide to split the patch, the first patch adds the decoder, > the second the demuxer. > Please increase MINOR versions for both libraries in the appropriate > patches. > > [...] > > > > > +static int pgx_probe(const AVProbeData *p) > > > > +{ > > > > + const uint8_t *b = p->buf; > > > > + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; > > > > > > if (AV_RB24(b) != ...) > > > return 0; > > > > > > > + ret = ret && av_match_ext(p->filename, "pgx"); > > > > + if (ret) > > > > + return AVPROBE_SCORE_EXTENSION + 1; > > > > > > You should instead check if the file internally looks like pgx, > > > checking the extension is a final possibility for things that > > > are impossible to detect (which I think is not the case here). > > > > > The previous check does that. It checks if the first 3 bytes are PG and space. > > Please check more than three bytes - use memcmp() - and please > remove the usage of av_match_ext(), it should only be used in rare > cases, this is not one of them. I found the instructions to add a new codec. It says add an entry to libavcodec/codec_desc.c. I understand this CoDec would have a lossless property. What other properties should this codec have?
Gautam Ramakrishnan (12020-06-25): > The type conversions confused me a bit. I will remove them. Please be sure you have the basics about C type promotions right. C will automatically convert types whenever necessary and notify when one such conversion may cause unexpected results. Adding a cast will force a conversion, possibly a wrong conversion followed by the right automatic conversion. It will also silence possible warnings. The rule of thumb is that a C program should contain as little casts as possible, usually to silence warnings in tricky pointer arithmetic where the developer has checked carefully. > I guess changing from bytestream2_get_byteu to bytestrea2_get_byte should > do the trick here? What? You use the u variant if you are sure there is enough data available, it has nothing to do with a cast. > So you mean to say no need to store height and width in the > PGX context? Exactly. Just define all the variables decoded from the header in decode_frame(). If there are too many, you can group them in another structure. Regards,
On Sat, Jun 27, 2020 at 5:45 PM Nicolas George <george@nsup.org> wrote: > > Gautam Ramakrishnan (12020-06-25): > > The type conversions confused me a bit. I will remove them. > > Please be sure you have the basics about C type promotions right. C will > automatically convert types whenever necessary and notify when one such > conversion may cause unexpected results. Adding a cast will force a > conversion, possibly a wrong conversion followed by the right automatic > conversion. It will also silence possible warnings. > > The rule of thumb is that a C program should contain as little casts as > possible, usually to silence warnings in tricky pointer arithmetic where > the developer has checked carefully. > > > I guess changing from bytestream2_get_byteu to bytestrea2_get_byte should > > do the trick here? > > What? You use the u variant if you are sure there is enough data > available, it has nothing to do with a cast. > > > So you mean to say no need to store height and width in the > > PGX context? > > Exactly. Just define all the variables decoded from the header in > decode_frame(). If there are too many, you can group them in another > structure. This will require me to remove the decode_header() function and add the logic directly into the decode_frame function. If that is fine, I shall remove PGX context structrure. > > Regards, > > -- > Nicolas George > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Gautam Ramakrishnan (12020-06-27): > This will require me to remove the decode_header() function and add the > logic directly into the decode_frame function. If that is fine, I shall remove > PGX context structrure. No, it requires nothing of the sort. You can pass pointers to the individual variables to decode_header() just as well as you could pass a pointer to the whole context. Or, as I already explained, you can group the variable in another structure if that is more convenient. Regards,
On Sat, Jun 27, 2020 at 6:06 PM Nicolas George <george@nsup.org> wrote: > > Gautam Ramakrishnan (12020-06-27): > > This will require me to remove the decode_header() function and add the > > logic directly into the decode_frame function. If that is fine, I shall remove > > PGX context structrure. > > No, it requires nothing of the sort. You can pass pointers to the > individual variables to decode_header() just as well as you could pass a > pointer to the whole context. Or, as I already explained, you can group > the variable in another structure if that is more convenient. > Oh yes, got it. I shall make these changes and resubmit.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 5a6ea59715..0198c244e0 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -533,6 +533,7 @@ OBJS-$(CONFIG_PCX_ENCODER) += pcxenc.o OBJS-$(CONFIG_PFM_DECODER) += pnmdec.o pnm.o OBJS-$(CONFIG_PGM_DECODER) += pnmdec.o pnm.o OBJS-$(CONFIG_PGM_ENCODER) += pnmenc.o +OBJS-$(CONFIG_PGX_DECODER) += pgxdec.o OBJS-$(CONFIG_PGMYUV_DECODER) += pnmdec.o pnm.o OBJS-$(CONFIG_PGMYUV_ENCODER) += pnmenc.o OBJS-$(CONFIG_PGSSUB_DECODER) += pgssubdec.o diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c index fa0c08d42e..b0217e6d6a 100644 --- a/libavcodec/allcodecs.c +++ b/libavcodec/allcodecs.c @@ -236,6 +236,7 @@ extern AVCodec ff_pcx_decoder; extern AVCodec ff_pfm_decoder; extern AVCodec ff_pgm_encoder; extern AVCodec ff_pgm_decoder; +extern AVCodec ff_pgx_decoder; extern AVCodec ff_pgmyuv_encoder; extern AVCodec ff_pgmyuv_decoder; extern AVCodec ff_pictor_decoder; diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h index d885962c9c..027ef21c62 100644 --- a/libavcodec/codec_id.h +++ b/libavcodec/codec_id.h @@ -111,6 +111,7 @@ enum AVCodecID { AV_CODEC_ID_PPM, AV_CODEC_ID_PBM, AV_CODEC_ID_PGM, + AV_CODEC_ID_PGX, AV_CODEC_ID_PGMYUV, AV_CODEC_ID_PAM, AV_CODEC_ID_FFVHUFF, diff --git a/libavcodec/pgx.h b/libavcodec/pgx.h new file mode 100644 index 0000000000..bbe93fafe7 --- /dev/null +++ b/libavcodec/pgx.h @@ -0,0 +1,38 @@ +/* + * 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 + */ + +#ifndef AVCODEC_PGX_H +#define AVCODEC_PGX_H + +#include "avcodec.h" +#include "bytestream.h" + +typedef struct PGXContext { + GetByteContext g; + int depth; + int sgnd; + int width; + int height; +} PGXContext; + +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * const s); + +#endif /* AVCODEC_PNM_H */ \ No newline at end of file diff --git a/libavcodec/pgxdec.c b/libavcodec/pgxdec.c new file mode 100644 index 0000000000..233bf34717 --- /dev/null +++ b/libavcodec/pgxdec.c @@ -0,0 +1,176 @@ +/* + * 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 "pgx.h" + +static int pgx_get_number(AVCodecContext *avctx, PGXContext * const s) { + char number[10]; + int i; + int ret; + number[9] = '\0'; + for (i = 0; i < 9; i++) { + if (!bytestream2_get_bytes_left(&s->g)) + return AVERROR_INVALIDDATA; + number[i] = (char)bytestream2_get_byteu(&s->g); + if (number[i] == ' ' || number[i] == 0xA || number[i] == 0xD) { + number[i] = '\0'; + break; + } else if (number[i] < '0' || number[i] > '9') { + return AVERROR_INVALIDDATA; + } + } + if (i > 9) + return AVERROR_INVALIDDATA; + ret = (int)strtol(number, NULL, 10); + if (ret >= 0) + return ret; + return AVERROR_INVALIDDATA; +} + +int ff_pgx_decode_header(AVCodecContext *avctx, PGXContext * s) +{ + const char *header_start = "PG ML "; + int i; + int byte; + if (bytestream2_get_bytes_left(&s->g) < 13) { + return AVERROR_INVALIDDATA; + } + + for (i = 0; i < 6; i++) { + if (header_start[i] != (char)bytestream2_get_byteu(&s->g)) { + return AVERROR_INVALIDDATA; + } + } + byte = bytestream2_peek_byte(&s->g); + if (byte == '+') { + s->sgnd = 0; + bytestream2_skip(&s->g, 1); + } else if (byte == '-') { + s->sgnd = 1; + bytestream2_skip(&s->g, 1); + } + + if (bytestream2_peek_byte(&s->g) == ' ') + bytestream2_skip(&s->g, 1); + s->depth = pgx_get_number(avctx, s); + s->width = pgx_get_number(avctx, s); + s->height = pgx_get_number(avctx, s); + if (bytestream2_peek_byte(&s->g) == 0xA) + bytestream2_skip(&s->g, 1); + return 0; +} + +static inline int write_frame_16(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) +{ + int i, j; + for (i = 0; i < s->height; i++) { + uint16_t* line = (uint16_t*)frame->data[0] + i*frame->linesize[0]/2; + for (j = 0; j < s->width; j++) { + int val; + if (s->sgnd) { + val = (int16_t)bytestream2_get_be16(&s->g) + (1 << (s->depth - 1)); + val <<= (16 - s->depth); + *(line + j) = val; + } else { + val = bytestream2_get_be16u(&s->g); + val <<= (16 - s->depth); + *(line + j) = val; + } + } + line += frame->linesize[0]/2; + } + return 0; +} + +static inline int write_frame_8(AVPacket *avpkt, AVFrame *frame, PGXContext * const s) +{ + int i, j; + for (i = 0; i < s->height; i++) { + uint8_t* line = frame->data[0] + i*frame->linesize[0]; + for (j = 0; j < s->width; j++) { + int val; + if (s->sgnd) { + val = bytestream2_get_byte(&s->g) + (1 << (s->depth - 1)); + val <<= (8 - s->depth); + *(line + j) = val; + } else { + val = bytestream2_get_byteu(&s->g); + val <<= (8 - s->depth); + *(line + j) = val; + } + } + line += frame->linesize[0]; + } + return 0; +} + +static int pgx_decode_frame(AVCodecContext *avctx, void *data, + int *got_frame, AVPacket *avpkt) +{ + PGXContext * s = avctx->priv_data; + AVFrame * const p = data; + int ret; + int num_vals; + int bpp; + bytestream2_init(&s->g, avpkt->data, avpkt->size); + if ((ret = ff_pgx_decode_header(avctx, s)) < 0) + return ret; + + num_vals = s->width * s->height; + + if (ret = ff_set_dimensions(avctx, s->width, s->height) < 0) + return ret; + + if (s->depth <= 8) { + avctx->pix_fmt = AV_PIX_FMT_GRAY8; + bpp = 8; + } else if (s->depth <= 16) { + avctx->pix_fmt = AV_PIX_FMT_GRAY16BE; + bpp = 16; + } else { + av_log(avctx, AV_LOG_ERROR, "Maximum depth of 16 bits supported.\n"); + return AVERROR_PATCHWELCOME; + } + + if ((ret = ff_get_buffer(avctx, p, 0)) < 0) + return ret; + *got_frame = 1; + p->pict_type = AV_PICTURE_TYPE_I; + p->key_frame = 1; + avctx->bits_per_raw_sample = s->depth; + if (bpp == 8) + write_frame_8(avpkt, p, s); + else if (bpp == 16) + write_frame_16(avpkt, p, s); + return 0; +} + +AVCodec ff_pgx_decoder = { + .name = "pgx", + .long_name = NULL_IF_CONFIG_SMALL("PGX image"), + .type = AVMEDIA_TYPE_VIDEO, + .id = AV_CODEC_ID_PGX, + .priv_data_size = sizeof(PGXContext), + .decode = pgx_decode_frame, + .capabilities = AV_CODEC_CAP_DR1, +}; \ No newline at end of file diff --git a/libavformat/allformats.c b/libavformat/allformats.c index 97fd06debb..f8527b1fd4 100644 --- a/libavformat/allformats.c +++ b/libavformat/allformats.c @@ -488,6 +488,7 @@ extern AVInputFormat ff_image_pbm_pipe_demuxer; extern AVInputFormat ff_image_pcx_pipe_demuxer; extern AVInputFormat ff_image_pgmyuv_pipe_demuxer; extern AVInputFormat ff_image_pgm_pipe_demuxer; +extern AVInputFormat ff_image_pgx_pipe_demuxer; extern AVInputFormat ff_image_pictor_pipe_demuxer; extern AVInputFormat ff_image_png_pipe_demuxer; extern AVInputFormat ff_image_ppm_pipe_demuxer; diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c index ee7ceed08f..43270901ff 100644 --- a/libavformat/img2dec.c +++ b/libavformat/img2dec.c @@ -1000,6 +1000,16 @@ static int pgmyuv_probe(const AVProbeData *p) // custom FFmpeg format recognized return ret && av_match_ext(p->filename, "pgmyuv") ? ret : 0; } +static int pgx_probe(const AVProbeData *p) +{ + const uint8_t *b = p->buf; + int ret = (AV_RB32(b) & 0xFFFFFF00) == 0x50472000; + ret = ret && av_match_ext(p->filename, "pgx"); + if (ret) + return AVPROBE_SCORE_EXTENSION + 1; + return 0; +} + static int ppm_probe(const AVProbeData *p) { return pnm_magic_check(p, 3) || pnm_magic_check(p, 6) ? pnm_probe(p) : 0; @@ -1094,6 +1104,7 @@ IMAGEAUTO_DEMUXER(pbm, AV_CODEC_ID_PBM) IMAGEAUTO_DEMUXER(pcx, AV_CODEC_ID_PCX) IMAGEAUTO_DEMUXER(pgm, AV_CODEC_ID_PGM) IMAGEAUTO_DEMUXER(pgmyuv, AV_CODEC_ID_PGMYUV) +IMAGEAUTO_DEMUXER(pgx, AV_CODEC_ID_PGX) IMAGEAUTO_DEMUXER(pictor, AV_CODEC_ID_PICTOR) IMAGEAUTO_DEMUXER(png, AV_CODEC_ID_PNG) IMAGEAUTO_DEMUXER(ppm, AV_CODEC_ID_PPM)
From: Gautam Ramakrishnan <gautamramk@gmail.com> This patch support to read and decode pgx files. --- libavcodec/Makefile | 1 + libavcodec/allcodecs.c | 1 + libavcodec/codec_id.h | 1 + libavcodec/pgx.h | 38 +++++++++ libavcodec/pgxdec.c | 176 +++++++++++++++++++++++++++++++++++++++ libavformat/allformats.c | 1 + libavformat/img2dec.c | 11 +++ 7 files changed, 229 insertions(+) create mode 100644 libavcodec/pgx.h create mode 100644 libavcodec/pgxdec.c