On 09/14/2013 03:46 AM, Kostya Shishkov wrote: > On Fri, Sep 13, 2013 at 04:31:00PM -0400, Justin Ruggles wrote: >> --- >> doc/general.texi | 2 +- >> libavcodec/webp.c | 1173 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 1164 insertions(+), 11 deletions(-) > > still no Changelog entry?
Will fix. >> diff --git a/doc/general.texi b/doc/general.texi >> index 18e425a..aeda8ea 100644 >> --- a/doc/general.texi >> +++ b/doc/general.texi >> @@ -413,7 +413,7 @@ following image formats are supported: >> @item Truevision Targa @tab X @tab X >> @tab Targa (.TGA) image format >> @item WebP @tab @tab X >> - @tab WebP image format (lossy only) >> + @tab WebP image format >> @item XBM @tab X @tab >> @tab X BitMap image format >> @item XWD @tab X @tab X >> diff --git a/libavcodec/webp.c b/libavcodec/webp.c >> index 33b242f..04313b4 100644 >> --- a/libavcodec/webp.c >> +++ b/libavcodec/webp.c >> @@ -1,6 +1,7 @@ >> /* >> * WebP (.webp) image decoder >> * Copyright (c) 2013 Aneesh Dogra <[email protected]> >> + * Copyright (c) 2013 Justin Ruggles <[email protected]> >> * >> * This file is part of Libav. >> * >> @@ -19,9 +20,11 @@ >> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 >> USA >> */ >> >> +#define BITSTREAM_READER_LE >> #include "avcodec.h" >> #include "bytestream.h" >> #include "internal.h" >> +#include "get_bits.h" >> #include "vp8.h" >> >> #define VP8X_FLAG_ANIMATION 0x2 >> @@ -30,33 +33,1164 @@ >> #define VP8X_FLAG_ALPHA 0x10 >> #define VP8X_FLAG_ICC 0x20 >> >> +#define MAX_PALETTE_SIZE 256 >> +#define MAX_CACHE_BITS 11 >> +#define NUM_CODE_LENGTH_CODES 19 >> +#define HUFFMAN_CODES_PER_META_CODE 5 >> +#define NUM_LITERAL_CODES 256 >> +#define NUM_LENGTH_CODES 24 >> +#define NUM_DISTANCE_CODES 40 >> +#define NUM_SHORT_DISTANCES 120 >> +#define MAX_HUFFMAN_CODE_LENGTH 15 >> + >> +static const uint16_t alphabet_sizes[HUFFMAN_CODES_PER_META_CODE] = { >> + NUM_LITERAL_CODES + NUM_LENGTH_CODES, >> + NUM_LITERAL_CODES, NUM_LITERAL_CODES, NUM_LITERAL_CODES, >> + NUM_DISTANCE_CODES >> +}; >> + >> +static const uint8_t code_length_code_order[NUM_CODE_LENGTH_CODES] = { >> + 17, 18, 0, 1, 2, 3, 4, 5, 16, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 >> +}; >> + >> +static const int8_t lz77_distance_offsets[NUM_SHORT_DISTANCES][2] = { >> + { 0, 1 }, { 1, 0 }, { 1, 1 }, { -1, 1 }, { 0, 2 }, { 2, 0 }, { 1, >> 2 }, { -1, 2 }, >> + { 2, 1 }, { -2, 1 }, { 2, 2 }, { -2, 2 }, { 0, 3 }, { 3, 0 }, { 1, >> 3 }, { -1, 3 }, >> + { 3, 1 }, { -3, 1 }, { 2, 3 }, { -2, 3 }, { 3, 2 }, { -3, 2 }, { 0, >> 4 }, { 4, 0 }, >> + { 1, 4 }, { -1, 4 }, { 4, 1 }, { -4, 1 }, { 3, 3 }, { -3, 3 }, { 2, >> 4 }, { -2, 4 }, >> + { 4, 2 }, { -4, 2 }, { 0, 5 }, { 3, 4 }, { -3, 4 }, { 4, 3 }, { -4, >> 3 }, { 5, 0 }, >> + { 1, 5 }, { -1, 5 }, { 5, 1 }, { -5, 1 }, { 2, 5 }, { -2, 5 }, { 5, >> 2 }, { -5, 2 }, >> + { 4, 4 }, { -4, 4 }, { 3, 5 }, { -3, 5 }, { 5, 3 }, { -5, 3 }, { 0, >> 6 }, { 6, 0 }, >> + { 1, 6 }, { -1, 6 }, { 6, 1 }, { -6, 1 }, { 2, 6 }, { -2, 6 }, { 6, >> 2 }, { -6, 2 }, >> + { 4, 5 }, { -4, 5 }, { 5, 4 }, { -5, 4 }, { 3, 6 }, { -3, 6 }, { 6, >> 3 }, { -6, 3 }, >> + { 0, 7 }, { 7, 0 }, { 1, 7 }, { -1, 7 }, { 5, 5 }, { -5, 5 }, { 7, >> 1 }, { -7, 1 }, >> + { 4, 6 }, { -4, 6 }, { 6, 4 }, { -6, 4 }, { 2, 7 }, { -2, 7 }, { 7, >> 2 }, { -7, 2 }, >> + { 3, 7 }, { -3, 7 }, { 7, 3 }, { -7, 3 }, { 5, 6 }, { -5, 6 }, { 6, >> 5 }, { -6, 5 }, >> + { 8, 0 }, { 4, 7 }, { -4, 7 }, { 7, 4 }, { -7, 4 }, { 8, 1 }, { 8, >> 2 }, { 6, 6 }, >> + { -6, 6 }, { 8, 3 }, { 5, 7 }, { -5, 7 }, { 7, 5 }, { -7, 5 }, { 8, >> 4 }, { 6, 7 }, >> + { -6, 7 }, { 7, 6 }, { -7, 6 }, { 8, 5 }, { 7, 7 }, { -7, 7 }, { 8, >> 6 }, { 8, 7 } >> +}; >> + >> +enum TransformType { >> + PREDICTOR_TRANSFORM = 0, >> + COLOR_TRANSFORM = 1, >> + SUBTRACT_GREEN = 2, >> + COLOR_INDEXING_TRANSFORM = 3, >> +}; >> + >> +enum PredictionMode { >> + PRED_MODE_BLACK, >> + PRED_MODE_L, >> + PRED_MODE_T, >> + PRED_MODE_TR, >> + PRED_MODE_TL, >> + PRED_MODE_AVG_T_AVG_L_TR, >> + PRED_MODE_AVG_L_TL, >> + PRED_MODE_AVG_L_T, >> + PRED_MODE_AVG_TL_T, >> + PRED_MODE_AVG_T_TR, >> + PRED_MODE_AVG_AVG_L_TL_AVG_T_TR, >> + PRED_MODE_SELECT, >> + PRED_MODE_ADD_SUBTRACT_FULL, >> + PRED_MODE_ADD_SUBTRACT_HALF, >> +}; >> + >> +enum HuffmanIndex { >> + HUFF_IDX_GREEN = 0, >> + HUFF_IDX_RED = 1, >> + HUFF_IDX_BLUE = 2, >> + HUFF_IDX_ALPHA = 3, >> + HUFF_IDX_DIST = 4 >> +}; >> + >> +enum ImageRole { >> + /* Stores the actual pixels of the image. */ >> + IMAGE_ROLE_ARGB, >> + >> + /* Stores the meta Huffman codes. The red and green components of a >> pixel >> + define the meta Huffman code used in a particular block of the ARGB >> + image. */ >> + IMAGE_ROLE_ENTROPY, >> + >> + /* Stores the metadata for Predictor Transform. The green component of a >> + pixel defines which of the 14 predictors is used within a particular >> + block of the ARGB image. */ >> + IMAGE_ROLE_PREDICTOR, >> + >> + /* It is created by ColorTransformElement values for different blocks of >> + the image. Each ColorTransformElement is treated as a pixel whose >> alpha >> + component is 255, red component is cte.red_to_blue, green component >> is >> + cte.green_to_blue and blue component is cte.green_to_red. */ >> + IMAGE_ROLE_COLOR_TRANSFORM, >> + >> + /* Color indexing image: An array of of size color_table_size (up to 256 >> + ARGB values) storing the metadata for the Color Indexing Transform. >> This >> + is stored as an image of width color_table_size and height 1. */ >> + IMAGE_ROLE_COLOR_INDEXING, >> + >> + IMAGE_ROLE_NB, >> +}; > > WTF, simply WTF > Even with comments it's impossible to comprehend what is it for. > Was it borrowed from the original specification as is? Yes, that's from the spec. Basically the information for each of those is coded as an "entropy coded image". All are essentially coded the same way and are treated as an ARGB image (though not all color components are always used for real information). I do admit the terminology from the spec is not particularly good... I'll try to rewrite them in a less confusing way. >> + >> +typedef struct HuffReader { >> + VLC vlc; >> + int simple; > > I'd rather call it binary, not simple. That term is from the spec. And yeah, it's either no code (typically used for all 0's) or binary. >> + int nb_symbols; >> + uint16_t simple_symbols[2]; >> +} HuffReader; >> + >> +typedef struct HuffmanGroup { >> + HuffReader huffman_code[HUFFMAN_CODES_PER_META_CODE]; >> +} HuffmanGroup; >> + >> +typedef struct ImageContext { >> + enum ImageRole role; >> + AVFrame *frame; >> + int color_cache_bits; >> + uint8_t *color_cache; >> + int num_huffman_groups; >> + HuffmanGroup *huffman_groups; >> +} ImageContext; >> + >> +typedef union { >> + uint8_t u8; >> + int8_t s8; >> +} av_alias alias8; > > looks unneeded - we have such thing in lavu for sure Will someone please point this out? I obviously looked there first before putting it here and didn't find anything. >> typedef struct WebPContext { >> VP8Context v; >> + AVCodecContext *avctx; >> int initialized; >> + GetBitContext gb; >> GetByteContext alpha_gb; >> uint8_t has_alpha; >> int width; >> int height; >> + >> + int lossless; >> + ImageContext image[IMAGE_ROLE_NB]; >> + enum TransformType transforms[4]; >> + int nb_transforms; >> + int subtract_green; >> + int predictor_block_size; >> + int color_block_size; >> + int color_index_width_bits; >> + int reduced_width; >> + int use_entropy_image; >> + int entropy_size; >> } WebPContext; >> >> +static av_always_inline uint8_t *get_pixel(AVFrame *frame, int x, int y) >> +{ >> + if (x < 0 || y < 0) >> + return NULL; >> + if (x >= frame->width) { >> + x = 0; >> + y++; >> + } >> + if (y >= frame->height) >> + return NULL; >> + >> + return frame->data[0] + y * frame->linesize[0] + 4 * x; >> +} > > looks unneeded The part that is useful is selecting the first pixel on the next row when getting the top-right pixel from a pixel on the far right column. Plus it simplifies the code. The checks are probably unnecessary though. >> +static av_always_inline uint8_t add_comp(int a, int b) >> +{ >> + return (a + b) & 0xFF; >> +} > > looks unneeded (especially masking) and you seem to use it on uint8_t->uint8_t > anyway Ok, I'll go through and make sure everything still works without it. >> + >> +static av_always_inline uint8_t avg2_comp(int a, int b) >> +{ >> + return (a + b) / 2; >> +} >> + >> +static av_always_inline uint8_t clamp_add_subtract_full(int a, int b, int c) >> +{ >> + return av_clip_uint8(a + b - c); >> +} >> + >> +static av_always_inline uint8_t clamp_add_subtract_half(int a, int b, int c) >> +{ >> + int d = avg2_comp(a, b); >> + return av_clip_uint8(d + (d - c) / 2); >> +} > > nit: this one looks like it can be simplified a bit In what way? >> +static void image_ctx_free(ImageContext *img) >> +{ >> + int i, j; >> + >> + if (img->color_cache) >> + av_free(img->color_cache); > > av_free(NULL) works fine, there's no need to check indeed. >> + if (img->role != IMAGE_ROLE_ARGB) >> + av_frame_free(&img->frame); >> + if (img->huffman_groups && img->num_huffman_groups > 0) { >> + for (i = 0; i < img->num_huffman_groups; i++) { >> + for (j = 0; j < HUFFMAN_CODES_PER_META_CODE; j++) { >> + if (!img->huffman_groups[i].huffman_code[j].simple) >> + >> ff_free_vlc(&img->huffman_groups[i].huffman_code[j].vlc); >> + } >> + } >> + av_free(img->huffman_groups); > > it will leak with img->huffman_groups allocated but not used (hint: the second > part of condition is not needed) will fix. >> + } >> + memset(img, 0, sizeof(*img)); >> +} >> + >> + >> +/* Differs from get_vlc2() in the following ways: >> + * - codes are bit-reversed >> + * - assumes 8-bit table to make reversal simpler >> + * - assumes max depth of 2 since the max code length for WebP is 15 >> + */ >> +static av_always_inline int webp_get_vlc(GetBitContext *gb, VLC_TYPE >> (*table)[2]) >> +{ >> + int n, nb_bits; >> + unsigned int index; >> + int code; >> + >> + OPEN_READER(re, gb); >> + UPDATE_CACHE(re, gb); >> + >> + index = SHOW_UBITS(re, gb, 8); >> + index = ff_reverse[index]; >> + code = table[index][0]; >> + n = table[index][1]; >> + >> + if (n < 0) { >> + LAST_SKIP_BITS(re, gb, 8); >> + UPDATE_CACHE(re, gb); >> + >> + nb_bits = -n; >> + >> + index = SHOW_UBITS(re, gb, nb_bits); >> + index = (ff_reverse[index] >> (8 - nb_bits)) + code; >> + code = table[index][0]; >> + n = table[index][1]; >> + } >> + SKIP_BITS(re, gb, n); >> + >> + CLOSE_READER(re, gb); >> + >> + return code; >> +} > > is it possible just to swap/reverse the whole bitstream or there are reasons > preventing this (e.g. mix of bit fields in normal order and those VLCs)? That is exactly the case. It is a mix. >> + >> +static int huff_reader_get_symbol(HuffReader *r, GetBitContext *gb) >> +{ >> + if (!r->nb_symbols) >> + return AVERROR_BUG; >> + >> + if (r->simple) { >> + if (r->nb_symbols == 1) { >> + return r->simple_symbols[0]; >> + } else { >> + return r->simple_symbols[get_bits1(gb)]; >> + } >> + } else { >> + return webp_get_vlc(gb, r->vlc.table); >> + } >> +} >> + >> +static int huff_reader_build_canonical(HuffReader *r, int *code_lengths, >> + int alphabet_size) >> +{ >> + int len, sym, code; >> + int max_code_length = 0; >> + uint16_t *codes; >> + >> + for (sym = 0; sym < alphabet_size; sym++) >> + max_code_length = FFMAX(max_code_length, code_lengths[sym]); >> + >> + if (max_code_length == 0 || max_code_length > MAX_HUFFMAN_CODE_LENGTH) >> + return AVERROR(EINVAL); >> + >> + codes = av_malloc(alphabet_size * sizeof(*codes)); >> + if (!codes) >> + return AVERROR(ENOMEM); >> + >> + code = 0; >> + r->nb_symbols = 0; >> + for (len = 1; len <= max_code_length; len++) { >> + for (sym = 0; sym < alphabet_size; sym++) { >> + if (code_lengths[sym] != len) >> + continue; >> + codes[sym] = code++; >> + r->nb_symbols++; >> + } >> + code <<= 1; >> + } >> + >> + init_vlc(&r->vlc, 8, alphabet_size, >> + code_lengths, sizeof(*code_lengths), sizeof(*code_lengths), >> + codes, sizeof(*codes), sizeof(*codes), 0); > > it can fail (e.g. no memory for the VLC tables) oops... >> + r->simple = 0; >> + >> + av_free(codes); >> + return 0; >> +} >> + >> +static void read_huffman_code_simple(WebPContext *s, HuffReader *hc) >> +{ >> + hc->nb_symbols = get_bits1(&s->gb) + 1; >> + >> + if (get_bits1(&s->gb)) >> + hc->simple_symbols[0] = get_bits(&s->gb, 8); >> + else >> + hc->simple_symbols[0] = get_bits1(&s->gb); >> + >> + if (hc->nb_symbols == 2) { >> + hc->simple_symbols[1] = get_bits(&s->gb, 8); >> + } >> + hc->simple = 1; >> +} >> + >> +static int read_huffman_code_normal(WebPContext *s, HuffReader *hc, >> + int alphabet_size) >> +{ >> + HuffReader code_len_hc; >> + int *code_lengths = NULL; >> + int code_length_code_lengths[NUM_CODE_LENGTH_CODES] = { 0 }; >> + int i, symbol, max_symbol, prev_code_len, ret = 0; >> + int num_codes = 4 + get_bits(&s->gb, 4); >> + >> + if (num_codes > NUM_CODE_LENGTH_CODES) >> + return AVERROR_INVALIDDATA; >> + >> + for (i = 0; i < num_codes; i++) >> + code_length_code_lengths[code_length_code_order[i]] = >> get_bits(&s->gb, 3); >> + >> + memset(&code_len_hc, 0, sizeof(code_len_hc)); >> + ret = huff_reader_build_canonical(&code_len_hc, >> code_length_code_lengths, >> + NUM_CODE_LENGTH_CODES); >> + if (ret < 0) >> + goto finish; >> + >> + code_lengths = av_mallocz_array(alphabet_size, sizeof(*code_lengths)); >> + if (!code_lengths) { >> + ret = AVERROR(ENOMEM); >> + goto finish; >> + } >> + >> + if (get_bits1(&s->gb)) { >> + int bits = 2 + 2 * get_bits(&s->gb, 3); >> + max_symbol = 2 + get_bits(&s->gb, bits); >> + if (max_symbol > alphabet_size) { >> + av_log(s->avctx, AV_LOG_ERROR, "max symbol %d > alphabet size >> %d\n", >> + max_symbol, alphabet_size); >> + ret = AVERROR_INVALIDDATA; >> + goto finish; >> + } >> + } else { >> + max_symbol = alphabet_size; >> + } >> + >> + prev_code_len = 8; >> + symbol = 0; >> + while (symbol < alphabet_size) { >> + int code_len; >> + >> + if (!max_symbol--) >> + break; >> + code_len = huff_reader_get_symbol(&code_len_hc, &s->gb); >> + if (code_len < 16) { >> + /* Code length code [0..15] indicates literal code lengths. */ >> + code_lengths[symbol++] = code_len; >> + if (code_len) >> + prev_code_len = code_len; >> + } else { >> + int repeat = 0, length = 0; >> + if (code_len == 16) { >> + /* Code 16 repeats the previous non-zero value [3..6] times, >> + i.e., 3 + ReadBits(2) times. If code 16 is used before a >> + non-zero value has been emitted, a value of 8 is >> repeated. */ >> + repeat = 3 + get_bits(&s->gb, 2); >> + length = prev_code_len; >> + } else if (code_len == 17) { >> + /* Code 17 emits a streak of zeros [3..10], i.e., >> + 3 + ReadBits(3) times. */ >> + repeat = 3 + get_bits(&s->gb, 3); >> + } else if (code_len == 18) { >> + /* Code 18 emits a streak of zeros of length [11..138], >> i.e., >> + 11 + ReadBits(7) times. */ >> + repeat = 11 + get_bits(&s->gb, 7); >> + } >> + if (symbol + repeat > alphabet_size) { >> + av_log(s->avctx, AV_LOG_ERROR, >> + "invalid symbol %d + repeat %d > alphabet size %d\n", >> + symbol, repeat, alphabet_size); >> + ret = AVERROR_INVALIDDATA; >> + goto finish; >> + } >> + while (repeat-- > 0) >> + code_lengths[symbol++] = length; >> + } >> + } >> + >> + ret = huff_reader_build_canonical(hc, code_lengths, alphabet_size); >> + >> +finish: >> + ff_free_vlc(&code_len_hc.vlc); >> + av_free(code_lengths); >> + return ret; >> +} >> + >> +static int decode_entropy_coded_image(WebPContext *s, enum ImageRole role); >> + >> +static int decode_image(WebPContext *s, enum ImageRole role, int w, int h) >> +{ >> + ImageContext *img; >> + int ret; >> + >> + img = &s->image[role]; >> + img->role = role; >> + >> + img->frame = av_frame_alloc(); >> + if (!img->frame) >> + return AVERROR(ENOMEM); >> + > >> + img->frame->format = AV_PIX_FMT_ARGB; >> + img->frame->width = w; >> + img->frame->height = h; >> + >> + ret = av_frame_get_buffer(img->frame, 1); >> + if (ret < 0) { >> + av_frame_free(&img->frame); >> + return ret; >> + } >> + >> + ret = decode_entropy_coded_image(s, img->role); >> + if (ret < 0) { >> + av_frame_free(&img->frame); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +#define PARSE_BLOCK_SIZE(w, h) do { >> \ >> + block_bits = get_bits(&s->gb, 3) + 2; >> \ >> + block_size = 1 << block_bits; >> \ >> + blocks_w = FFALIGN((w), block_size) / block_size; >> \ >> + blocks_h = FFALIGN((h), block_size) / block_size; >> \ > > FFALIGN() >> block_bits? will fix >> +} while (0) >> + >> +static int decode_entropy_image(WebPContext *s, ImageContext *rgba_img) >> +{ >> + int ret, block_bits, block_size, width, blocks_w, blocks_h; >> + >> + width = s->width; >> + if (s->color_index_width_bits > 0) >> + width = s->reduced_width; >> + >> + PARSE_BLOCK_SIZE(width, s->height); >> + >> + ret = decode_image(s, IMAGE_ROLE_ENTROPY, blocks_w, blocks_h); >> + if (ret < 0) >> + return ret; > > Please select better names, because here it looked like a loop (decode_image > calls decode_entropy_coded_image and not decode_entropy_image). Yeah, the names suck a lot. It's partly because the terminology in the spec sucks a lot and I can't think of anything better. I should probably write up an overview at the top of the file that describes how this crazy format is structured. In this case, decode_image() is mainly a wrapper around decode_entropy_coded_image() that just sets up the AVFrame stuff, so I'll merge those two. >> + >> + s->use_entropy_image = 1; >> + s->entropy_size = block_size; >> + >> + /* the number of huffman groups is determined by the maximum group >> number >> + coded in the entropy image */ >> + { >> + ImageContext *img; >> + int x, y, max; >> + uint8_t *p; >> + >> + img = &s->image[IMAGE_ROLE_ENTROPY]; >> + max = 0; >> + for (y = 0; y < img->frame->height; y++) { >> + for (x = 0; x < img->frame->width; x++) { >> + p = get_pixel(img->frame, x, y); >> + max = FFMAX(max, p[2]); >> + } >> + } >> + rgba_img->num_huffman_groups = max + 1; >> + } > > why such block? no particular reason. I'll change it. >> + >> + return 0; >> +} >> + >> +static int parse_transform_predictor(WebPContext *s) >> +{ >> + int block_bits, block_size, blocks_w, blocks_h, ret; >> + >> + PARSE_BLOCK_SIZE(s->width, s->height); >> + >> + ret = decode_image(s, IMAGE_ROLE_PREDICTOR, blocks_w, blocks_h); >> + if (ret < 0) >> + return ret; >> + >> + s->predictor_block_size = block_size; >> + >> + return 0; >> +} >> + >> +static int parse_transform_color(WebPContext *s) >> +{ >> + int block_bits, block_size, blocks_w, blocks_h, ret; >> + >> + PARSE_BLOCK_SIZE(s->width, s->height); >> + >> + ret = decode_image(s, IMAGE_ROLE_COLOR_TRANSFORM, blocks_w, blocks_h); >> + if (ret < 0) >> + return ret; >> + >> + s->color_block_size = block_size; >> + >> + return 0; >> +} >> + >> +static int parse_transform_color_indexing(WebPContext *s) >> +{ >> + int width_bits, index_size, ret; >> + >> + index_size = get_bits(&s->gb, 8) + 1; >> + >> + if (index_size <= 2) >> + width_bits = 3; >> + else if (index_size <= 4) >> + width_bits = 2; >> + else if (index_size <= 16) >> + width_bits = 1; >> + else >> + width_bits = 0; >> + >> + ret = decode_image(s, IMAGE_ROLE_COLOR_INDEXING, index_size, 1); >> + if (ret < 0) >> + return ret; >> + >> + s->color_index_width_bits = width_bits; >> + if (width_bits > 0) >> + s->reduced_width = (s->width + ((1 << width_bits) - 1)) >> >> width_bits; >> + >> + /* color index values are delta-coded */ >> + { >> + ImageContext *img; >> + int x; >> + uint8_t *ct, *ct0 = NULL; >> + >> + img = &s->image[IMAGE_ROLE_COLOR_INDEXING]; >> + ct = img->frame->data[0]; >> + for (x = 0; x < img->frame->width; x++) { >> + if (ct0) { >> + ct[0] = add_comp(ct[0], ct0[0]); >> + ct[1] = add_comp(ct[1], ct0[1]); >> + ct[2] = add_comp(ct[2], ct0[2]); >> + ct[3] = add_comp(ct[3], ct0[3]); > > ct[0] += ct0[0] would be just as good or even better > >> + } >> + ct0 = ct; >> + ct += 4; > > or maybe even > > for (x = 0; x < width * 4; x++, ct++) > if (x >= 4) > ct[0] += ct[-4]; Cool. I'll give that a try. >> + } >> + } > > again an anonymous block i'll change that one too >> + >> + return 0; >> +} >> + >> +static HuffmanGroup *get_huffman_group(WebPContext *s, ImageContext *img, >> + int x, int y) >> +{ >> + ImageContext *gimg = &s->image[IMAGE_ROLE_ENTROPY]; >> + int group = 0; >> + uint8_t *p; >> + >> + if (s->use_entropy_image) { >> + int group_x, group_y; >> + >> + group_x = x / s->entropy_size; >> + group_y = y / s->entropy_size; >> + p = get_pixel(gimg->frame, group_x, group_y); >> + group = p[2]; >> + } >> + >> + return &img->huffman_groups[group]; >> +} >> + >> +/* PRED_MODE_BLACK */ >> +static void inv_predict_0(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + static const uint8_t black[4] = { 0xFF, 0x00, 0x00, 0x00 }; >> + >> + AV_COPY32(p, black); >> +} >> + >> +/* PRED_MODE_L */ >> +static void inv_predict_1(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + AV_COPY32(p, p_l); >> +} >> + >> +/* PRED_MODE_T */ >> +static void inv_predict_2(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + AV_COPY32(p, p_t); >> +} >> + >> +/* PRED_MODE_TR */ >> +static void inv_predict_3(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + AV_COPY32(p, p_tr); >> +} >> + >> +/* PRED_MODE_TL */ >> +static void inv_predict_4(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + AV_COPY32(p, p_tl); >> +} >> + >> +/* PRED_MODE_AVG_T_AVG_L_TR */ >> +static void inv_predict_5(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = avg2_comp(p_t[0], avg2_comp(p_l[0], p_tr[0])); >> + p[1] = avg2_comp(p_t[1], avg2_comp(p_l[1], p_tr[1])); >> + p[2] = avg2_comp(p_t[2], avg2_comp(p_l[2], p_tr[2])); >> + p[3] = avg2_comp(p_t[3], avg2_comp(p_l[3], p_tr[3])); >> +} >> + >> +/* PRED_MODE_AVG_L_TL */ >> +static void inv_predict_6(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = avg2_comp(p_l[0], p_tl[0]); >> + p[1] = avg2_comp(p_l[1], p_tl[1]); >> + p[2] = avg2_comp(p_l[2], p_tl[2]); >> + p[3] = avg2_comp(p_l[3], p_tl[3]); >> +} >> + >> +/* PRED_MODE_AVG_L_T */ >> +static void inv_predict_7(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = avg2_comp(p_l[0], p_t[0]); >> + p[1] = avg2_comp(p_l[1], p_t[1]); >> + p[2] = avg2_comp(p_l[2], p_t[2]); >> + p[3] = avg2_comp(p_l[3], p_t[3]); >> +} >> + >> +/* PRED_MODE_AVG_TL_T */ >> +static void inv_predict_8(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = avg2_comp(p_tl[0], p_t[0]); >> + p[1] = avg2_comp(p_tl[1], p_t[1]); >> + p[2] = avg2_comp(p_tl[2], p_t[2]); >> + p[3] = avg2_comp(p_tl[3], p_t[3]); >> +} >> + >> +/* PRED_MODE_AVG_T_TR */ >> +static void inv_predict_9(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = avg2_comp(p_t[0], p_tr[0]); >> + p[1] = avg2_comp(p_t[1], p_tr[1]); >> + p[2] = avg2_comp(p_t[2], p_tr[2]); >> + p[3] = avg2_comp(p_t[3], p_tr[3]); >> +} >> + >> +/* PRED_MODE_AVG_AVG_L_TL_AVG_T_TR */ >> +static void inv_predict_10(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = avg2_comp(avg2_comp(p_l[0], p_tl[0]), avg2_comp(p_t[0], >> p_tr[0])); >> + p[1] = avg2_comp(avg2_comp(p_l[1], p_tl[1]), avg2_comp(p_t[1], >> p_tr[1])); >> + p[2] = avg2_comp(avg2_comp(p_l[2], p_tl[2]), avg2_comp(p_t[2], >> p_tr[2])); >> + p[3] = avg2_comp(avg2_comp(p_l[3], p_tl[3]), avg2_comp(p_t[3], >> p_tr[3])); > > p_l[0] + p_tl[0] + p_t[0] + p_tr[0] >> 2 ? That's not the same due to rounding. Example: 2, 3, 7, 12 avg(2,3)=2; avg(7,12)=9; avg(2,9)=5 2 + 3 + 7 + 12 >> 2 = 6 > honestly, those avg2_comp() everywhere do not improve anything Ok, I can expand it all out if you think it's clearer. >> +} >> + >> +/* PRED_MODE_SELECT */ >> +static void inv_predict_11(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + int diff = (abs(p_l[0] - p_tl[0]) - abs(p_t[0] - p_tl[0])) + >> + (abs(p_l[1] - p_tl[1]) - abs(p_t[1] - p_tl[1])) + >> + (abs(p_l[2] - p_tl[2]) - abs(p_t[2] - p_tl[2])) + >> + (abs(p_l[3] - p_tl[3]) - abs(p_t[3] - p_tl[3])); >> + if (diff <= 0) >> + AV_COPY32(p, p_t); >> + else >> + AV_COPY32(p, p_l); >> +} >> + >> +/* PRED_MODE_ADD_SUBTRACT_FULL */ >> +static void inv_predict_12(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = clamp_add_subtract_full(p_l[0], p_t[0], p_tl[0]); >> + p[1] = clamp_add_subtract_full(p_l[1], p_t[1], p_tl[1]); >> + p[2] = clamp_add_subtract_full(p_l[2], p_t[2], p_tl[2]); >> + p[3] = clamp_add_subtract_full(p_l[3], p_t[3], p_tl[3]); >> +} >> + >> +/* PRED_MODE_ADD_SUBTRACT_HALF */ >> +static void inv_predict_13(uint8_t *p, const uint8_t *p_l, const uint8_t >> *p_tl, >> + const uint8_t *p_t, const uint8_t *p_tr) >> +{ >> + p[0] = clamp_add_subtract_half(p_l[0], p_t[0], p_tl[0]); >> + p[1] = clamp_add_subtract_half(p_l[1], p_t[1], p_tl[1]); >> + p[2] = clamp_add_subtract_half(p_l[2], p_t[2], p_tl[2]); >> + p[3] = clamp_add_subtract_half(p_l[3], p_t[3], p_tl[3]); >> +} >> + >> +typedef void (*inv_predict_func)(uint8_t *p, const uint8_t *p_l, >> + const uint8_t *p_tl, const uint8_t *p_t, >> + const uint8_t *p_tr); >> + >> +static const inv_predict_func inverse_predict[14] = { >> + inv_predict_0, inv_predict_1, inv_predict_2, inv_predict_3, >> + inv_predict_4, inv_predict_5, inv_predict_6, inv_predict_7, >> + inv_predict_8, inv_predict_9, inv_predict_10, inv_predict_11, >> + inv_predict_12, inv_predict_13, >> +}; >> + >> +static void inverse_prediction(AVFrame *frame, enum PredictionMode m, int >> x, int y) >> +{ >> + uint8_t *dec, *p_l, *p_tl, *p_t, *p_tr; >> + uint8_t p[4]; >> + >> + dec = get_pixel(frame, x, y); >> + p_l = get_pixel(frame, x - 1, y); >> + p_tl = get_pixel(frame, x - 1, y - 1); >> + p_t = get_pixel(frame, x, y - 1); >> + p_tr = get_pixel(frame, x + 1, y - 1); >> + >> + inverse_predict[m](p, p_l, p_tl, p_t, p_tr); >> + >> + dec[0] = add_comp(dec[0], p[0]); >> + dec[1] = add_comp(dec[1], p[1]); >> + dec[2] = add_comp(dec[2], p[2]); >> + dec[3] = add_comp(dec[3], p[3]); >> +} >> + >> +static int apply_predictor_transform(WebPContext *s) >> +{ >> + ImageContext *img = &s->image[IMAGE_ROLE_ARGB]; >> + ImageContext *pimg = &s->image[IMAGE_ROLE_PREDICTOR]; >> + int block_size = s->predictor_block_size; >> + int x, y; >> + >> + for (y = 0; y < img->frame->height; y++) { >> + for (x = 0; x < img->frame->width; x++) { >> + int tx = x / block_size; >> + int ty = y / block_size; >> + uint8_t *p = get_pixel(pimg->frame, tx, ty); >> + enum PredictionMode m = p[2]; >> + >> + if (x == 0) { >> + if (y == 0) >> + m = PRED_MODE_BLACK; >> + else >> + m = PRED_MODE_T; >> + } else if (y == 0) { >> + m = PRED_MODE_L; >> + } >> + >> + if (m > 13) { >> + av_log(s->avctx, AV_LOG_ERROR, "invalid predictor mode: >> %d\n", m); >> + return AVERROR_INVALIDDATA; >> + } >> + inverse_prediction(img->frame, m, x, y); >> + } >> + } >> + return 0; >> +} >> + >> +static av_always_inline int color_transform_delta(int color_pred, int color) >> +{ >> + return (color_pred * color) >> 5; >> +} >> + >> +static int apply_color_transform(WebPContext *s) >> +{ >> + ImageContext *img, *cimg; >> + int x, y, cx, cy; >> + uint8_t *p, *cp; >> + int block_size = s->color_block_size; >> + >> + img = &s->image[IMAGE_ROLE_ARGB]; >> + cimg = &s->image[IMAGE_ROLE_COLOR_TRANSFORM]; >> + >> + for (y = 0; y < img->frame->height; y++) { >> + for (x = 0; x < img->frame->width; x++) { >> + alias8 green_to_red, green_to_blue, red_to_blue; >> + alias8 green, red; >> + int blue; >> + >> + cx = x / block_size; >> + cy = y / block_size; >> + cp = get_pixel(cimg->frame, cx, cy); >> + red_to_blue.u8 = cp[1]; >> + green_to_blue.u8 = cp[2]; >> + green_to_red.u8 = cp[3]; >> + >> + p = get_pixel(img->frame, x, y); >> + red.u8 = p[1]; >> + green.u8 = p[2]; >> + blue = p[3]; >> + >> + red.u8 = add_comp(red.u8, >> color_transform_delta(green_to_red.s8, green.s8)); >> + blue += >> color_transform_delta(green_to_blue.s8, green.s8); >> + blue = add_comp(blue, color_transform_delta(red_to_blue.s8, >> red.s8)); > > hmm? Yeah, it's pretty weird. I'll see if I can break it down further to simplify it without breaking the math. >> + >> + p[1] = red.u8; >> + p[3] = blue; >> + } >> + } >> + return 0; >> +} >> + >> +static int apply_subtract_green_transform(WebPContext *s) >> +{ >> + int x, y; >> + ImageContext *img = &s->image[IMAGE_ROLE_ARGB]; >> + >> + for (y = 0; y < img->frame->height; y++) { >> + for (x = 0; x < img->frame->width; x++) { >> + uint8_t *p = get_pixel(img->frame, x, y); >> + p[1] = add_comp(p[1], p[2]); >> + p[3] = add_comp(p[3], p[2]); >> + } >> + } >> + return 0; >> +} >> + >> +static int apply_color_indexing_transform(WebPContext *s) >> +{ >> + ImageContext *img; >> + ImageContext *pal; >> + int i, x, y; >> + uint8_t *p, *pi; >> + int width_bits = s->color_index_width_bits; >> + >> + img = &s->image[IMAGE_ROLE_ARGB]; >> + pal = &s->image[IMAGE_ROLE_COLOR_INDEXING]; >> + >> + if (width_bits > 0) { >> + GetBitContext gb_g; >> + uint8_t *line; >> + int pixel_bits = 8 >> width_bits; >> + >> + line = av_malloc(img->frame->linesize[0]); >> + if (!line) >> + return AVERROR(ENOMEM); >> + >> + for (y = 0; y < img->frame->height; y++) { >> + p = get_pixel(img->frame, 0, y); >> + memcpy(line, p, img->frame->linesize[0]); >> + init_get_bits(&gb_g, line, img->frame->linesize[0] * 8); >> + skip_bits(&gb_g, 16); >> + i = 0; >> + for (x = 0; x < img->frame->width; x++) { >> + p = get_pixel(img->frame, x, y); >> + p[2] = get_bits(&gb_g, pixel_bits); >> + i++; >> + if (i == (1 << width_bits)) { >> + skip_bits(&gb_g, 24); >> + i = 0; >> + } >> + } >> + } >> + av_free(line); >> + } >> + >> + for (y = 0; y < img->frame->height; y++) { >> + for (x = 0; x < img->frame->width; x++) { >> + p = get_pixel(img->frame, x, y); >> + i = p[2]; >> + pi = get_pixel(pal->frame, i, 0); >> + if (!pi) { >> + av_log(s->avctx, AV_LOG_ERROR, "invalid palette index >> %d\n", i); >> + return AVERROR_INVALIDDATA; >> + } >> + AV_COPY32(p, pi); >> + } >> + } >> + >> + return 0; >> +} >> + >> +static av_always_inline int color_cache_put(ImageContext *img, uint32_t c) >> +{ >> + int cache_idx = (0x1E35A7BD * c) >> (32 - img->color_cache_bits); >> + if (cache_idx >= (1 << img->color_cache_bits)) >> + return AVERROR_BUG; > > use uint32_t here instead and throw out that silly check will do >> + AV_WB32(&img->color_cache[4 * cache_idx], c); >> + return 0; >> +} >> + >> +static int decode_entropy_coded_image(WebPContext *s, enum ImageRole role) >> +{ >> + ImageContext *img; >> + HuffmanGroup *hg; >> + int i, j, ret, x, y, width; >> + >> + img = &s->image[role]; >> + >> + if (role == IMAGE_ROLE_ARGB) { >> + s->subtract_green = 0; >> + s->nb_transforms = 0; >> + ret = 0; >> + while (get_bits1(&s->gb)) { >> + enum TransformType transform = get_bits(&s->gb, 2); >> + s->transforms[s->nb_transforms++] = transform; >> + switch (transform) { >> + case PREDICTOR_TRANSFORM: >> + ret = parse_transform_predictor(s); >> + if (ret < 0) >> + return ret; >> + break; >> + case COLOR_TRANSFORM: >> + ret = parse_transform_color(s); >> + if (ret < 0) >> + return ret; >> + break; >> + case SUBTRACT_GREEN: >> + s->subtract_green = 1; >> + break; >> + case COLOR_INDEXING_TRANSFORM: >> + ret = parse_transform_color_indexing(s); >> + if (ret < 0) >> + return ret; >> + break; >> + } >> + } >> + } >> + >> + if (get_bits1(&s->gb)) { >> + img->color_cache_bits = get_bits(&s->gb, 4); >> + if (img->color_cache_bits < 1 || img->color_cache_bits > 11) { >> + av_log(s->avctx, AV_LOG_ERROR, "invalid color cache bits: %d\n", >> + img->color_cache_bits); >> + return AVERROR_INVALIDDATA; >> + } >> + img->color_cache = av_mallocz_array(1 << img->color_cache_bits, 4); >> + if (!img->color_cache) >> + return AVERROR(ENOMEM); >> + } else { >> + img->color_cache_bits = 0; >> + } >> + >> + img->num_huffman_groups = 1; >> + s->use_entropy_image = 0; >> + if (role == IMAGE_ROLE_ARGB && get_bits1(&s->gb)) { >> + ret = decode_entropy_image(s, img); >> + if (ret < 0) >> + return ret; >> + } >> + img->huffman_groups = av_mallocz_array(img->num_huffman_groups, >> + sizeof(*img->huffman_groups)); >> + if (!img->huffman_groups) >> + return AVERROR(ENOMEM); >> + >> + for (i = 0; i < img->num_huffman_groups; i++) { >> + hg = &img->huffman_groups[i]; >> + for (j = 0; j < HUFFMAN_CODES_PER_META_CODE; j++) { >> + int alphabet_size = alphabet_sizes[j]; >> + if (!j && img->color_cache_bits > 0) >> + alphabet_size += 1 << img->color_cache_bits; >> + >> + if (get_bits1(&s->gb)) { >> + read_huffman_code_simple(s, &hg->huffman_code[j]); >> + } else { >> + ret = read_huffman_code_normal(s, &hg->huffman_code[j], >> + alphabet_size); >> + if (ret < 0) >> + return ret; >> + } >> + } >> + } >> + >> + width = img->frame->width; >> + if (role == IMAGE_ROLE_ARGB && s->color_index_width_bits > 0) >> + width = s->reduced_width; >> + >> + x = 0; y = 0; >> + while (y < img->frame->height) { >> + int v; >> + >> + hg = get_huffman_group(s, img, x, y); >> + v = huff_reader_get_symbol(&hg->huffman_code[HUFF_IDX_GREEN], >> &s->gb); >> + if (v < NUM_LITERAL_CODES) { >> + /* literal pixel values */ >> + uint8_t *p = get_pixel(img->frame, x, y); >> + p[2] = v; >> + p[1] = huff_reader_get_symbol(&hg->huffman_code[HUFF_IDX_RED], >> &s->gb); >> + p[3] = huff_reader_get_symbol(&hg->huffman_code[HUFF_IDX_BLUE], >> &s->gb); >> + p[0] = >> huff_reader_get_symbol(&hg->huffman_code[HUFF_IDX_ALPHA], &s->gb); >> + if (img->color_cache_bits) { >> + if ((ret = color_cache_put(img, AV_RB32(p))) < 0) >> + return ret; >> + } >> + x++; >> + if (x == width) { >> + x = 0; >> + y++; >> + } >> + } else if (v < NUM_LITERAL_CODES + NUM_LENGTH_CODES) { >> + /* LZ77 backwards mapping */ >> + int prefix_code, length, distance, ref_x, ref_y; >> + >> + /* parse length and distance */ >> + prefix_code = v - NUM_LITERAL_CODES; >> + if (prefix_code < 4) { >> + length = prefix_code + 1; >> + } else { >> + int extra_bits = (prefix_code - 2) >> 1; >> + int offset = (2 + (prefix_code & 1)) << extra_bits; >> + length = offset + get_bits(&s->gb, extra_bits) + 1; >> + } >> + prefix_code = >> huff_reader_get_symbol(&hg->huffman_code[HUFF_IDX_DIST], &s->gb); >> + if (prefix_code < 4) { >> + distance = prefix_code + 1; >> + } else { >> + int extra_bits = (prefix_code - 2) >> 1; >> + int offset = (2 + (prefix_code & 1)) << extra_bits; >> + distance = offset + get_bits(&s->gb, extra_bits) + 1; >> + } >> + >> + /* find reference location */ >> + if (distance <= NUM_SHORT_DISTANCES) { >> + int xi = lz77_distance_offsets[distance - 1][0]; >> + int yi = lz77_distance_offsets[distance - 1][1]; >> + distance = FFMAX(1, xi + yi * width); >> + } else { >> + distance -= NUM_SHORT_DISTANCES; >> + } >> + ref_x = x; >> + ref_y = y; >> + if (distance <= x) { >> + ref_x -= distance; >> + distance = 0; >> + } else { >> + ref_x = 0; >> + distance -= x; >> + } >> + while (distance >= width) { >> + ref_y--; >> + distance -= width; >> + } >> + if (distance > 0) { >> + ref_x = width - distance; >> + ref_y--; >> + } >> + ref_x = FFMAX(0, ref_x); >> + ref_y = FFMAX(0, ref_y); >> + >> + /* copy pixels */ >> + /* source and dest regions can overlap and wrap lines, so just >> + copy per-pixel */ >> + for (i = 0; i < length; i++) { >> + uint8_t *p_ref = get_pixel(img->frame, ref_x, ref_y); >> + uint8_t *p = get_pixel(img->frame, x, y); >> + > > you don't seem to check get_pixel() return value for NULL anywhere I'll change get_pixel() to not bother with doing unnecessary validation. Thanks for the review! -Justin _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
