[FFmpeg-devel] [PATCH v3] [RFC] lavc, lavfmt: add FLIF decoding support

[Andreas Rheinhardt]

Anamitra Ghorui:
> This patch adds support for non animated, animated, non interlaced and
> interlaced FLIF images.
> Hopefully, all previously mentioned mistakes have been resolved.
> However, there are a few things I want to ask:
> 
> * The decoder can accept arbitary packet sizes. How do I make the
>   demuxer (specifically flif16_read_packet) produce a packet that is
>   reasonably sized? Using a static packet size (such as 4096) produces
>   errors
> * FLIF can encode ICCP color profiles as metadata. These however, as
>   per libavcodec/pngdec.c some processing is done per frame, along with
>   setting the dictionary entry. How should this be handled in the case 
>   of a non-still image situation?
>   
> Test files are available here: https://0x0.st/iYs_.zip
> 
> Thanks,
> Anamitra
> 
> Co-authored-by: Anamitra Ghorui <aghorui at teknik.io>
> Co-authored-by: Kartik K Khullar <kartikkhullar840 at gmail.com>
> 
> Signed-off-by: Anamitra Ghorui <aghorui at teknik.io>
> ---

[...]

> diff --git a/libavformat/flifdec.c b/libavformat/flifdec.c
> new file mode 100644
> index 0000000000..8fbedd6b58
> --- /dev/null
> +++ b/libavformat/flifdec.c
> @@ -0,0 +1,435 @@
> +/*
> + * FLIF16 demuxer
> + * Copyright (c) 2020 Anamitra Ghorui <aghorui at teknik.io>
> + *
> + * 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
> + */
> +
> +/**
> + * @file
> + * FLIF demuxer.
> + */
> +
> +#include "avformat.h"
> +#include "libavutil/common.h"
> +#include "libavutil/bprint.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/opt.h"
> +#include "internal.h"
> +#include "libavcodec/exif.h"
> +
> +#include "libavcodec/flif16.h"
> +#include "libavcodec/flif16_rangecoder.h"
> +
> +#include "config.h"
> +
> +#if CONFIG_ZLIB
> +#include <zlib.h>
> +#endif
> +
> +/*
> + * FLIF's reference encoder currently encodes metadata as a raw DEFLATE stream
> + * (RFC 1951). In order to decode a raw deflate stream using Zlib, inflateInit2
> + * must be used with windowBits being between -8 .. -15.
> + */
> +#define ZLIB_WINDOW_BITS -15
> +#define BUF_SIZE 4096
> +
> +typedef struct FLIFDemuxContext {
> +    const AVClass *class;
> +#if CONFIG_ZLIB
> +    z_stream stream;
> +    uint8_t active;
> +#endif
> +} FLIFDemuxContext;
> +
> +

[..]

> +
> +
> +static const AVOption options[] = {
> +    { NULL }
> +};

You don't have options, so you don't need options or an AVClass. The
const AVClass * in your context should then also be removed.

> +
> +static const AVClass demuxer_class = {
> +    .class_name = "FLIF demuxer",
> +    .item_name  = av_default_item_name,
> +    .option     = options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +    .category   = AV_CLASS_CATEGORY_DEMUXER,
> +};
> +
> +AVInputFormat ff_flif_demuxer = {
> +    .name           = "flif",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Free Lossless Image Format (FLIF)"),
> +    .priv_data_size = sizeof(FLIFDemuxContext),
> +    .extensions     = "flif",
> +    .read_probe     = flif16_probe,
> +    .read_header    = flif16_read_header,
> +    .read_packet    = flif16_read_packet,
> +    .priv_class     = &demuxer_class,
> +};

I haven't looked at it closely at all, yet we typically want decoders
and demuxers to be in separate patches.

- Andreas