[FFmpeg-devel] [PATCH] Add XPM decoder

[Paul B Mahol]

On 3/11/17, Paras Chadha <paraschadha18 at gmail.com> wrote:
> Here is the patch with all the changes mentioned above
>
> Signed-off-by: Paras <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     | 430
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/img2.c      |   1 +
>  9 files changed, 446 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/xpmdec.c
>

[...]

> +    { "NavajoWhite",          0xFFFFDEAD },
> +    { "Navy",                 0xFF000080 },
> +    { "None",                 0x00FFFFFF },

None is 0x00000000 AFAIK. Is it?

[...]

> +/*
> +**  functions same as strcspn but ignores characters in reject if they are
> inside a C style comment...
> +**  @param str, reject - same as that of strcspn
> +**  @return length till any character in reject does not occur in str
> +*/
> +static size_t mod_strcspn(const char *str, const char *rej)
> +{
> +    size_t v1, v2;
> +    const char * ptr = str, *temp;
> +
> +    v1 = strcspn(str, "/");
> +    v2 = strcspn(str, rej);
> +
> +    while (v1<v2) {
> +        ptr += v1+1;
> +        if (*(ptr)=='*') {
> +            if (temp = strstr(ptr, "*/")) {
> +                ptr = temp+2;
> +            } else {
> +                ptr++;
> +            }
> +        } else if (*(ptr)=='/') {
> +            if (temp = strstr(ptr, "\n")) {
> +                ptr = temp+1;
> +            } else {
> +                ptr++;
> +            }
> +        } else {
> +            ptr++;
> +        }
> +        v1 = strcspn(ptr, "/");
> +        v2 = strcspn(ptr, rej);
> +    }
> +
> +    return (size_t)(ptr-str)+v2;

This looks too complicated. Instead stole strcspn implementation and
do what it does
but also skip C comments.

[...]

 +
> +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 (sscanf(ptr, "\"%u %u %u %u\",",
> +               &avctx->width, &avctx->height, &ncolors, &cpp) != 4) {
> +        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++) {

style.

> +        size += j*94;
> +        j *= 95;
> +    }
> +    size *= 4;
> +
> +    if (size<0) {

Please follow style, spaces between '<' : if (size < 0) {...

> +        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;
> +    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;
> +        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;
> +            ret = ascii2index(ptr, cpp);

Check ret value, it can be negative.

> +            *dst++ = x->pixels[ret];
> +            ptr += cpp;
> +        }
> +        ptr += mod_strcspn(ptr, ",") + 1;
> +    }
> +
> +    p->key_frame = 1;
> +    p->pict_type = AV_PICTURE_TYPE_I;
> +
> +    *got_frame = 1;
> +    *(AVFrame *)data = *p;

This line is redundant.

> +
> +    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/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       }

Please also add xpm_pipe demuxer with its probe function.