My apologies, I should have removed other parts of the quoted code which I was not replying to. The reply is unnecesarily long now.
On Mon, 3 Aug, 2020, 12:39 am Kartik K. Khullar, <kartikkhullar...@gmail.com> wrote: > > > On Sun, Aug 2, 2020 at 5:57 PM Nicolas George <geo...@nsup.org> wrote: > >> Anamitra Ghorui (12020-07-30): >> > Visible errors have been fixed in libavformat/flifdec.c, however the >> > problem regarding metadata decoding still exists. The function does work >> > with dummy data from zlib's example program (see >> https://0x0.st/ix_E.zip for >> > an example with "1234" as the encoded sequence), so the problem may be >> > in providing the appropriate parameters. >> >> Thanks for the patch. See a few comments below. This is so long, I was >> not as careful at the end as in the beginning. >> >> > >> > Other test files: https://0x0.st/ixSs.7z >> > >> > Co-authored-by: Anamitra Ghorui <agho...@teknik.io> >> > Co-authored-by: Kartik K Khullar <kartikkhullar...@gmail.com> >> > >> > Signed-off-by: Anamitra Ghorui <agho...@teknik.io> >> > --- >> > Changelog | 3 +- >> > configure | 2 + >> > doc/general.texi | 2 + >> > libavcodec/Makefile | 2 + >> > libavcodec/allcodecs.c | 1 + >> > libavcodec/codec_desc.c | 7 + >> > libavcodec/codec_id.h | 1 + >> > libavcodec/flif16.c | 198 +++ >> > libavcodec/flif16.h | 278 +++ >> > libavcodec/flif16_parser.c | 189 ++ >> > libavcodec/flif16_rangecoder.c | 464 +++++ >> > libavcodec/flif16_rangecoder.h | 824 +++++++++ >> > libavcodec/flif16_transform.c | 2964 ++++++++++++++++++++++++++++++++ >> > libavcodec/flif16_transform.h | 123 ++ >> > libavcodec/flif16dec.c | 1146 ++++++++++++ >> > libavcodec/parsers.c | 1 + >> > libavformat/Makefile | 1 + >> > libavformat/allformats.c | 1 + >> > libavformat/flifdec.c | 377 ++++ >> > 19 files changed, 6583 insertions(+), 1 deletion(-) >> > create mode 100644 libavcodec/flif16.c >> > create mode 100644 libavcodec/flif16.h >> > create mode 100644 libavcodec/flif16_parser.c >> > create mode 100644 libavcodec/flif16_rangecoder.c >> > create mode 100644 libavcodec/flif16_rangecoder.h >> > create mode 100644 libavcodec/flif16_transform.c >> > create mode 100644 libavcodec/flif16_transform.h >> > create mode 100644 libavcodec/flif16dec.c >> > create mode 100644 libavformat/flifdec.c >> > >> > diff --git a/Changelog b/Changelog >> > index 6f648bff2b..ac5a21b1a9 100644 >> > --- a/Changelog >> > +++ b/Changelog >> > @@ -10,7 +10,8 @@ version <next>: >> > - ADPCM IMA Ubisoft APM encoder >> > - Rayman 2 APM muxer >> > - AV1 encoding support SVT-AV1 >> > - >> > +- FLIF16 decoder >> > +- FLIF16 demuxer >> > >> > version 4.3: >> > - v360 filter >> > diff --git a/configure b/configure >> > index 169f23e17f..50936fef4a 100755 >> > --- a/configure >> > +++ b/configure >> > @@ -2718,6 +2718,8 @@ ffvhuff_encoder_select="huffyuv_encoder" >> > fic_decoder_select="golomb" >> > flac_decoder_select="flacdsp" >> > flac_encoder_select="bswapdsp flacdsp lpc" >> > +flif16_decoder_select="flif16dec" >> > +flif16_encoder_select="flif16enc" >> > flashsv2_decoder_deps="zlib" >> > flashsv2_encoder_deps="zlib" >> > flashsv_decoder_deps="zlib" >> > diff --git a/doc/general.texi b/doc/general.texi >> > index dfcfd394e6..71b61100e3 100644 >> > --- a/doc/general.texi >> > +++ b/doc/general.texi >> > @@ -903,6 +903,8 @@ following image formats are supported: >> > @item Flash Screen Video v2 @tab X @tab X >> > @item Flash Video (FLV) @tab X @tab X >> > @tab Sorenson H.263 used in Flash >> > +@item FLIF (Free Lossless Image Format @tab @tab X >> > + @tab Precursor to JPEG XL and FUIF >> > @item FM Screen Capture Codec @tab @tab X >> > @item Forward Uncompressed @tab @tab X >> > @item Fraps @tab @tab X >> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile >> > index 9d4d52d048..96d06d2479 100644 >> > --- a/libavcodec/Makefile >> > +++ b/libavcodec/Makefile >> > @@ -328,6 +328,7 @@ OBJS-$(CONFIG_FLASHSV_ENCODER) += >> flashsvenc.o >> > OBJS-$(CONFIG_FLASHSV2_ENCODER) += flashsv2enc.o >> > OBJS-$(CONFIG_FLASHSV2_DECODER) += flashsv.o >> > OBJS-$(CONFIG_FLIC_DECODER) += flicvideo.o >> > +OBJS-$(CONFIG_FLIF16_DECODER) += flif16dec.o >> flif16_rangecoder.o flif16.o flif16_transform.o >> > OBJS-$(CONFIG_FMVC_DECODER) += fmvc.o >> > OBJS-$(CONFIG_FOURXM_DECODER) += 4xm.o >> > OBJS-$(CONFIG_FRAPS_DECODER) += fraps.o >> > @@ -1069,6 +1070,7 @@ OBJS-$(CONFIG_DVD_NAV_PARSER) += >> dvd_nav_parser.o >> > OBJS-$(CONFIG_DVDSUB_PARSER) += dvdsub_parser.o >> > OBJS-$(CONFIG_FLAC_PARSER) += flac_parser.o flacdata.o >> flac.o \ >> > vorbis_data.o >> > +OBJS-$(CONFIG_FLIF16_PARSER) += flif16_parser.o >> > OBJS-$(CONFIG_G723_1_PARSER) += g723_1_parser.o >> > OBJS-$(CONFIG_G729_PARSER) += g729_parser.o >> > OBJS-$(CONFIG_GIF_PARSER) += gif_parser.o >> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c >> > index 80142899fe..032ff422f8 100644 >> > --- a/libavcodec/allcodecs.c >> > +++ b/libavcodec/allcodecs.c >> > @@ -119,6 +119,7 @@ extern AVCodec ff_flashsv_decoder; >> > extern AVCodec ff_flashsv2_encoder; >> > extern AVCodec ff_flashsv2_decoder; >> > extern AVCodec ff_flic_decoder; >> > +extern AVCodec ff_flif16_decoder; >> > extern AVCodec ff_flv_encoder; >> > extern AVCodec ff_flv_decoder; >> > extern AVCodec ff_fmvc_decoder; >> > diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c >> > index ced00bd34c..4ca0d1f514 100644 >> > --- a/libavcodec/codec_desc.c >> > +++ b/libavcodec/codec_desc.c >> > @@ -1784,6 +1784,13 @@ static const AVCodecDescriptor >> codec_descriptors[] = { >> > .long_name = NULL_IF_CONFIG_SMALL("PFM (Portable FloatMap) >> image"), >> > .props = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS, >> > }, >> > + { >> > + .id = AV_CODEC_ID_FLIF16, >> > + .type = AVMEDIA_TYPE_VIDEO, >> > + .name = "flif16", >> > + .long_name = NULL_IF_CONFIG_SMALL("FLIF16 (Free Lossless Image >> Format)"), >> > + .props = AV_CODEC_PROP_LOSSLESS, >> > + }, >> > >> > /* various PCM "codecs" */ >> > { >> > diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h >> > index 896ecb0ce0..5c4f2dd7d0 100644 >> > --- a/libavcodec/codec_id.h >> > +++ b/libavcodec/codec_id.h >> > @@ -296,6 +296,7 @@ enum AVCodecID { >> > AV_CODEC_ID_MV30, >> > AV_CODEC_ID_NOTCHLC, >> > AV_CODEC_ID_PFM, >> > + AV_CODEC_ID_FLIF16, >> > >> > /* various PCM "codecs" */ >> > AV_CODEC_ID_FIRST_AUDIO = 0x10000, ///< A dummy id pointing at >> the start of audio codecs >> > diff --git a/libavcodec/flif16.c b/libavcodec/flif16.c >> > new file mode 100644 >> > index 0000000000..d8ffb31c34 >> > --- /dev/null >> > +++ b/libavcodec/flif16.c >> > @@ -0,0 +1,198 @@ >> > +/* >> > + * FLIF16 Image Format Definitions >> > + * Copyright (c) 2020 Anamitra Ghorui <agho...@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 >> > + * FLIF16 format definitions and functions. >> > + */ >> > + >> > +#include "flif16.h" >> > +#include "flif16_transform.h" >> > + >> > +/** >> > + * Initialise property ranges for non interlaced images. >> > + * @param[out] prop_ranges resultant ranges >> > + * @param[in] color ranges of each channel >> > + * @param[in] channels number of channels >> > + */ >> >> > +int32_t (*ff_flif16_maniac_ni_prop_ranges_init(unsigned int >> *prop_ranges_size, >> > + FLIF16RangesContext >> *ranges, >> > + uint8_t plane, >> > + uint8_t channels))[2] >> >> I would prefer avoiding pointers to arrays of arrays, they are tricky to >> use and the syntax is awful, as clearly visible here. >> >> I suggest to define "struct FLIFMinMax { int32_t min, max; }" and to use >> it instead. >> >> > +{ >> > + int min = ff_flif16_ranges_min(ranges, plane); >> > + int max = ff_flif16_ranges_max(ranges, plane); >> > + int mind = min - max, maxd = max - min; >> > + int32_t (*prop_ranges)[2]; >> > + unsigned int top = 0; >> >> > + unsigned int size = (((plane < 3) ? plane : 0) + 2 + 5) + ((plane >> < 3) && (ranges->num_planes > 3)); >> >> Am I wrong, or is it always at most 10 here and 12 in the other function? >> >> If so, then let us get rid of this dynamic allocation and just have: >> >> #define FLIF_MAX_RANGES 12 >> >> FLIFMinMax prop_ranges[FLIF_MAX_RANGES]; >> >> > + *prop_ranges_size = size; >> >> > + prop_ranges = av_mallocz(sizeof(*prop_ranges) * size); >> >> av_mallocz_array(), but moot if we avoid the dynamic alloc. >> >> > + if (!prop_ranges) >> > + return NULL; >> > + if (plane < 3) { >> > + for (int i = 0; i < plane; i++) { >> > + prop_ranges[top][0] = ff_flif16_ranges_min(ranges, i); >> > + prop_ranges[top++][1] = ff_flif16_ranges_max(ranges, i); >> // pixels on previous planes >> > + } >> > + if (ranges->num_planes > 3) { >> > + prop_ranges[top][0] = ff_flif16_ranges_min(ranges, 3); >> > + prop_ranges[top++][1] = ff_flif16_ranges_max(ranges, 3); >> // pixel on alpha plane >> > + } >> > + } >> > + prop_ranges[top][0] = min; >> > + prop_ranges[top++][1] = max; // guess (median of 3) >> > + prop_ranges[top][0] = 0; >> > + prop_ranges[top++][1] = 2; // which predictor was it >> >> > + for (int i = 0; i < 5; ++i) { >> >> Nit: We usually write i++. At least be consistent. >> >> > + prop_ranges[top][0] = mind; >> > + prop_ranges[top++][1] = maxd; >> > + } >> > + return prop_ranges; >> > +} >> > + >> > +int32_t (*ff_flif16_maniac_prop_ranges_init(unsigned int >> *prop_ranges_size, >> > + FLIF16RangesContext >> *ranges, >> > + uint8_t property, >> > + uint8_t channels))[2] >> > +{ >> > + int min = ff_flif16_ranges_min(ranges, property); >> > + int max = ff_flif16_ranges_max(ranges, property); >> > + unsigned int top = 0, pp; >> > + int mind = min - max, maxd = max - min; >> > + int32_t (*prop_ranges)[2]; >> >> > + unsigned int size = (((property < 3) ? ((ranges->num_planes > 3) >> ? property + 1 : property) : 0) \ >> > + + ((property == 1 || property == 2) ? 1 : 0) \ >> > + + ((property != 2) ? 2 : 0) + 1 + 5); >> > + prop_ranges = av_mallocz(sizeof(*prop_ranges) * size); >> > + if (!prop_ranges) >> > + return NULL; >> > + *prop_ranges_size = size; >> > + >> > + if (property < 3) { >> > + for (pp = 0; pp < property; pp++) { >> > + prop_ranges[top][0] = ff_flif16_ranges_min(ranges, pp); >> > + prop_ranges[top++][1] = ff_flif16_ranges_max(ranges, pp); >> > + } >> > + if (ranges->num_planes > 3) { >> > + prop_ranges[top][0] = ff_flif16_ranges_min(ranges, 3); >> > + prop_ranges[top++][1] = ff_flif16_ranges_max(ranges, 3);; >> > + } >> > + } >> > + >> > + prop_ranges[top][0] = 0; >> > + prop_ranges[top++][0] = 2; >> > + >> > + if (property == 1 || property == 2){ >> > + prop_ranges[top][0] = ff_flif16_ranges_min(ranges, 0) - >> ff_flif16_ranges_max(ranges, 0); >> > + prop_ranges[top++][1] = ff_flif16_ranges_max(ranges, 0) - >> ff_flif16_ranges_min(ranges, 0); // luma prediction miss >> > + } >> > + for (int i = 0; i < 4; ++i) { >> > + prop_ranges[top][0] = mind; >> > + prop_ranges[top++][1] = maxd; >> > + } >> > + prop_ranges[top][0] = min; >> > + prop_ranges[top++][0] = max; >> > + >> > + if (property != 2) { >> > + prop_ranges[top][0] = mind; >> > + prop_ranges[top++][1] = maxd; >> > + } >> > + return prop_ranges; >> > +} >> > + >> > + >> > +int ff_flif16_planes_init(FLIF16Context *s, FLIF16PixelData *frames, >> > + uint8_t *plane_mode, uint8_t >> *const_plane_value, >> > + uint8_t lookback) >> > +{ >> > + for (int j = 0; j < s->num_frames; ++j) { >> > + if (frames[j].seen_before >= 0) >> > + continue; >> > + >> >> > + frames[j].data = av_mallocz(sizeof(*frames->data) * >> s->num_planes); >> >> Can num_planes be greater than 5? If yes, then av_mallocz_array(). If >> no, then let us get rid of this dynamic allocation too. >> >> > + >> > + if (!frames[j].data) { >> > + return AVERROR(ENOMEM); >> > + } >> > + >> > + for (int i = 0; i < (s->num_planes + lookback); ++i) { >> >> > + printf("Plane: %d ", i); >> >> Remember to get rid of all printf() for the final version. >> >> > + switch (plane_mode[i]) { >> > + case FLIF16_PLANEMODE_NORMAL: >> >> > + frames[j].data[i] = av_mallocz(sizeof(int32_t) * >> s->width * s->height); >> >> av_malloc_array() and missing error check. >> >> Are width and height validated against multiplication overflow? IIRC, >> ff_set_dimensions() checks against avctx->max_pixels, which can be set >> to more than INT_MAX. >> >> Is the initialization to 0 necessary? It is expensive. >> >> > + break; >> > + >> > + case FLIF16_PLANEMODE_CONSTANT: >> >> > + frames[j].data[i] = av_mallocz(sizeof(int32_t)); >> >> Missing error check. And the initialization to 0 is not necessary. >> >> > + ((int32_t *) frames[j].data[i])[0] = >> const_plane_value[i]; >> > + break; >> > + >> > + case FLIF16_PLANEMODE_FILL: >> >> > + frames[j].data[i] = av_mallocz(sizeof(int32_t) * >> s->width * s->height);; >> >> Same as above. >> >> > + if (!frames[j].data[i]) >> > + return AVERROR(ENOMEM); >> > + for (int k = 0; k < s->height * s->width; ++k) >> > + ((int32_t *) frames[j].data[i])[k] = >> const_plane_value[i]; >> > + break; >> > + } >> > + } >> > + } >> > + >> > + return 0; >> > +} >> > + >> > + >> > +static void ff_flif16_planes_free(FLIF16PixelData *frame, uint8_t >> num_planes, >> > + uint8_t lookback) >> > +{ >> >> > + for(uint8_t i = 0; i < (num_planes + lookback); ++i) { >> >> int i, to avoid confusing the compiler about your intent. >> >> > + av_free(frame->data[i]); >> > + } >> > + av_free(frame->data); >> > +} >> > + >> > +FLIF16PixelData *ff_flif16_frames_init(FLIF16Context *s) >> > +{ >> > + FLIF16PixelData *frames = av_mallocz(sizeof(*frames) * >> s->num_frames); >> >> av_malloc_array() >> >> > + if (!frames) >> > + return NULL; >> > + >> > + for (int i = 0; i < s->num_frames; ++i) >> > + frames[i].seen_before = -1; >> > + return frames; >> > +} >> > + >> > +void ff_flif16_frames_free(FLIF16PixelData **frames, uint32_t >> num_frames, >> > + uint32_t num_planes, uint8_t lookback) >> > +{ >> > + for (int i = 0; i < num_frames; ++i) { >> > + if ((*frames)[i].seen_before >= 0) >> > + continue; >> > + ff_flif16_planes_free(&(*frames)[i], num_planes, lookback); >> > + if ((*frames)[i].col_begin) >> > + av_freep(&(*frames)[i].col_begin); >> > + if ((*frames)[i].col_end) >> > + av_freep(&(*frames)[i].col_end); >> > + } >> > + >> > + av_freep(frames); >> > +} >> > diff --git a/libavcodec/flif16.h b/libavcodec/flif16.h >> > new file mode 100644 >> > index 0000000000..d21cfd79a4 >> > --- /dev/null >> > +++ b/libavcodec/flif16.h >> > @@ -0,0 +1,278 @@ >> > +/* >> > + * FLIF16 Image Format Definitions >> > + * Copyright (c) 2020 Anamitra Ghorui <agho...@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 >> > + * FLIF16 format definitions and functions. >> > + */ >> > + >> > +#ifndef AVCODEC_FLIF16_H >> > +#define AVCODEC_FLIF16_H >> > + >> > +#include <stdint.h> >> > +#include <stdlib.h> >> > + >> > +#include "avcodec.h" >> > +#include "libavutil/pixfmt.h" >> > +#include "flif16_rangecoder.h" >> > + >> > +#define MAX_PLANES 5 >> > +#define MAX_PREDICTORS 2 >> > + >> > +#define VARINT_APPEND(a,x) (a) = ((a) << 7) | (uint32_t) ((x) & 127) >> > +#define ZOOM_ROWPIXELSIZE(zoomlevel) (1 << (((zoomlevel) + 1) / 2)) >> > +#define ZOOM_COLPIXELSIZE(zoomlevel) (1 << (((zoomlevel)) / 2)) >> > +#define ZOOM_HEIGHT(r, z) ((!z) ? 0 : (1 + ((r) - 1) / >> ZOOM_ROWPIXELSIZE(z))) >> > +#define ZOOM_WIDTH(w, z) ((!z) ? 0 : (1 + ((w) - 1) / >> ZOOM_COLPIXELSIZE(z))) >> > +#define MEDIAN3(a, b, c) (((a) < (b)) ? (((b) < (c)) ? (b) : ((a) < >> (c) ? (c) : (a))) : (((a) < (c)) ? (a) : ((b) < (c) ? (c) : (b)))) >> > + >> > +static const uint8_t flif16_header[4] = "FLIF"; >> > + >> > +// Pixeldata types >> > +static const enum AVPixelFormat flif16_out_frame_type[][2] = { >> > + { -1, -1 }, // Padding >> > + { AV_PIX_FMT_GRAY8, AV_PIX_FMT_GRAY16 }, >> > + { -1 , -1 }, // Padding >> > + { AV_PIX_FMT_RGB24, AV_PIX_FMT_RGB48 }, >> > + { AV_PIX_FMT_RGB32, AV_PIX_FMT_RGBA64 } >> > +}; >> > + >> > +typedef enum FLIF16Plane { >> > + FLIF16_PLANE_Y = 0, >> > + FLIF16_PLANE_CO, >> > + FLIF16_PLANE_CG, >> > + FLIF16_PLANE_ALPHA, >> > + FLIF16_PLANE_LOOKBACK, // Frame lookback >> > + FLIF16_PLANE_GRAY = 0, // Is this needed? >> > +} FLIF16Plane; >> > + >> > +typedef enum FLIF16PlaneMode { >> > + FLIF16_PLANEMODE_CONSTANT = 0, ///< A true constant plane >> > + FLIF16_PLANEMODE_NORMAL, ///< A normal pixel matrix >> > + FLIF16_PLANEMODE_FILL /**< A constant plane that is >> later manipulated >> > + by transforms, making it >> nonconstant and >> > + allocating a plane for it */ >> > + >> > +} FLIF16PlaneMode; >> > + >> > +typedef struct FLIF16PixelData { >> >> > + int8_t seen_before; // Required by FrameDup >> > + uint32_t *col_begin; // Required by FrameShape >> > + uint32_t *col_end; // Required by FrameShape >> > + int s_r[MAX_PLANES]; >> > + int s_c[MAX_PLANES]; >> > + void **data; >> >> Nit: Larger fields first, smaller fields last, to avoid padding. Same at >> other places. >> >> > +} FLIF16PixelData; >> > + >> > +typedef int32_t FLIF16ColorVal; >> > + >> > +typedef struct FLIF16Context { >> > + GetByteContext gb; >> > + FLIF16MANIACContext maniac_ctx; >> > + FLIF16RangeCoder rc; >> > + >> >> > + // Dimensions and other things. >> > + uint32_t width; >> > + uint32_t height; >> > + uint32_t num_frames; >> > + uint32_t meta; ///< Size of a meta chunk >> > + >> > + // Primary Header >> > + uint8_t ia; ///< Is image interlaced or/and animated or >> not >> > + uint32_t bpc; ///< 2 ^ Bytes per channel >> > + uint8_t num_planes; ///< Number of planes >> > + uint8_t loops; ///< Number of times animation loops >> > + uint16_t *framedelay; ///< Frame delay for each frame >> > + uint8_t plane_mode[MAX_PLANES]; >> > + >> > + // Transform flags >> > + uint8_t framedup; >> > + uint8_t frameshape; >> > + uint8_t framelookback; >> > +} FLIF16Context; >> > + >> > +typedef struct FLIF16RangesContext { >> > + uint8_t r_no; >> > + uint8_t num_planes; >> >> > + void* priv_data; >> >> Nit: "void *priv_data". >> >> > +} FLIF16RangesContext; >> > + >> > +typedef struct FLIF16Ranges { >> > + uint8_t priv_data_size; >> > + >> > + FLIF16ColorVal (*min)(FLIF16RangesContext *ranges, int plane); >> > + FLIF16ColorVal (*max)(FLIF16RangesContext *ranges, int plane); >> > + void (*minmax)(FLIF16RangesContext *ranges, const int plane, >> > + FLIF16ColorVal *prev_planes, FLIF16ColorVal *minv, >> > + FLIF16ColorVal *maxv); >> > + void (*snap)(FLIF16RangesContext*, const int, FLIF16ColorVal*, >> > + FLIF16ColorVal*, FLIF16ColorVal*, FLIF16ColorVal*); >> > + uint8_t is_static; >> > + void (*close)(FLIF16RangesContext*); >> > + void (*previous)(FLIF16RangesContext*); //TODO : Maybe remove it >> later >> > +} FLIF16Ranges; >> > + >> >> > +typedef struct FLIF16TransformContext{ >> >> Nit: space. >> >> > + uint8_t t_no; >> > + unsigned int segment; ///< Segment the code is executing in. >> > + int i; ///< Variable to store iteration number. >> > + uint8_t done; >> > + void *priv_data; >> > +} FLIF16TransformContext; >> > + >> > +typedef struct FLIF16Transform { >> > + int16_t priv_data_size; >> > + //Functions >> > + int (*init) (FLIF16TransformContext *t_ctx, FLIF16RangesContext >> *r_ctx); >> > + int (*read) (FLIF16TransformContext *t_ctx, FLIF16Context *ctx, >> > + FLIF16RangesContext *r_ctx); >> > + FLIF16RangesContext *(*meta) (FLIF16Context *ctx, >> > + FLIF16PixelData *frame, uint32_t >> frame_count, >> > + FLIF16TransformContext *t_ctx, >> > + FLIF16RangesContext *r_ctx); >> > + int (*forward) (FLIF16Context *ctx, FLIF16TransformContext *t_ctx, >> FLIF16PixelData *frame); >> > + int (*reverse) (FLIF16Context *ctx, FLIF16TransformContext *t_ctx, >> FLIF16PixelData *frame, >> > + uint32_t stride_row, uint32_t stride_col); >> > + void (*configure) (FLIF16TransformContext *, const int); >> > + void (*close) (FLIF16TransformContext *t_ctx); >> > +} FLIF16Transform; >> > + >> > +int32_t (*ff_flif16_maniac_ni_prop_ranges_init(unsigned int >> *prop_ranges_size, >> > + FLIF16RangesContext >> *ranges, >> > + uint8_t property, >> > + uint8_t channels))[2]; >> > + >> > +int32_t (*ff_flif16_maniac_prop_ranges_init(unsigned int >> *prop_ranges_size, >> > + FLIF16RangesContext >> *ranges, >> > + uint8_t property, >> > + uint8_t channels))[2]; >> > + >> > +int ff_flif16_planes_init(FLIF16Context *s, FLIF16PixelData *frames, >> > + uint8_t *is_const, uint8_t >> *const_plane_value, >> > + uint8_t lookback); >> > + >> > +FLIF16PixelData *ff_flif16_frames_init(FLIF16Context *s); >> > + >> > +void ff_flif16_frames_free(FLIF16PixelData **frames, uint32_t >> num_frames, >> > + uint32_t num_planes, uint8_t lookback); >> > + >> > + >> > + >> > +/* >> > + * All constant plane pixel setting should be illegal in theory. >> > + */ >> > + >> > +static inline void ff_flif16_pixel_set(FLIF16Context *s, >> FLIF16PixelData *frame, >> > + uint8_t plane, uint32_t row, >> uint32_t col, >> > + FLIF16ColorVal value) >> > +{ >> > + if (s->plane_mode[plane]) >> > + ((FLIF16ColorVal *) frame->data[plane])[s->width * row + col] >> = value; >> > + else >> > + ((FLIF16ColorVal *) frame->data[plane])[0] = value; >> > +} >> > + >> > +static inline FLIF16ColorVal ff_flif16_pixel_get(FLIF16Context *s, >> > + FLIF16PixelData >> *frame, >> > + uint8_t plane, >> uint32_t row, >> > + uint32_t col) >> > +{ >> > + if (s->plane_mode[plane]) >> > + return ((FLIF16ColorVal *) frame->data[plane])[s->width * row >> + col]; >> > + else >> > + return ((FLIF16ColorVal *) frame->data[plane])[0]; >> > +} >> > + >> > + >> > +static inline void ff_flif16_pixel_setz(FLIF16Context *s, >> > + FLIF16PixelData *frame, >> > + uint8_t plane, int z, uint32_t >> row, >> > + uint32_t col, FLIF16ColorVal >> value) >> > +{ >> > + if (s->plane_mode[plane]) >> > + ((FLIF16ColorVal *) frame->data[plane])[(row * >> ZOOM_ROWPIXELSIZE(z)) * s->width + >> > + (col * >> ZOOM_COLPIXELSIZE(z))] = value; >> > + else >> > + ((FLIF16ColorVal *) frame->data[plane])[0] = value; >> > +} >> > + >> > +static inline FLIF16ColorVal ff_flif16_pixel_getz(FLIF16Context *s, >> > + FLIF16PixelData >> *frame, >> > + uint8_t plane, int z, >> > + size_t row, size_t >> col) >> > +{ >> > + if (s->plane_mode[plane]) >> > + return ((FLIF16ColorVal *) frame->data[plane])[(row * >> ZOOM_ROWPIXELSIZE(z)) * >> > + s->width + (col >> * ZOOM_COLPIXELSIZE(z))]; >> > + else >> > + return ((FLIF16ColorVal *) frame->data[plane])[0]; >> > +} >> > + >> > +static inline void ff_flif16_prepare_zoomlevel(FLIF16Context *s, >> > + FLIF16PixelData *frame, >> > + uint8_t plane, int z) >> > +{ >> > + frame->s_r[plane] = ZOOM_ROWPIXELSIZE(z) * s->width; >> > + frame->s_c[plane] = ZOOM_COLPIXELSIZE(z); >> > +} >> > + >> > +static inline FLIF16ColorVal ff_flif16_pixel_get_fast(FLIF16Context *s, >> > + FLIF16PixelData >> *frame, >> > + uint8_t plane, >> uint32_t row, >> > + uint32_t col) >> > +{ >> > + if (s->plane_mode[plane]) >> > + return ((FLIF16ColorVal *) frame->data[plane])[row * >> frame->s_r[plane] + col * frame->s_c[plane]]; >> > + >> > + return 0; >> > +} >> > + >> > +static inline void ff_flif16_pixel_set_fast(FLIF16Context *s, >> > + FLIF16PixelData *frame, >> > + uint8_t plane, uint32_t >> row, >> > + uint32_t col, >> FLIF16ColorVal value) >> > +{ >> > + if (s->plane_mode[plane]) >> > + ((FLIF16ColorVal *) frame->data[plane])[row * >> frame->s_r[plane] + col * frame->s_c[plane]] = value; >> > +} >> > + >> > +static inline void ff_flif16_copy_rows(FLIF16Context *s, >> > + FLIF16PixelData *dest, >> > + FLIF16PixelData *src, uint8_t >> plane, >> > + uint32_t row, uint32_t >> col_start, >> > + uint32_t col_end) >> > +{ >> > + for(uint32_t col = col_start; col < col_end; ++col) { >> >> > + ff_flif16_pixel_set(s, dest, plane, row, col, >> ff_flif16_pixel_get(s, src, plane, row, col)); >> >> ff_flif16_pixel_get() and ff_flif16_pixel_set() hide some arithmetic, >> re-computed for each pixel. I do not trust compilers to optimize them. >> >> Better take a pointer to the source, a pointer to the destination, and >> increment them by the right amount. >> >> > + } >> > +} >> > + >> > +static inline void ff_flif16_copy_rows_stride(FLIF16Context *s, >> > + FLIF16PixelData *dest, >> > + FLIF16PixelData *src, >> uint8_t plane, >> > + uint32_t row, uint32_t >> col_start, >> > + uint32_t col_end, >> uint32_t stride) >> > +{ >> > + for(uint32_t col = col_start; col < col_end; col += stride) { >> > + ff_flif16_pixel_set(s, dest, plane, row, col, >> ff_flif16_pixel_get(s, src, plane, row, col)); >> > + } >> >> Same. >> >> > +} >> > +#endif /* AVCODEC_FLIF16_H */ >> > diff --git a/libavcodec/flif16_parser.c b/libavcodec/flif16_parser.c >> > new file mode 100644 >> > index 0000000000..c795b44b4d >> > --- /dev/null >> > +++ b/libavcodec/flif16_parser.c >> > @@ -0,0 +1,189 @@ >> > +/* >> > + * FLIF16 parser >> > + * Copyright (c) 2020 Anamitra Ghorui <agho...@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 >> > + * FLIF16 parser >> > + */ >> > + >> > +#include "flif16.h" >> > +#include "parser.h" >> > +#include "libavutil/avassert.h" >> > +#include "libavutil/bswap.h" >> > + >> > +#include <stdio.h> //remove >> > +#include <stdint.h> >> > +#include <stdlib.h> >> > + >> > +typedef enum FLIF16ParseStates { >> >> FLIF16_INIT_STATE = 0, >> >> It is not elegant to use an enum and a value that is not part of the >> enum. >> >> > + FLIF16_HEADER = 1, >> > + FLIF16_METADATA, >> > + FLIF16_BITSTREAM >> > +} FLIF16ParseStates; >> > + >> > +typedef struct FLIF16ParseContext { >> > + ParseContext pc; >> >> > + int state; ///< The section of the file the parser is in >> currently. >> >> FLIF16ParseStates state; >> >> > + unsigned int index; ///< An index based on the current state. >> > + uint8_t animated; ///< Is image animated or not >> > + uint8_t varint; ///< Number of varints to process in sequence >> > + uint32_t width; >> > + uint32_t height; >> > + uint32_t frames; >> > + uint32_t meta; ///< Size of a meta chunk >> > + uint32_t count; >> > +} FLIF16ParseContext; >> > + >> > + >> > +// TODO revamp this function >> > +static int flif16_find_frame(FLIF16ParseContext *f, const uint8_t *buf, >> > + int buf_size) >> > +{ >> > + int next = END_NOT_FOUND; >> > + int index; >> > + >> > + for (index = 0; index < buf_size; index++) { >> > + if (!f->state) { >> > + if (!memcmp(flif16_header, buf + index, 4)) >> > + f->state = FLIF16_HEADER; >> > + ++f->index; >> >> > + } else if (f->state == FLIF16_HEADER) { >> >> switch (f->state)? >> >> > + if (f->index == 3 + 1) { >> > + // See whether image is animated or not >> > + f->animated = (((buf[index] >> 4) > 4)?1:0); >> > + } else if (f->index == (3 + 1 + 1)) { >> > + // Start - 1 of the first varint >> > + f->varint = 1; >> > + } else if (f->varint) { >> > + // Count varint >> > + if (f->count == 5) >> >> > + return AVERROR(ENOMEM); >> >> AVERROR_INVALIDDATA >> >> > + >> > + switch (f->varint) { >> > + case 1: >> > + VARINT_APPEND(f->width, buf[index]); >> > + break; >> > + >> > + case 2: >> > + VARINT_APPEND(f->height, buf[index]); >> > + break; >> > + >> > + case 3: >> > + VARINT_APPEND(f->frames, buf[index]); >> > + break; >> > + } >> > + if (buf[index] < 128) { >> > + if (f->varint < (2 + f->animated)) { >> > + switch (f->varint) { >> > + case 1: f->width++; break; >> > + case 2: f->height++; break; >> > + } >> > + f->varint++; >> > + f->count = 0; >> > + } else { >> > + if (f->varint == 2) >> > + f->height++; >> > + if (f->animated) >> > + f->frames += 2; >> > + else >> > + f->frames = 1; >> > + f->state = FLIF16_METADATA; >> > + f->varint = 0; >> > + f->index = 0; >> > + f->count = 0; >> > + continue; >> > + } >> > + } else { >> > + f->count++; >> > + } >> > + } >> > + f->index++; >> > + } else if (f->state == FLIF16_METADATA) { >> > + if (f->index == 0) { >> > + // Identifier for the bitstream chunk is a null byte. >> > + if (buf[index] == 0) { >> > + f->state = FLIF16_BITSTREAM; >> > + return buf_size; >> > + } >> > + } else if (f->index < 3) { >> > + // nop >> > + } else if (f->index == 3) { >> > + // Handle the size varint >> > + f->varint = 1; >> > + } else if (f->varint) { >> > + if (f->count == 9) >> >> > + return AVERROR(ENOMEM); >> >> AVERROR_INVALIDDATA >> >> > + if (buf[index] < 128) { >> > + f->varint = 0; >> > + f->count = 0; >> > + } >> > + VARINT_APPEND(f->meta, buf[index]); >> > + f->count++; >> > + } else if (f->meta > 1) { >> > + // increment varint until equal to size >> > + f->meta--; >> > + } else { >> > + f->meta = 0; >> > + f->index = 0; >> > + continue; >> > + } >> > + f->index++; >> > + } else if (f->state == FLIF16_BITSTREAM) { >> > + /* Since we cannot find the end of the bitstream without >> any >> > + * processing, we will simply return each read chunk as a >> packet >> > + * to the decoder. >> > + */ >> > + printf("<Bitstream chunk size %dd>\n", buf_size); >> > + return buf_size; >> > + } >> > + } >> > + printf("End not found\n"); >> > + return next; >> > +} >> > + >> > +static int flif16_parse(AVCodecParserContext *s, AVCodecContext *avctx, >> > + const uint8_t **poutbuf, int *poutbuf_size, >> > + const uint8_t *buf, int buf_size) >> > +{ >> > + FLIF16ParseContext *fpc = s->priv_data; >> > + int next; >> > + >> > + next = flif16_find_frame(fpc, buf, buf_size); >> > + >> > + if (ff_combine_frame(&fpc->pc, next, &buf, &buf_size) < 0) { >> > + *poutbuf = NULL; >> > + *poutbuf_size = 0; >> > + return buf_size; >> > + } >> > + printf("Width:%u\nHeight:%u\nFrames:%u\nEnd:%d\n", >> > + fpc->width, fpc->height, fpc->frames, buf_size); >> > + *poutbuf = buf; >> > + *poutbuf_size = buf_size; >> > + return next; >> > +} >> > + >> > +AVCodecParser ff_flif16_parser = { >> > + .codec_ids = { AV_CODEC_ID_FLIF16 }, >> > + .priv_data_size = sizeof(FLIF16ParseContext), >> > + .parser_parse = flif16_parse, >> > + .parser_close = ff_parse_close, >> > +}; >> > + >> > diff --git a/libavcodec/flif16_rangecoder.c >> b/libavcodec/flif16_rangecoder.c >> > new file mode 100644 >> > index 0000000000..c8f1b7bbb0 >> > --- /dev/null >> > +++ b/libavcodec/flif16_rangecoder.c >> > @@ -0,0 +1,464 @@ >> > +/* >> > + * Range coder for FLIF16 >> >> > + * Copyright (c) 2004, Michael Niedermayer, >> >> This looks like new code. Can you explain where Michael's copyright >> comes from? >> >> > + * 2010-2016, Jon Sneyers & Pieter Wuille, >> >> Same here. >> >> > + * 2020, Anamitra Ghorui <agho...@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 >> > + * Range coder for FLIF16 >> > + */ >> > + >> > +#include "avcodec.h" >> > +#include "libavutil/common.h" >> > +#include "flif16_rangecoder.h" >> > +#include "flif16.h" >> > + >> > +// TODO write separate function for RAC encoder >> > + >> > +// The coder requires a certain number of bytes for initiialization. >> buf >> > +// provides it. gb is used by the coder functions for actual coding. >> > +void ff_flif16_rac_init(FLIF16RangeCoder *rc, GetByteContext *gb, >> uint8_t *buf, >> > + uint8_t buf_size) >> > +{ >> > + GetByteContext gbi; >> > + if(!rc) >> > + return; >> > + >> > + if(buf_size < FLIF16_RAC_MAX_RANGE_BYTES) >> > + return; >> > + >> > + bytestream2_init(&gbi, buf, buf_size); >> > + >> > + rc->range = FLIF16_RAC_MAX_RANGE; >> > + rc->gb = gb; >> > + >> >> > + for (uint32_t r = FLIF16_RAC_MAX_RANGE; r > 1; r >>= 8) { >> > + rc->low <<= 8; >> > + rc->low |= bytestream2_get_byte(&gbi); >> > + } >> >> Do you need bytestream2_get_byte() for that? Testing that buf_size is >> large enough at the beginning and directly accessing buf seems simpler >> and more efficient. >> >> > +} >> > + >> > +void ff_flif16_rac_free(FLIF16RangeCoder *rc) >> > +{ >> > + if (!rc) >> > + return; >> >> > + av_freep(rc); >> >> Was this tested? av_freep() wants a pointer to pointer. >> >> > +} >> > + >> > +// TODO Maybe restructure rangecoder.c/h to fit a more generic case >> > +static void build_table(uint16_t *zero_state, uint16_t *one_state, >> size_t size, >> > + uint32_t factor, unsigned int max_p) >> > +{ >> > + const int64_t one = 1LL << 32; >> > + int64_t p = one / 2; >> > + unsigned int last_p8 = 0, p8; >> > + unsigned int i; >> > + >> > + for (i = 0; i < size / 2; i++) { >> > + p8 = (size * p + one / 2) >> 32; >> > + if (p8 <= last_p8) >> > + p8 = last_p8 + 1; >> > + if (last_p8 && last_p8 < size && p8 <= max_p) >> > + one_state[last_p8] = p8; >> > + p += ((one - p) * factor + one / 2) >> 32; >> > + last_p8 = p8; >> > + } >> > + >> > + for (i = size - max_p; i <= max_p; i++) { >> > + if (one_state[i]) >> > + continue; >> > + p = (i * one + size / 2) / size; >> > + p += ((one - p) * factor + one / 2) >> 32; >> > + p8 = (size * p + one / 2) >> 32; //FIXME try without the one >> > + if (p8 <= i) >> > + p8 = i + 1; >> > + if (p8 > max_p) >> > + p8 = max_p; >> > + one_state[i] = p8; >> > + } >> > + >> > + for (i = 1; i < size; i++) >> > + zero_state[i] = size - one_state[size - i]; >> > +} >> > + >> > +static inline uint32_t log4kf(int x, uint32_t base) >> > +{ >> >> > + int bits = 8 * sizeof(int) - ff_clz(x); >> >> Code relying on sizeof(int) for anything but allocating memory is very >> suspicious. >> >> > + uint64_t y = ((uint64_t)x) << (32 - bits); >> > + uint32_t res = base * (13 - bits); >> > + uint32_t add = base; >> > + while ((add > 1) && ((y & 0x7FFFFFFF) != 0)) { >> > + y = (((uint64_t)y) * y + 0x40000000) >> 31; >> > + add >>= 1; >> > + if ((y >> 32) != 0) { >> > + res -= add; >> > + y >>= 1; >> > + } >> > + } >> > + return res; >> > +} >> > + >> > +void ff_flif16_build_log4k_table(FLIF16Log4kTable *log4k) >> > +{ >> > + log4k->table[0] = 0; >> > + for (int i = 1; i < 4096; i++) >> > + log4k->table[i] = (log4kf(i, (65535UL << 16) / 12) + >> > + (1 << 15)) >> 16; >> > + log4k->scale = 65535 / 12; >> > +} >> > + >> > +void ff_flif16_chancetable_init(FLIF16ChanceTable *ct, int alpha, int >> cut) >> > +{ >> > + build_table(ct->zero_state, ct->one_state, 4096, alpha, 4096 - >> cut); >> > +} >> > + >> > +void ff_flif16_chancecontext_init(FLIF16ChanceContext *ctx) >> > +{ >> >> > + if(!ctx) >> > + return; >> >> You never call this except with &something as argument: remove this >> useless check, and let your code crash if you get something wrong when >> debugging. >> >> > + memcpy(&ctx->data, &flif16_nz_int_chances, >> sizeof(flif16_nz_int_chances)); >> > +} >> > + >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > +FLIF16MultiscaleChanceTable >> *ff_flif16_multiscale_chancetable_init(void) >> > +{ >> > + unsigned int len = MULTISCALE_CHANCETABLE_DEFAULT_SIZE; >> > + FLIF16MultiscaleChanceTable *ct = av_malloc(sizeof(*ct)); >> > + if (!ct) >> > + return null >> > + for (int i = 0; i < len; ++i) { >> > + ff_flif16_chancetable_init(&ct->sub_table[i], >> > + flif16_multiscale_alphas[i], >> > + MULTISCALE_CHANCETABLE_DEFAULT_CUT); >> > + } >> > + return ct; >> > +} >> > + >> > +/** >> > + * Allocate and set all chances according to flif16_nz_int_chances >> > + */ >> > +void >> ff_flif16_multiscale_chancecontext_init(FLIF16MultiscaleChanceContext *ctx) >> > +{ >> >> > + for (int i = 0; i < sizeof(flif16_nz_int_chances) / >> > + sizeof(flif16_nz_int_chances[0]); ++i) >> >> FF_ARRAY_ELEMS(); possibly same in other places. >> >> > + ff_flif16_multiscale_chance_set(&ctx->data[i], >> flif16_nz_int_chances[i]); >> > + return ctx; >> > +} >> > + >> > +#endif >> > + >> > +int ff_flif16_read_maniac_tree(FLIF16RangeCoder *rc, >> > + FLIF16MANIACContext *m, >> > + int32_t (*prop_ranges)[2], >> > + unsigned int prop_ranges_size, >> > + unsigned int channel) >> > +{ >> > + int oldp = 0, p = 0, split_val = 0, temp; >> > + >> > + switch (rc->segment2) { >> >> > + default: case 0: >> >> Nit: do not indent cases more than switch(). >> >> > + rc->segment2 = 0; >> > + if (!(m->forest[channel])) { >> > + m->forest[channel] = >> av_mallocz(sizeof(*(m->forest[channel]))); >> > + if (!(m->forest[channel])) >> > + return AVERROR(ENOMEM); >> > + m->forest[channel]->data = >> av_mallocz(MANIAC_TREE_BASE_SIZE * >> > + >> sizeof(*(m->forest[channel]->data))); >> >> av_mallocz_array(). >> >> > + if (!m->forest[channel]->data) >> > + return AVERROR(ENOMEM); >> > + >> > + m->stack = av_mallocz(MANIAC_TREE_BASE_SIZE * >> sizeof(*(m->stack))); >> >> Same. >> >> > + >> > + if (!(m->stack)) >> >> Nit: parentheses not necessary. >> >> > + return AVERROR(ENOMEM); >> > + >> > + for (int i = 0; i < 3; ++i) { >> > + #ifdef MULTISCALE_CHANCES_ENABLED >> > + >> ff_flif16_multiscale_chancecontext_init(&m->ctx[i]); >> > + #else >> > + ff_flif16_chancecontext_init(&m->ctx[i]); >> > + #endif >> > + } >> > + m->stack_top = m->tree_top = 0; >> >> > + m->forest[channel]->size = MANIAC_TREE_BASE_SIZE; >> >> Strange spacing. >> >> > + m->stack_size = MANIAC_TREE_BASE_SIZE; >> > + m->stack[m->stack_top].id = m->tree_top; >> > + m->stack[m->stack_top].mode = 0; >> > + ++m->stack_top; >> > + ++m->tree_top; >> > + } >> > + ++rc->segment2; >> > + >> > + case 1: >> >> > + start: >> > + if(!m->stack_top) >> > + goto end; >> > + >> >> Looks like precisely the kind of code for which the rule "don't use >> goto" was coined. Better make it a proper loop. >> >> > + oldp = m->stack[m->stack_top - 1].p; >> > + if (!m->stack[m->stack_top - 1].visited) { >> > + switch (m->stack[m->stack_top - 1].mode) { >> > + case 1: >> > + prop_ranges[oldp][0] = m->stack[m->stack_top - >> 1].min; >> > + prop_ranges[oldp][1] = m->stack[m->stack_top - >> 1].max; >> > + break; >> > + >> > + case 2: >> > + prop_ranges[oldp][0] = m->stack[m->stack_top - >> 1].min; >> > + break; >> > + } >> > + } else { >> > + prop_ranges[oldp][1] = m->stack[m->stack_top - 1].max2; >> > + --m->stack_top; >> > + rc->segment2 = 1; >> > + goto start; >> > + } >> > + m->stack[m->stack_top - 1].visited = 1; >> > + ++rc->segment2; >> > + >> > + case 2: >> > + #ifdef MULTISCALE_CHANCES_ENABLED >> > + RAC_GET(rc, &m->ctx[0], 0, prop_ranges_size, >> > + &m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].property, >> > + FLIF16_RAC_GNZ_MULTISCALE_INT); >> > + #else >> > + RAC_GET(rc, &m->ctx[0], 0, prop_ranges_size, >> > + &m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].property, >> > + FLIF16_RAC_GNZ_INT); >> > + #endif >> > + p = --(m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].property); >> > + if (p == -1) { >> > + --m->stack_top; >> > + rc->segment2 = 1; >> > + goto start; >> > + } >> > + >> > + m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].child_id = m->tree_top; >> > + rc->oldmin = prop_ranges[p][0]; >> > + rc->oldmax = prop_ranges[p][1]; >> > + if (rc->oldmin >= rc->oldmax) { >> > + printf("!!! rc->oldmin >= rc->oldmax\n"); >> >> > + return AVERROR(EINVAL); >> >> AVERROR_INVALIDDATA >> >> > + } >> > + ++rc->segment2; >> > + >> > + case 3: >> > + #ifdef MULTISCALE_CHANCES_ENABLED >> > + RAC_GET(rc, &m->ctx[1], MANIAC_TREE_MIN_COUNT, >> MANIAC_TREE_MAX_COUNT, >> > + &m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].count, >> > + FLIF16_RAC_GNZ_MULTISCALE_INT); >> > + #else >> > + RAC_GET(rc, &m->ctx[1], MANIAC_TREE_MIN_COUNT, >> MANIAC_TREE_MAX_COUNT, >> > + &m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].count, >> > + FLIF16_RAC_GNZ_INT); >> > + #endif >> > + ++rc->segment2; >> > + >> > + case 4: >> > + #ifdef MULTISCALE_CHANCES_ENABLED >> > + RAC_GET(rc, &m->ctx[2], rc->oldmin, rc->oldmax - 1, >> > + &m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].split_val, >> > + FLIF16_RAC_GNZ_MULTISCALE_INT); >> > + #else >> > + RAC_GET(rc, &m->ctx[2], rc->oldmin, rc->oldmax - 1, >> > + &m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].split_val, >> > + FLIF16_RAC_GNZ_INT); >> > + #endif >> > + split_val = m->forest[channel]->data[m->stack[m->stack_top >> - 1].id].split_val; >> > + ++rc->segment2; >> > + >> > + case 5: >> > + if ((m->tree_top + 2) >= m->forest[channel]->size) { >> > + m->forest[channel]->data = >> av_realloc(m->forest[channel]->data, >> > + (m->forest[channel]->size) * 2 * >> sizeof(*(m->forest[channel]->data))); >> > + if(!(m->forest[channel]->data)) >> > + return AVERROR(ENOMEM); >> > + m->forest[channel]->size *= 2; >> > + } >> > + >> > + if ((m->stack_top + 2) >= m->stack_size) { >> >> > + m->stack = av_realloc(m->stack, (m->stack_size) * 2 * >> sizeof(*(m->stack))); >> >> av_realloc_array() >> >> > + if(!(m->stack)) >> > + return AVERROR(ENOMEM); >> >> This leaks the old m->stack. See av_realloc_f(). >> >> > + m->stack_size *= 2; >> > + } >> > + >> > + temp = m->forest[channel]->data[m->stack[m->stack_top - >> 1].id].property; >> > + >> > + // Parent >> > + m->stack[m->stack_top - 1].p = temp; >> > + m->stack[m->stack_top - 1].max2 = rc->oldmax; >> > + >> > + // Right child >> > + m->stack[m->stack_top].id = m->tree_top + 1; >> > + m->stack[m->stack_top].p = temp; >> > + m->stack[m->stack_top].min = rc->oldmin; >> > + m->stack[m->stack_top].max = split_val; >> > + m->stack[m->stack_top].mode = 1; >> > + m->stack[m->stack_top].visited = 0; >> > + ++m->stack_top; >> > + >> > + // Left Child >> > + m->stack[m->stack_top].id = m->tree_top; >> > + m->stack[m->stack_top].p = temp; >> > + m->stack[m->stack_top].min = split_val + 1; >> > + m->stack[m->stack_top].mode = 2; >> > + m->stack[m->stack_top].visited = 0; >> > + ++m->stack_top; >> > + >> > + m->tree_top += 2; >> > + rc->segment2 = 1; >> > + goto start; >> > + } >> > + >> > + end: >> > + m->forest[channel]->data = av_realloc(m->forest[channel]->data, >> > + m->tree_top * >> sizeof(*m->forest[channel]->data)); // Maybe replace by fast realloc >> > + if (!m->forest[channel]->data) >> > + return AVERROR(ENOMEM); >> > + m->forest[channel]->size = m->tree_top; >> > + av_freep(&m->stack); >> > + m->stack_top = 0; >> > + rc->segment2 = 0; >> > + return 0; >> > + >> > + need_more_data: >> > + return AVERROR(EAGAIN); >> > +} >> > + >> > +void ff_flif16_maniac_close(FLIF16MANIACContext *m, uint8_t >> num_planes) >> > +{ >> > + for (int i = 0; i < num_planes; ++i) { >> > + if (!m->forest[i]) >> > + continue; >> > + if (m->forest[i]->data) >> > + av_freep(&m->forest[i]->data); >> > + if (m->forest[i]->leaves) >> > + av_freep(&m->forest[i]->leaves); >> > + av_freep(&m->forest[i]); >> > + } >> > + >> > + av_freep(&m->forest); >> > + // Should be already freed in maniac reading, but checking anyway. >> > + if(m->stack) >> > + av_freep(&m->stack); >> > +} >> > + >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > +FLIF16MultiscaleChanceContext >> *ff_flif16_maniac_findleaf(FLIF16MANIACContext *m, >> > + uint8_t >> channel, >> > + int32_t >> *properties) >> > +#else >> > +FLIF16ChanceContext *ff_flif16_maniac_findleaf(FLIF16MANIACContext *m, >> > + uint8_t channel, >> > + int32_t *properties) >> > +#endif >> > +{ >> > + unsigned int pos = 0; >> > + uint32_t old_leaf; >> > + uint32_t new_leaf; >> > + FLIF16MANIACTree *tree = m->forest[channel]; >> > + FLIF16MANIACNode *nodes = tree->data; >> > + >> > + if (!m->forest[channel]->leaves) { >> > + m->forest[channel]->leaves = av_mallocz(MANIAC_TREE_BASE_SIZE * >> > + >> sizeof(*m->forest[channel]->leaves)); >> >> av_mallocz_array(); >> >> > + m->forest[channel]->leaves_size = MANIAC_TREE_BASE_SIZE; >> > + if(!m->forest[channel]->leaves) >> > + return NULL; >> > + ff_flif16_chancecontext_init(&m->forest[channel]->leaves[0]); >> > + tree->leaves_top = 1; >> > + } >> > + >> > + while (nodes[pos].property != -1) { >> > + if (nodes[pos].count < 0) { >> > + if (properties[nodes[pos].property] > nodes[pos].split_val) >> > + pos = nodes[pos].child_id; >> > + else >> > + pos = nodes[pos].child_id + 1; >> > + } else if (nodes[pos].count > 0) { >> > + --nodes[pos].count; >> > + break; >> > + } else { >> > + --nodes[pos].count; >> > + if ((tree->leaves_top) >= tree->leaves_size) { >> > + m->forest[channel]->leaves = >> av_realloc(m->forest[channel]->leaves, >> > + >> sizeof(*m->forest[channel]->leaves) * >> > + >> m->forest[channel]->leaves_size * 2); >> >> > + if (!m->forest[channel]->leaves) >> > + return NULL; >> >> This leaks old leaves. >> >> > + m->forest[channel]->leaves_size *= 2; >> > + } >> > + old_leaf = nodes[pos].leaf_id; >> > + new_leaf = tree->leaves_top; >> > + memcpy(&m->forest[channel]->leaves[tree->leaves_top], >> > + &m->forest[channel]->leaves[nodes[pos].leaf_id], >> > + sizeof(*m->forest[channel]->leaves)); >> > + ++tree->leaves_top; >> > + nodes[nodes[pos].child_id].leaf_id = old_leaf; >> > + nodes[nodes[pos].child_id + 1].leaf_id = new_leaf; >> > + >> > + if (properties[nodes[pos].property] > nodes[pos].split_val) >> > + return &m->forest[channel]->leaves[old_leaf]; >> > + else >> > + return &m->forest[channel]->leaves[new_leaf]; >> > + } >> > + } >> > + return >> &m->forest[channel]->leaves[m->forest[channel]->data[pos].leaf_id]; >> > +} >> > + >> > +int ff_flif16_maniac_read_int(FLIF16RangeCoder *rc, >> > + FLIF16MANIACContext *m, >> > + int32_t *properties, >> > + uint8_t channel, >> > + int min, int max, int *target) >> > +{ >> > + if (!rc->maniac_ctx) >> > + rc->segment2 = 0; >> > + >> > + switch(rc->segment2) { >> > + case 0: >> > + if (min == max) { >> > + *target = min; >> > + goto end; >> > + } >> > + rc->maniac_ctx = ff_flif16_maniac_findleaf(m, channel, >> properties); >> > + if(!rc->maniac_ctx) { >> > + return AVERROR(ENOMEM); >> > + } >> > + ++rc->segment2; >> > + >> > + case 1: >> > + #ifdef MULTISCALE_CHANCES_ENABLED >> > + RAC_GET(rc, rc->maniac_ctx, min, max, target, >> FLIF16_RAC_NZ_MULTISCALE_INT); >> > + #else >> > + RAC_GET(rc, rc->maniac_ctx, min, max, target, >> FLIF16_RAC_NZ_INT); >> > + #endif >> > + >> > + } >> > + >> > + end: >> > + rc->maniac_ctx = NULL; >> > + rc->segment2 = 0; >> > + return 1; >> > + >> > + need_more_data: >> > + return 0; >> > +} >> > diff --git a/libavcodec/flif16_rangecoder.h >> b/libavcodec/flif16_rangecoder.h >> > new file mode 100644 >> > index 0000000000..9cd2d5ee22 >> > --- /dev/null >> > +++ b/libavcodec/flif16_rangecoder.h >> > @@ -0,0 +1,824 @@ >> > +/* >> > + * Range coder for FLIF16 >> > + * Copyright (c) 2020 Anamitra Ghorui <agho...@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 >> > + * Range coder for FLIF16. >> > + */ >> > + >> > +#ifndef FLIF16_RANGECODER_H >> > +#define FLIF16_RANGECODER_H >> > + >> > +#include "libavutil/mem.h" >> > +#include "libavutil/intmath.h" >> > +#include "bytestream.h" >> > +#include "rangecoder.h" >> > + >> > +#include <stdint.h> >> > + >> > + >> > +#define FLIF16_RAC_MAX_RANGE_BITS 24 >> > +#define FLIF16_RAC_MAX_RANGE_BYTES (FLIF16_RAC_MAX_RANGE_BITS / 8) >> > +#define FLIF16_RAC_MIN_RANGE_BITS 16 >> > +#define FLIF16_RAC_MAX_RANGE (uint32_t) 1 << FLIF16_RAC_MAX_RANGE_BITS >> > +#define FLIF16_RAC_MIN_RANGE (uint32_t) 1 << FLIF16_RAC_MIN_RANGE_BITS >> > + >> > +#define CHANCETABLE_DEFAULT_ALPHA (0xFFFFFFFF / 19) >> > +#define CHANCETABLE_DEFAULT_CUT 2 >> > + >> > +// #define MULTISCALE_CHANCES_ENABLED >> > + >> > +#define MULTISCALE_CHANCETABLE_DEFAULT_SIZE 6 >> > +#define MULTISCALE_CHANCETABLE_DEFAULT_CUT 8 >> > + >> > +#define MANIAC_TREE_BASE_SIZE 1600 >> > +#define MANIAC_TREE_MIN_COUNT 1 >> > +#define MANIAC_TREE_MAX_COUNT 512 >> > + >> > +typedef enum FLIF16RACReader { >> > + FLIF16_RAC_BIT = 0, >> > + FLIF16_RAC_UNI_INT8, >> > + FLIF16_RAC_UNI_INT16, >> > + FLIF16_RAC_UNI_INT32, >> > + FLIF16_RAC_CHANCE, >> > + FLIF16_RAC_NZ_INT, >> > + FLIF16_RAC_GNZ_INT, >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > + FLIF16_RAC_NZ_MULTISCALE_INT, >> > + FLIF16_RAC_GNZ_MULTISCALE_INT >> > +#endif >> > +} FLIF16RACReader; >> > + >> > +typedef struct FLIF16ChanceTable { >> > + uint16_t zero_state[4096]; >> > + uint16_t one_state[4096]; >> > +} FLIF16ChanceTable; >> > + >> > +typedef struct FLIF16MultiscaleChanceTable { >> > + FLIF16ChanceTable sub_table[MULTISCALE_CHANCETABLE_DEFAULT_SIZE]; >> > +} FLIF16MultiscaleChanceTable; >> > + >> > + >> > +typedef struct FLIF16Log4kTable { >> > + uint16_t table[4097]; >> > + int scale; >> > +} FLIF16Log4kTable; >> > + >> >> > +static const uint32_t flif16_multiscale_alphas[] = { >> > + 21590903, 66728412, 214748365, 7413105, 106514140, 10478104 >> > +}; >> >> Please add a short comment to explain. >> >> > + >> > +typedef struct FLIF16MultiscaleChance { >> > + uint16_t chances[MULTISCALE_CHANCETABLE_DEFAULT_SIZE]; >> > + uint32_t quality[MULTISCALE_CHANCETABLE_DEFAULT_SIZE]; >> > + uint8_t best; >> > +} FLIF16MultiscaleChance; >> > + >> > +static uint16_t flif16_nz_int_chances[] = { >> > + 1000, // ZERO >> > + 2048, // SIGN (0) (1) >> > + 1000, 1000, // EXP: 0, 1 >> > + 1200, 1200, // EXP: 2, 3 >> > + 1500, 1500, // EXP: 4, 5 >> > + 1750, 1750, // EXP: 6, 7 >> > + 2000, 2000, // EXP: 8, 9 >> > + 2300, 2300, // EXP: 10, 11 >> > + 2800, 2800, // EXP: 12, 13 >> > + 2400, 2400, // EXP: 14, 15 >> > + 2300, 2300, // EXP: 16, 17 >> > + 2048, 2048, // EXP: 18, 19 >> > + 2048, 2048, // EXP: 20, 21 >> > + 2048, 2048, // EXP: 22, 23 >> > + 2048, 2048, // EXP: 24, 25 >> > + 2048, 2048, // EXP: 26, 27 >> > + 2048, 2048, // EXP: 28, 29 >> > + 2048, 2048, // EXP: 30, 31 >> > + 2048, 2048, // EXP: 32, 33 >> > + 1900, // MANT: 0 >> > + 1850, // MANT: 1 >> > + 1800, // MANT: 2 >> > + 1750, // MANT: 3 >> > + 1650, // MANT: 4 >> > + 1600, // MANT: 5 >> > + 1600, // MANT: 6 >> > + 2048, // MANT: 7 >> > + 2048, // MANT: 8 >> > + 2048, // MANT: 9 >> > + 2048, // MANT: 10 >> > + 2048, // MANT: 11 >> > + 2048, // MANT: 12 >> > + 2048, // MANT: 13 >> > + 2048, // MANT: 14 >> > + 2048, // MANT: 15 >> > + 2048, // MANT: 16 >> > + 2048 // MANT: 17 >> > +}; >> > + >> > +#define NZ_INT_ZERO (0) >> > +#define NZ_INT_SIGN (1) >> > +#define NZ_INT_EXP(k) ((2 + (k))) >> > +#define NZ_INT_MANT(k) ((36 + (k))) >> > + >> > + >> > +typedef struct FLIF16MultiscaleChanceContext { >> > + FLIF16MultiscaleChance data[sizeof(flif16_nz_int_chances) / >> sizeof(flif16_nz_int_chances[0])]; >> > +} FLIF16MultiscaleChanceContext; >> > + >> > +// Maybe rename to symbol context >> > +typedef struct FLIF16ChanceContext { >> >> > + uint16_t data[sizeof(flif16_nz_int_chances) / >> sizeof(flif16_nz_int_chances[0])]; >> >> FF_ARRAY_ELEMS() >> >> > +} FLIF16ChanceContext; >> > + >> > +typedef struct FLIF16RangeCoder { >> > + uint_fast32_t range; >> > + uint_fast32_t low; >> > + uint16_t chance; >> > + uint8_t active; ///< Is an integer reader currently active (to >> save/ >> > + /// transfer state) >> > + >> > + // uni_int state management >> > + uint32_t min; >> > + uint32_t len; >> > + >> > + // nz_int state management >> > + uint8_t segment; ///< The "segment" the function currently is in >> > + uint8_t sign; >> > + int amin, amax, emax, e, have, left, minabs1, maxabs0, pos; >> > + >> > + // maniac_int state management >> > + uint8_t segment2; >> > + int oldmin, oldmax; >> > + >> > + #ifdef MULTISCALE_CHANCES_ENABLED >> > + FLIF16MultiscaleChanceContext *maniac_ctx; >> > + #else >> > + FLIF16ChanceContext *maniac_ctx; >> > + #endif >> > + >> > + FLIF16ChanceTable ct; >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > + FLIF16MultiscaleChanceTable *mct; >> > + FLIF16Log4kTable log4k; >> > +#endif >> > + GetByteContext *gb; >> > +} FLIF16RangeCoder; >> > + >> > +/** >> > + * The Stack used to construct the MANIAC tree >> > + */ >> > +typedef struct FLIF16MANIACStack { >> > + unsigned int id; >> > + int p; >> > + int min; >> > + int max; >> > + int max2; >> > + uint8_t mode; >> > + uint8_t visited; >> > +} FLIF16MANIACStack; >> > + >> > +typedef struct FLIF16MANIACNode { >> > + int32_t property; >> > + int32_t count; >> > + int32_t split_val; >> > + int32_t child_id; >> > + int32_t leaf_id; >> > +} FLIF16MANIACNode; >> > + >> > +typedef struct FLIF16MANIACTree { >> > + FLIF16MANIACNode *data; >> >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > + FLIF16MultiscaleChanceContext *leaves; >> > +#else >> > + FLIF16ChanceContext *leaves; >> > +#endif >> >> You could avoid these multiple ifdef with a single conditional typedef. >> >> > + unsigned int size; >> > + unsigned int leaves_size; >> > + unsigned int leaves_top; >> > +} FLIF16MANIACTree; >> > + >> > +typedef struct FLIF16MANIACContext { >> > + FLIF16MANIACTree **forest; >> > + FLIF16MANIACStack *stack; >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > + FLIF16MultiscaleChanceContext ctx[3]; >> > +#else >> > + FLIF16ChanceContext ctx[3]; >> > +#endif >> > + unsigned int tree_top; >> > + unsigned int stack_top; >> > + unsigned int stack_size; >> > +} FLIF16MANIACContext; >> > + >> > +void ff_flif16_rac_init(FLIF16RangeCoder *rc, GetByteContext *gb, >> uint8_t *buf, >> > + uint8_t buf_size); >> > + >> > +void ff_flif16_rac_free(FLIF16RangeCoder *rc); >> > + >> > +void ff_flif16_chancecontext_init(FLIF16ChanceContext *ctx); >> > + >> > +void ff_flif16_chancetable_init(FLIF16ChanceTable *ct, int alpha, int >> cut); >> > + >> > +void ff_flif16_build_log4k_table(FLIF16Log4kTable *log4k); >> > + >> > +int ff_flif16_read_maniac_tree(FLIF16RangeCoder *rc, >> > + FLIF16MANIACContext *m, >> > + int32_t (*prop_ranges)[2], >> > + unsigned int prop_ranges_size, >> > + unsigned int channel); >> > + >> > +void ff_flif16_maniac_close(FLIF16MANIACContext *m, uint8_t >> num_planes); >> > + >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > + >> > +void >> ff_flif16_multiscale_chancecontext_init(FLIF16MultiscaleChanceContext *ctx); >> > + >> > +FLIF16MultiscaleChanceTable >> *ff_flif16_multiscale_chancetable_init(void); >> > + >> > +FLIF16MultiscaleChanceContext >> *ff_flif16_maniac_findleaf(FLIF16MANIACContext *m, >> > + uint8_t >> channel, >> > + int32_t >> *properties); >> > +#else >> > +FLIF16ChanceContext *ff_flif16_maniac_findleaf(FLIF16MANIACContext *m, >> > + uint8_t channel, >> > + int32_t *properties); >> > +#endif >> > + >> > +int ff_flif16_maniac_read_int(FLIF16RangeCoder *rc, >> > + FLIF16MANIACContext *m, >> > + int32_t *properties, >> > + uint8_t channel, >> > + int min, int max, int *target); >> > + >> > +#define MANIAC_GET(rc, m, prop, channel, min, max, target) \ >> > + if (!ff_flif16_maniac_read_int((rc), (m), (prop), (channel), >> (min), (max), (target))) {\ >> > + goto need_more_data; \ >> > + } >> > + >> > +// Functions >> > + >> > +static inline int ff_flif16_rac_renorm(FLIF16RangeCoder *rc) >> > +{ >> > + uint32_t left; >> > + while (rc->range <= FLIF16_RAC_MIN_RANGE) { >> > + left = bytestream2_get_bytes_left(rc->gb); >> > + if (!left) { >> > + return 0; >> > + } >> > + rc->low <<= 8; >> > + rc->range <<= 8; >> > + rc->low |= bytestream2_get_byte(rc->gb); >> > + if(!left) { >> > + return 0; >> > + } else { >> > + --left; >> > + } >> > + } >> > + return 1; >> > +} >> > + >> > +static inline uint8_t ff_flif16_rac_get(FLIF16RangeCoder *rc, uint32_t >> chance, >> > + uint8_t *target) >> > +{ >> > + if (rc->low >= rc->range - chance) { >> > + rc->low -= rc->range - chance; >> > + rc->range = chance; >> > + *target = 1; >> > + } else { >> > + rc->range -= chance; >> > + *target = 0; >> > + } >> > + >> > + return 1; >> > +} >> > + >> > +static inline uint8_t ff_flif16_rac_read_bit(FLIF16RangeCoder *rc, >> > + uint8_t *target) >> > +{ >> > + return ff_flif16_rac_get(rc, rc->range >> 1, target); >> > +} >> > + >> > +static inline uint32_t ff_flif16_rac_read_chance(FLIF16RangeCoder *rc, >> > + uint16_t b12, uint8_t >> *target) >> > +{ >> > + uint32_t ret; >> > + >> >> > + if (sizeof(rc->range) > 4) >> > + ret = ((rc->range) * b12 + 0x800) >> 12; >> > + else >> > + ret = (((((rc->range) & 0xFFF) * b12 + 0x800) >> 12) + >> > + (((rc->range) >> 12) * b12)); >> >> Cast b12 to uint64_t and let the compiler optimize this. >> >> > + >> > + return ff_flif16_rac_get(rc, ret, target); >> > +} >> > + >> > +/** >> > + * Reads a Uniform Symbol Coded Integer. >> > + */ >> > +static inline int ff_flif16_rac_read_uni_int(FLIF16RangeCoder *rc, >> > + uint32_t min, uint32_t >> len, >> > + int type, >> > + void *target) >> > +{ >> > + int med; >> > + uint8_t bit; >> > + >> > + if (!rc->active) { >> > + rc->min = min; >> > + rc->len = len; >> > + rc->active = 1; >> > + } >> > + >> > + if ((rc->len) > 0) { >> > + ff_flif16_rac_read_bit(rc, &bit); >> > + med = (rc->len) / 2; >> > + if (bit) { >> > + rc->min += med + 1; >> > + rc->len -= med + 1; >> > + } else { >> > + rc->len = med; >> > + } >> > + return 0; >> > + } else { >> > + switch (type) { >> > + case FLIF16_RAC_UNI_INT8: >> > + *((uint8_t *) target) = rc->min; >> > + break; >> > + >> > + case FLIF16_RAC_UNI_INT16: >> > + *((uint16_t *) target) = rc->min; >> > + break; >> > + >> > + case FLIF16_RAC_UNI_INT32: >> > + *((uint32_t *) target) = rc->min; >> > + break; >> > + } >> > + rc->active = 0; >> > + return 1; >> > + } >> > +} >> > + >> > +// Nearzero integer definitions >> > + >> > +static inline void ff_flif16_chancetable_put(FLIF16RangeCoder *rc, >> > + FLIF16ChanceContext *ctx, >> > + uint16_t type, uint8_t >> bit) >> > +{ >> > + ctx->data[type] = (!bit) ? rc->ct.zero_state[ctx->data[type]] >> > + : rc->ct.one_state[ctx->data[type]]; >> > +} >> > + >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > +static inline void ff_flif16_chance_estim(FLIF16RangeCoder *rc, >> > + uint16_t chance, uint8_t bit, >> > + uint64_t *total) >> > +{ >> > + *total += rc->log4k.table[bit ? chance : 4096 - chance]; >> > +} >> > +#endif >> > + >> > +/** >> > + * Reads a near-zero encoded symbol into the RAC probability >> model/chance table >> > + * @param type The symbol chance specified by the NZ_INT_* macros >> > + */ >> > +// TODO remove return value >> > +static inline uint8_t ff_flif16_rac_read_symbol(FLIF16RangeCoder *rc, >> > + FLIF16ChanceContext >> *ctx, >> > + uint16_t type, >> > + uint8_t *target) >> > +{ >> > + ff_flif16_rac_read_chance(rc, ctx->data[type], target); >> > + ff_flif16_chancetable_put(rc, ctx, type, *target); >> > + return 1; >> > +} >> > + >> > +// NearZero Integer Coder >> > + >> > +static inline int ff_flif16_rac_nz_read_internal(FLIF16RangeCoder *rc, >> > + FLIF16ChanceContext >> *ctx, >> > + uint16_t type, >> uint8_t *target) >> > +{ >> > + int flag = 0; >> > + while (!flag) { >> > + if(!ff_flif16_rac_renorm(rc)) >> > + return 0; // EAGAIN condition >> > + flag = ff_flif16_rac_read_symbol(rc, ctx, type, target); >> > + } >> > + return 1; >> > +} >> > + >> > +#define RAC_NZ_GET(rc, ctx, chance, target) >> \ >> > + if (!ff_flif16_rac_nz_read_internal((rc), (ctx), (chance), >> \ >> > + (uint8_t *) (target))) { >> \ >> > + goto need_more_data; >> \ >> > + } >> > + >> >> > +static inline int ff_flif16_rac_read_nz_int(FLIF16RangeCoder *rc, >> > + FLIF16ChanceContext *ctx, >> > + int min, int max, int >> *target) >> >> I am worried about the size of all these inline functions that call each >> other multiple times, growing exponentially. Bigger code will stress the >> cache more and make everything slower. Better make them normal >> functions. >> >> > +{ >> > + uint8_t temp = 0; >> > + if (min == max) { >> > + *target = min; >> > + rc->active = 0; >> > + return 1; >> > + } >> > + >> > + if (!rc->active) { >> > + rc->segment = 0; >> > + rc->amin = 1; >> > + rc->active = 1; >> > + rc->sign = 0; >> > + rc->have = 0; >> > + } >> > + >> >> > + switch (rc->segment) { >> > + case 0: >> >> Nit: indentation. >> >> > + RAC_NZ_GET(rc, ctx, NZ_INT_ZERO, &(temp)); >> > + if (temp) { >> > + *target = 0; >> > + rc->active = 0; >> > + return 1; >> > + } >> > + ++rc->segment; >> > + >> > + case 1: >> > + if (min < 0) { >> > + if (max > 0) { >> > + RAC_NZ_GET(rc, ctx, NZ_INT_SIGN, &(rc->sign)); >> > + } else { >> > + rc->sign = 0; >> > + } >> > + } else { >> > + rc->sign = 1; >> > + } >> > + rc->amax = (rc->sign ? max : -min); >> > + rc->emax = ff_log2(rc->amax); >> > + rc->e = ff_log2(rc->amin); >> > + ++rc->segment; >> > + >> > + case 2: >> > + for (; (rc->e) < (rc->emax); (rc->e++)) { >> > + RAC_NZ_GET(rc, ctx, NZ_INT_EXP((((rc->e) << 1) + >> rc->sign)), >> > + &(temp)); >> > + if (temp) >> > + break; >> > + temp = 0; >> > + } >> > + rc->have = (1 << (rc->e)); >> > + rc->left = rc->have - 1; >> > + rc->pos = rc->e; >> > + ++rc->segment; >> > + >> > + /* >> > + case 3 and case 4 mimic a for loop. >> > + This is done to separate the RAC read statement. >> > + for(pos = e; pos > 0; --pos) ... >> > + TODO replace entirely with an actual for loop. >> > + */ >> > + case 3: >> >> > + loop: /* start for */ >> >> Avoid goto, make it a real loop. >> >> > + if ((rc->pos) <= 0) >> > + goto end; >> > + --(rc->pos); >> > + rc->left >>= 1; >> > + rc->minabs1 = (rc->have) | (1 << (rc->pos)); >> > + rc->maxabs0 = (rc->have) | (rc->left); >> > + ++rc->segment; >> > + >> > + case 4: >> > + if ((rc->minabs1) > (rc->amax)) { >> > + --rc->segment; >> > + goto loop; /* continue; */ >> > + } else if ((rc->maxabs0) >= (rc->amin)) { >> > + RAC_NZ_GET(rc, ctx, NZ_INT_MANT(rc->pos), &temp); >> > + if (temp) >> > + rc->have = rc->minabs1; >> > + temp = 0; >> > + } else >> > + rc->have = rc->minabs1; >> > + --rc->segment; >> > + goto loop; /* end for */ >> > + } >> > + >> > + end: >> > + *target = ((rc->sign) ? (rc->have) : -(rc->have)); >> > + rc->active = 0; >> > + return 1; >> > + >> > + need_more_data: >> > + return 0; >> > +} >> > + >> > +static inline int ff_flif16_rac_read_gnz_int(FLIF16RangeCoder *rc, >> > + FLIF16ChanceContext *ctx, >> > + int min, int max, int >> *target) >> > +{ >> > + int ret; >> > + if (min > 0) { >> > + ret = ff_flif16_rac_read_nz_int(rc, ctx, 0, max - min, target); >> > + if (ret) >> > + *target += min; >> > + } else if (max < 0) { >> > + ret = ff_flif16_rac_read_nz_int(rc, ctx, min - max, 0, >> target); >> > + if (ret) >> > + *target += max; >> > + } else >> > + ret = ff_flif16_rac_read_nz_int(rc, ctx, min, max, target); >> > + return ret; >> > + >> > +} >> > + >> > +#ifdef MULTISCALE_CHANCES_ENABLED >> > +// Multiscale chance definitions >> > + >> > +static inline void >> ff_flif16_multiscale_chance_set(FLIF16MultiscaleChance *c, >> > + uint16_t chance) >> > +{ >> > + for (int i = 0; i < MULTISCALE_CHANCETABLE_DEFAULT_SIZE; i++) { >> > + c->chances[i] = chance; >> > + c->quality[i] = 0; >> > + } >> > + c->best = 0; >> > +} >> > + >> >> > +static inline uint16_t >> ff_flif16_multiscale_chance_get(FLIF16MultiscaleChance c) >> > +{ >> > + return c.chances[c.best]; >> > +} >> >> This does not look very useful. >> >> > + >> > +static inline void >> ff_flif16_multiscale_chancetable_put(FLIF16RangeCoder *rc, >> > + >> FLIF16MultiscaleChanceContext *ctx, >> > + uint16_t type, >> uint8_t bit) >> > +{ >> > + FLIF16MultiscaleChance *c = &ctx->data[type]; >> > + uint64_t sbits, oqual; >> > + for (int i = 0; i < MULTISCALE_CHANCETABLE_DEFAULT_SIZE; ++i) { >> > + sbits = 0; >> > + ff_flif16_chance_estim(rc, c->chances[i], bit, &sbits); >> > + oqual = c->quality[i]; >> > + c->quality[i] = (oqual * 255 + sbits * 4097 + 128) >> 8; >> > + c->chances[i] = (bit) ? >> rc->mct->sub_table[i].one_state[c->chances[i]] >> > + : >> rc->mct->sub_table[i].zero_state[c->chances[i]]; >> > + } >> > + for (int i = 0; i < MULTISCALE_CHANCETABLE_DEFAULT_SIZE; ++i) >> > + if (c->quality[i] < c->quality[c->best]) >> > + c->best = i; >> > +} >> > + >> > +static inline int >> ff_flif16_rac_read_multiscale_symbol(FLIF16RangeCoder *rc, >> > + >> FLIF16MultiscaleChanceContext *ctx, >> > + uint16_t type, >> uint8_t *target) >> > +{ >> > + ff_flif16_rac_read_chance(rc, >> ff_flif16_multiscale_chance_get(ctx->data[type]), target); >> > + ff_flif16_multiscale_chancetable_put(rc, ctx, type, *target); >> > + return 1; >> > +} >> > + >> > +static inline int >> ff_flif16_rac_nz_read_multiscale_internal(FLIF16RangeCoder *rc, >> > + >> FLIF16MultiscaleChanceContext *ctx, >> > + uint16_t >> type, uint8_t *target) >> > +{ >> > + int flag = 0; >> > + // Maybe remove the while loop >> > + while (!flag) { >> > + if(!ff_flif16_rac_renorm(rc)) >> > + return 0; // EAGAIN condition >> > + flag = ff_flif16_rac_read_multiscale_symbol(rc, ctx, type, >> target); >> > + } >> > + return 1; >> > +} >> > + >> > +#define RAC_NZ_MULTISCALE_GET(rc, ctx, chance, target) >> \ >> > + if (!ff_flif16_rac_nz_read_multiscale_internal((rc), (ctx), >> (chance), \ >> > + (uint8_t *) >> (target))) { \ >> > + goto need_more_data; >> \ >> > + } >> > + >> > +static inline int >> ff_flif16_rac_read_nz_multiscale_int(FLIF16RangeCoder *rc, >> > + >> FLIF16MultiscaleChanceContext *ctx, >> > + int min, int >> max, int *target) >> > +{ >> > + int temp = 0; >> > + >> > + if (min == max) { >> > + *target = min; >> > + goto end; >> > + } >> > + >> > + if (!rc->active) { >> > + rc->segment = 0; >> > + rc->amin = 1; >> > + rc->active = 1; >> > + rc->sign = 0; >> > + rc->have = 0; >> > + } >> > + >> >> > + switch (rc->segment) { >> > + case 0: >> >> Nit: indentation. >> >> > + RAC_NZ_MULTISCALE_GET(rc, ctx, NZ_INT_ZERO, &(temp)); >> > + if (temp) { >> > + *target = 0; >> > + goto end; >> > + } >> >> > + ++rc->segment;__PLN__ >> >> What is this __PLN__? >> >> > + >> > + case 1: >> > + if (min < 0) { >> > + if (max > 0) { >> > + > > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
