[FFmpeg-devel] [PATCH] Add XPM decoder
Nicolas George
george at nsup.org
Sun Mar 12 21:04:31 EET 2017
Le duodi 22 ventôse, an CCXXV, Paras Chadha a écrit :
> Xpm decoder was added
> Also added xpm_pipe demuxer with its probe function
>
> Signed-off-by: Paras Chadha <paraschadha18 at gmail.com>
> ---
> Changelog | 1 +
> doc/general.texi | 2 +
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 1 +
> libavcodec/avcodec.h | 1 +
> libavcodec/codec_desc.c | 7 +
> libavcodec/version.h | 4 +-
> libavcodec/xpmdec.c | 426 +++++++++++++++++++++++++++++++++++++++++++++++
> libavformat/Makefile | 1 +
> libavformat/allformats.c | 1 +
> libavformat/img2.c | 1 +
> libavformat/img2dec.c | 10 ++
> 12 files changed, 454 insertions(+), 2 deletions(-)
> create mode 100644 libavcodec/xpmdec.c
>
> diff --git a/Changelog b/Changelog
> index 13628ca..716b6ff 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -26,6 +26,7 @@ version <next>:
> - native Opus encoder
> - ScreenPressor decoder
> - incomplete ClearVideo decoder
> +- XPM decoder
>
> version 3.2:
> - libopenmpt demuxer
> diff --git a/doc/general.texi b/doc/general.texi
> index 30450c0..83f54b3 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -607,6 +607,8 @@ following image formats are supported:
> @tab WebP image format, encoding supported through external library libwebp
> @item XBM @tab X @tab X
> @tab X BitMap image format
> + at item XPM @tab X @tab X
> + @tab X PixMap image format
> @item XFace @tab X @tab X
> @tab X-Face image format
> @item XWD @tab X @tab X
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 65ccbad..b8d7a00 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -650,6 +650,7 @@ OBJS-$(CONFIG_XFACE_ENCODER) += xfaceenc.o xface.o
> OBJS-$(CONFIG_XL_DECODER) += xl.o
> OBJS-$(CONFIG_XMA1_DECODER) += wmaprodec.o wma.o wma_common.o
> OBJS-$(CONFIG_XMA2_DECODER) += wmaprodec.o wma.o wma_common.o
> +OBJS-$(CONFIG_XPM_DECODER) += xpmdec.o
> OBJS-$(CONFIG_XSUB_DECODER) += xsubdec.o
> OBJS-$(CONFIG_XSUB_ENCODER) += xsubenc.o
> OBJS-$(CONFIG_XWD_DECODER) += xwddec.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 074efd4..b7d03ad 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -378,6 +378,7 @@ static void register_all(void)
> REGISTER_ENCDEC (XBM, xbm);
> REGISTER_ENCDEC (XFACE, xface);
> REGISTER_DECODER(XL, xl);
> + REGISTER_DECODER(XPM, xpm);
> REGISTER_ENCDEC (XWD, xwd);
> REGISTER_ENCDEC (Y41P, y41p);
> REGISTER_DECODER(YLC, ylc);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 30ac236..e32f579 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -439,6 +439,7 @@ enum AVCodecID {
> AV_CODEC_ID_FMVC,
> AV_CODEC_ID_SCPR,
> AV_CODEC_ID_CLEARVIDEO,
> + AV_CODEC_ID_XPM,
>
> /* 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 06bcfc3..88cfddb 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -1591,6 +1591,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
> .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> },
> {
> + .id = AV_CODEC_ID_XPM,
> + .type = AVMEDIA_TYPE_VIDEO,
> + .name = "xpm",
> + .long_name = NULL_IF_CONFIG_SMALL("XPM (X PixMap) image"),
> + .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
> + },
> + {
> .id = AV_CODEC_ID_XWD,
> .type = AVMEDIA_TYPE_VIDEO,
> .name = "xwd",
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index b00e011..3ed5a71 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,8 +28,8 @@
> #include "libavutil/version.h"
>
> #define LIBAVCODEC_VERSION_MAJOR 57
> -#define LIBAVCODEC_VERSION_MINOR 82
> -#define LIBAVCODEC_VERSION_MICRO 102
> +#define LIBAVCODEC_VERSION_MINOR 83
> +#define LIBAVCODEC_VERSION_MICRO 100
>
> #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> LIBAVCODEC_VERSION_MINOR, \
> diff --git a/libavcodec/xpmdec.c b/libavcodec/xpmdec.c
> new file mode 100644
> index 0000000..7a1f4e1
> --- /dev/null
> +++ b/libavcodec/xpmdec.c
> @@ -0,0 +1,426 @@
> +/*
> + * XPM image format
> + *
> + * Copyright (c) 2012 Paul B Mahol
> + * Copyright (c) 2017 Paras Chadha
> + *
> + * 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 "libavutil/parseutils.h"
> +#include "libavutil/avstring.h"
> +#include "avcodec.h"
> +#include "internal.h"
> +
> +typedef struct XPMContext {
> + uint32_t *pixels;
> + int pixels_size;
The spacing is strange.
> +} XPMDecContext;
> +
> +typedef struct ColorEntry {
> + const char *name; ///< a string representing the name of the color
> + uint32_t rgb_color; ///< RGB values for the color
> +} ColorEntry;
> +
> +static int color_table_compare(const void *lhs, const void *rhs)
> +{
> + return av_strcasecmp(lhs, ((const ColorEntry *)rhs)->name);
> +}
> +
> +static const ColorEntry color_table[] = {
<snip>
The code duplication with parseutils is unacceptable, and I find the
reasons given by Paul weak. In particular, I do not see any conflict
with the database on my X.org version.
> +};
> +
> +static int convert(uint8_t x)
convert is not a very good name.
> +{
> + if (x >= 'a') {
> + x -= 87;
> + } else if (x >= 'A') {
> + x -= 55;
> + } else {
Avoid magic numbers in the code; x - 87 = x - 'a' + 10,
x - 55 = x - 'A' + 10, and "& ~32" can avoid making two cases anyway.
> + x -= '0';
> + }
> + return x;
> +}
> +
> +/*
> +** functions same as strcspn but ignores characters in reject if they are inside a C style comment...
> +** @param string, reject - same as that of strcspn
> +** @return length till any character in reject does not occur in string
> +*/
This is not a valid Doxygen comment.
> +static size_t mod_strcspn(const char *string, const char *reject)
> +{
> + int i, j;
> +
> + for (i = 0; string && string[i]; i++) {
The first clause of the condition is silly.
> + if (string[i] == '/' && string[i+1] == '*') {
> + i += 2;
> + while ( string && string[i] && (string[i] != '*' || string[i+1] != '/') )
Nit: no spaces within parentheses. And ditto for the first clause.
> + i++;
> + i++;
If the loop exits due to the "string[i]" part, this leaves I beyond the
end of the string, causing an illegal access on the next rounds.
> + } else if (string[i] == '/' && string[i+1] == '/') {
> + i += 2;
> + while ( string && string[i] && string[i] != '\n' )
Ditto for the first clause.
> + i++;
> + } else {
> + for (j = 0; reject && reject[j]; j++) {
> + if (string[i] == reject[j])
> + break;
Use strchr().
> + }
> + if (reject && reject[j])
> + break;
> + }
> + }
> + return i;
> +}
> +
> +static uint32_t hexstring_to_rgba(const char *p, int len)
This is a misnomer.
> +{
> + uint32_t ret = 0xFF000000;
> + const ColorEntry *entry;
> + char color_name[100];
> +
> + if (*p == '#') {
> + p++;
> + len--;
> + if (len == 3) {
> + ret |= (convert(p[2]) << 4) |
> + (convert(p[1]) << 12) |
> + (convert(p[0]) << 20);
> + } else if (len == 4) {
> + ret = (convert(p[3]) << 4) |
> + (convert(p[2]) << 12) |
> + (convert(p[1]) << 20) |
> + (convert(p[0]) << 28);
> + } else if (len == 6) {
> + ret |= convert(p[5]) |
> + (convert(p[4]) << 4) |
> + (convert(p[3]) << 8) |
> + (convert(p[2]) << 12) |
> + (convert(p[1]) << 16) |
> + (convert(p[0]) << 20);
> + } else if (len == 8) {
> + ret = convert(p[7]) |
> + (convert(p[6]) << 4) |
> + (convert(p[5]) << 8) |
> + (convert(p[4]) << 12) |
> + (convert(p[3]) << 16) |
> + (convert(p[2]) << 20) |
> + (convert(p[1]) << 24) |
> + (convert(p[0]) << 28);
> + }
> + } else {
> + strncpy(color_name, p, len);
> + color_name[len] = '\0';
This is completely wrong.
> +
> + entry = bsearch(color_name,
> + color_table,
> + (sizeof(color_table)/sizeof(color_table[0])),
> + sizeof(ColorEntry),
> + color_table_compare);
> +
> + if (!entry)
> + return ret;
> +
> + ret = entry->rgb_color;
> + }
> + return ret;
Is it specified somewhere that unknown colors should yield black?
Otherwise, an error code should be returned.
Also, you forgot to parse colors in standard X11 scheme
"rgb:RRRR/GGGG/BBBB".
> +}
> +
> +static int ascii2index(const uint8_t *cpixel, int cpp)
> +{
> + const uint8_t *p = cpixel;
> + int n = 0, m = 1, i;
> +
> + for (i = 0; i < cpp; i++) {
> + if (*p < ' ' || *p > '~')
> + return AVERROR_INVALIDDATA;
> + n += (*p++ - ' ') * m;
> + m *= 95;
> + }
> + return n;
> +}
> +
> +static int xpm_decode_frame(AVCodecContext *avctx, void *data,
> + int *got_frame, AVPacket *avpkt)
> +{
> + XPMDecContext *x = avctx->priv_data;
> + AVFrame *p=data;
> + const uint8_t *end, *ptr = avpkt->data;
> + int ncolors, cpp, ret, i, j;
> + int64_t size;
> + uint32_t *dst;
> +
> + avctx->pix_fmt = AV_PIX_FMT_BGRA;
> +
> + end = avpkt->data + avpkt->size;
> + if (memcmp(ptr, "/* XPM */", 9)) {
> + av_log(avctx, AV_LOG_ERROR, "missing signature\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + ptr += mod_strcspn(ptr, "\"");
If I read this correctly, you are skipping random characters until a
quote is found. This is not how a robust parser should be written.
> + if (sscanf(ptr, "\"%u %u %u %u\",",
> + &avctx->width, &avctx->height, &ncolors, &cpp) != 4) {
This is not properly checking the final quote and comma.
> + av_log(avctx, AV_LOG_ERROR, "missing image parameters\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if ((ret = ff_set_dimensions(avctx, avctx->width, avctx->height)) < 0)
> + return ret;
> +
> + if ((ret = ff_get_buffer(avctx, p, 0)) < 0)
> + return ret;
> +
> + if (ncolors <= 0) {
> + av_log(avctx, AV_LOG_ERROR, "invalid number of colors: %d\n", ncolors);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if (cpp <= 0) {
> + av_log(avctx, AV_LOG_ERROR, "invalid number of chars per pixel: %d\n", cpp);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + size = 1;
> + j = 1;
> + for (i = 0; i < cpp; i++) {
> + size += j*94;
> + j *= 95;
> + }
> + size *= 4;
This is a DoS waiting to happen.
> +
> + if (size < 0) {
This is deliberately invoking an undefined behaviour.
> + av_log(avctx, AV_LOG_ERROR, "unsupported number of chars per pixel: %d\n", cpp);
> + return AVERROR(ENOMEM);
> + }
> +
> + av_fast_padded_malloc(&x->pixels, &x->pixels_size, size);
> + if ( !x->pixels ) {
> + return AVERROR(ENOMEM);
> + }
> +
> + ptr += mod_strcspn(ptr, ",") + 1;
Same remark as above: skipping random contents. Same for other uses of
mod_strcspn().
> + for (i = 0; i < ncolors; i++) {
> + const uint8_t *index;
> + int len;
> +
> + ptr += mod_strcspn(ptr, "\"") + 1;
> + if (ptr + cpp > end)
> + return AVERROR_INVALIDDATA;
> + index = ptr;
index looks like a misnomer.
> + ptr += cpp;
> +
> + ptr = strstr(ptr, "c ");
> + if (ptr) {
> + ptr += 2;
> + } else {
> + return AVERROR_INVALIDDATA;
> + }
> +
> + len = strcspn(ptr, "\" ");
> +
> + if ((ret = ascii2index(index, cpp)) < 0)
> + return ret;
> +
> + x->pixels[ret] = hexstring_to_rgba(ptr, len);
> + ptr += mod_strcspn(ptr, ",") + 1;
> + }
> +
> + for (i = 0; i < avctx->height; i++) {
> + dst = (uint32_t *)(p->data[0] + i * p->linesize[0]);
> + ptr += mod_strcspn(ptr, "\"") + 1;
> +
> + for (j = 0; j < avctx->width; j++) {
> + if (ptr + cpp > end)
> + return AVERROR_INVALIDDATA;
> +
> + if ((ret = ascii2index(ptr, cpp)) < 0)
> + return ret;
> +
> + *dst++ = x->pixels[ret];
> + ptr += cpp;
> + }
> + ptr += mod_strcspn(ptr, ",") + 1;
This whole loop can go beyond the end of the input buffer very easily.
> + }
> +
> + p->key_frame = 1;
> + p->pict_type = AV_PICTURE_TYPE_I;
> +
> + *got_frame = 1;
> +
> + return avpkt->size;
> +}
> +
> +static av_cold int xpm_decode_close(AVCodecContext *avctx)
> +{
> + XPMDecContext *x = avctx->priv_data;
> + av_freep(&x->pixels);
> +
> + return 0;
> +}
> +
> +AVCodec ff_xpm_decoder = {
> + .name = "xpm",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = AV_CODEC_ID_XPM,
> + .priv_data_size = sizeof(XPMDecContext),
> + .close = xpm_decode_close,
> + .decode = xpm_decode_frame,
> + .capabilities = CODEC_CAP_DR1,
> + .long_name = NULL_IF_CONFIG_SMALL("XPM (X PixMap) image")
> +};
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index fc2d760..f56ef16 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -241,6 +241,7 @@ OBJS-$(CONFIG_IMAGE_SGI_PIPE_DEMUXER) += img2dec.o img2.o
> OBJS-$(CONFIG_IMAGE_SUNRAST_PIPE_DEMUXER) += img2dec.o img2.o
> OBJS-$(CONFIG_IMAGE_TIFF_PIPE_DEMUXER) += img2dec.o img2.o
> OBJS-$(CONFIG_IMAGE_WEBP_PIPE_DEMUXER) += img2dec.o img2.o
> +OBJS-$(CONFIG_IMAGE_XPM_PIPE_DEMUXER) += img2dec.o img2.o
> OBJS-$(CONFIG_INGENIENT_DEMUXER) += ingenientdec.o rawdec.o
> OBJS-$(CONFIG_IPMOVIE_DEMUXER) += ipmovie.o
> OBJS-$(CONFIG_IRCAM_DEMUXER) += ircamdec.o ircam.o pcm.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 132e58b..09e62c3 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -372,6 +372,7 @@ static void register_all(void)
> REGISTER_DEMUXER (IMAGE_SUNRAST_PIPE, image_sunrast_pipe);
> REGISTER_DEMUXER (IMAGE_TIFF_PIPE, image_tiff_pipe);
> REGISTER_DEMUXER (IMAGE_WEBP_PIPE, image_webp_pipe);
> + REGISTER_DEMUXER (IMAGE_XPM_PIPE, image_xpm_pipe);
>
> /* external libraries */
> REGISTER_MUXER (CHROMAPRINT, chromaprint);
> diff --git a/libavformat/img2.c b/libavformat/img2.c
> index f9f53ff..29df4f0 100644
> --- a/libavformat/img2.c
> +++ b/libavformat/img2.c
> @@ -75,6 +75,7 @@ const IdStrMap ff_img_tags[] = {
> { AV_CODEC_ID_V210X, "yuv10" },
> { AV_CODEC_ID_WEBP, "webp" },
> { AV_CODEC_ID_XBM, "xbm" },
> + { AV_CODEC_ID_XPM, "xpm" },
> { AV_CODEC_ID_XFACE, "xface" },
> { AV_CODEC_ID_XWD, "xwd" },
> { AV_CODEC_ID_NONE, NULL }
> diff --git a/libavformat/img2dec.c b/libavformat/img2dec.c
> index c3c2cf3..b454071 100644
> --- a/libavformat/img2dec.c
> +++ b/libavformat/img2dec.c
> @@ -943,6 +943,15 @@ static int pam_probe(AVProbeData *p)
> return pnm_magic_check(p, 7) ? pnm_probe(p) : 0;
> }
>
> +static int xpm_probe(AVProbeData *p)
> +{
> + const uint8_t *b = p->buf;
> +
> + if (AV_RB64(b) == 0x2f2a2058504d202a && *(b+8) == '/')
> + return AVPROBE_SCORE_MAX - 1;
> + return 0;
> +}
> +
> #define IMAGEAUTO_DEMUXER(imgname, codecid)\
> static const AVClass imgname ## _class = {\
> .class_name = AV_STRINGIFY(imgname) " demuxer",\
> @@ -983,3 +992,4 @@ IMAGEAUTO_DEMUXER(sgi, AV_CODEC_ID_SGI)
> IMAGEAUTO_DEMUXER(sunrast, AV_CODEC_ID_SUNRAST)
> IMAGEAUTO_DEMUXER(tiff, AV_CODEC_ID_TIFF)
> IMAGEAUTO_DEMUXER(webp, AV_CODEC_ID_WEBP)
> +IMAGEAUTO_DEMUXER(xpm, AV_CODEC_ID_XPM)
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170312/be937181/attachment.sig>
More information about the ffmpeg-devel
mailing list