There was a duplicated functionality with a lower quality which
could lead to decoded lossless WebP to YUV being different from
lossless WebP to PNG to YUV.
The rescaler is not using it yet.
Bug: 432241412
Change-Id: Id794880957935b69729d4b34ae453551d13364dc
The armv7, armv7s and i386 iOS targets were deprecated in Xcode 14 [1]
and failed to build with Xcode 16.4.
[1]: https://developer.apple.com/documentation/xcode-release-notes/xcode-14-release-notes
Building iOS projects with deployment targets for the armv7, armv7s, and
i386 architectures is no longer supported. (92831716)
Change-Id: I08d376fea64638d056258798bd7e586ca6880454
Reasoning:
Image Data Buffers:
The `data` parameter of `WebPDequantizeLevels` (in both .c and .h)
and `InitParams` (src/utils/quant_levels_dec_utils.c:232) is annotated
with `WEBP_SIZED_BY((long)stride * height)`, as it points to the start
of the image buffer.
The `src` and `dst` fields in `SmoothParams`
(src/utils/quant_levels_dec_utils.c:54) are annotated as
`WEBP_INDEXABLE`. They are initialized from `data` in `InitParams`
(L266) and are advanced row by row using pointer arithmetic (e.g.,
`p->src += p->stride` in `VFilter` L111, `p->dst += p->stride` in
`ApplyFilter` L165). `WEBP_INDEXABLE` is used because the pointers
iterate within the buffer and are only accessed with positive indices.
Scratch Buffers (`SmoothParams`):
Scratch buffers are allocated in `InitParams` via `WebPSafeMalloc`.
The local variable `mem` holding this allocation (L245) is explicitly
annotated as `WEBP_BIDI_INDEXABLE` to ensure safety when compiling with
error suppression.
- `start`, `cur`, `top`: These pointers are used for iteration and
pointer arithmetic within the circular scratch buffer. They are
annotated as `WEBP_INDEXABLE`.
- `end`: This pointer is annotated as `WEBP_BIDI_INDEXABLE` because it
is used in subtraction (`p->end - width`) in `InitParams` (L257) to
calculate `p->top`.
- `average`: This buffer is accessed sequentially up to `width`. It is
annotated as `WEBP_COUNTED_BY(width)`. Initialization in `InitParams`
is reordered (L261) to ensure `p->width` is set before `p->average`.
- `correction`: This lookup table requires negative indexing. To avoid
using `WEBP_BIDI_INDEXABLE` in the struct, it is annotated as
`WEBP_COUNTED_BY_OR_NULL(CORRECTION_LUT_SIZE)` (L75), pointing to the
start of the buffer. `CORRECTION_LUT_SIZE` is defined (L33).
`InitCorrectionLUT` (L188) and `ApplyFilter` (L147) calculate a local
middle pointer which is explicitly annotated as `WEBP_BIDI_INDEXABLE`
to allow safe negative indexing.
Local Pointers:
To ensure safety when compiling with error suppression (where locals
default to unsafe), explicit annotations are added to local pointers
derived from safe struct members:
- `VFilter` (L87): `src`, `cur`, `top`, `out` are `WEBP_INDEXABLE`.
- `HFilter` (L121): `in`, `out` are `WEBP_INDEXABLE`.
- `ApplyFilter` (L145): `average`, `dst` are `WEBP_INDEXABLE`.
- `CountLevels` (L214): `data` is `WEBP_INDEXABLE`.
Bug: 432511821
Change-Id: I6bdf86f80c94a5b182c5aef7e4092fe4ea24afb8
Reasoning:
In `HuffmanTablesSegment` (`src/utils/huffman_utils.h`), `start`
was annotated `WEBP_COUNTED_BY_OR_NULL(size)` as it points to an
allocation of `size` elements. `curr_table` was annotated
`WEBP_UNSAFE_INDEXABLE` because it iterates within `[start, start
+ size)`, a bound that cannot be expressed statically in the struct
without ABI changes. The code manually checks bounds for
`curr_table` (e.g., `src/utils/huffman_utils.c:240-241`). To
support the annotation on `start`, allocation sites in
`VP8LBuildHuffmanTable` and `VP8LHuffmanTablesAllocate`
(`src/utils/huffman_utils.c`) were refactored to assign `start` and
`size` side-by-side, using `WEBP_BIDI_INDEXABLE` local variables to
hold the safe pointer returned by `WebPSafeMalloc`.
`VP8LHuffmanTablesDeallocate` was updated to set `size` to 0 when
`start` is freed.
The `root_table` parameter of `BuildHuffmanTable`
(`src/utils/huffman_utils.c:86`) was annotated `WEBP_BIDI_INDEXABLE` to
accommodate accesses to secondary tables beyond the `root table` size
since with explicitly annotating the local variable `table`
as `WEBP_BIDI_INDEXABLE`, `table` inherits its bounds from `root_table`.
Call sites in `VP8LBuildHuffmanTable`
required `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` to convert the unsafe
`curr_table` to the safe `root_table`.
The `table` parameter of `ReplicateValue`
(`src/utils/huffman_utils.c:59`) was annotated
`WEBP_COUNTED_BY(end - step + 1)` and the function was refactored to
avoid modifying `end`. Call sites in `BuildHuffmanTable` required
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` because the strided access
patterns used for Huffman table construction cannot be statically
verified by the compiler.
Bug: 432511821
Change-Id: I77c5c82ac36bc9bb79cd5119a4113ac5d62af762
Memory footprint is increased by twice the canvas pixel count in bytes
at encoding. There should be little impact on encoding speed because
only buffer allocs/reads/writes are introduced, with little to no
added logic. Animation encoding may be 2% slower.
Bug: 42340478
Change-Id: I8f0048107a2bfbee7a8124c100f78eac93447d80
The calling function, `GetFilterMap()` only takes `width` and `height`;
the alpha data is assumed to have a stride equal to its width. The
`WebPEstimateBestFilter()` was inconsistently using the parameters,
setting up the current row with `stride`, but accessing the previous one
with `width`.
Change-Id: I9dd90222b6923eea3626e426a61bdef3985546ff
Reasoning:
The `irow` and `frow` pointers in `WebPRescaler`
(src/utils/rescaler_utils.h:49) were annotated with
`WEBP_COUNTED_BY(dst_width * num_channels)`. This is based on their
initialization in `WebPRescalerInit` (src/utils/rescaler_utils.c:82-83)
where they are assigned parts of the `work` buffer, whose total size
is `2 * dst_width * num_channels`. The `work` parameter in
`WebPRescalerInit` (src/utils/rescaler_utils.h:58,
src/utils/rescaler_utils.c:33) was also annotated accordingly.
To satisfy the side-by-side assignment requirement for external bounds,
assignments to `rescaler->irow` and `rescaler->frow` in
`WebPRescalerInit` were moved closer to the assignments of
`dst_width` and `num_channels` (src/utils/rescaler_utils.c:50-53).
Since `work` have bound information, `WEBP_UNSAFE_MEMSET` has
been changed to `memset`.
In `WebPRescalerImport` (src/utils/rescaler_utils.c:140-150), where
`irow` and `frow` are swapped, self-assignments for `dst_width` and
`num_channels` were added side-by-side with the pointer assignments.
Additionally, `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used for the
pointer assignments to handle the `WEBP_ASSUME_UNSAFE_INDEXABLE_ABI`
setting used during testing.
Bug: 432511821
Change-Id: If716fb79a06dee9e807eff060806daf038810523
This was necessary after:
44257cb8 apply clang-format
Which made some single-line statements into multi-line. Using braces on
multi-line statements better conforms with the Google style guide.
Bug: 433996651
Change-Id: I615c0ecf3b94571f67fceadfe8c15914aea45ccb
Reasoning:
The `sorted` parameter in `BuildHuffmanTable`
(`src/utils/huffman_utils.c:87`) is annotated with
`WEBP_COUNTED_BY_OR_NULL(code_lengths_size)`. Analysis of the
access patterns (lines 137, 177, 207) shows that the indices used
are bounded by `code_lengths_size`. Since `sorted` can be NULL,
`_OR_NULL` is used. When compiling, calls
to `BuildHuffmanTable` in `VP8LBuildHuffmanTable` (line 272) required forging bounds because the `sorted` buffer, allocated
via `WebPSafeMalloc` or on the stack, was treated as unsafe.
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` is used at the call sites to
provide the necessary bounds information.
Bug: 432511821
Change-Id: I6fea3ac5d77cb56139f9748ba0277a4f0ad21737
Reasoning:
The `data` pointer in `WebPEstimateBestFilter` is annotated with
`WEBP_COUNTED_BY((size_t)height * width)`. This reflects the
allocation size found by tracing the call graph back to
`EncodeAlpha` (src/enc/alpha_enc.c:332), where the buffer is
allocated using `WebPSafeMalloc` with a size of `width * height`.
Although the function signature includes a `stride` parameter used
for pointer arithmetic (`data + j * stride` at
src/utils/filters_utils.c:44), the implementation implicitly assumes
`stride == width` for vertical and gradient filtering logic (e.g.,
accesses like `p[i - width]` at line 49). The only caller
(src/enc/alpha_enc.c:220) respects this assumption by passing `width`
as the `stride`. Therefore, `height * width` accurately represents
the required buffer size.
Bug: 432511821
Change-Id: I7d308cf8dcccd7d6128a17dbc0f7b177ee9282a1
webpdecodeutils is a subset of webputils, so we add -fbounds-safety to
it to ensure that the files are being compiled the same way between
modules.
Bug: webp:432511821
Change-Id: I5c01a87601a13c331b628c605238e645d1efa77f
Reasoning:
The `palette` parameter in `PaletteSortModifiedZeng` (src/utils/palette.c)
was accessed using `palette[i]` in a loop from 0 to `num_colors` (line
386), indicating it's an array of size `num_colors`. The `palette_in`
parameter was also deduced to be of size `num_colors` based on its
usage and how the indices accessing it are generated. Therefore, both
`palette` and `palette_in` were annotated with
`WEBP_COUNTED_BY(num_colors)`. This change required updating the caller
function `PaletteSort` (lines 393-395 in src/utils/palette.c and lines
61-63 in src/utils/palette.h) to match the new signature by adding the
same annotations to its `palette_sorted` and `palette` parameters.
The `palette` parameter in
`PaletteHasNonMonotonousDeltas` was being indexed like an array but was
typed as a `WEBP_SINGLE` pointer. Since `palette` is indexed up to
`num_colors`, it was annotated with `WEBP_COUNTED_BY(num_colors)`. This
introduced a warning at the call site in `PaletteSortMinimizeDeltas`
(src/utils/palette.c:197), as it passed a `WEBP_SINGLE` pointer
(`palette_sorted`) where `WEBP_COUNTED_BY` was expected. Analysis showed
both `palette_sorted` and `palette` parameters in
`PaletteSortMinimizeDeltas` are accessed up to `num_colors`, so they were
annotated with `WEBP_COUNTED_BY(num_colors)`.
The `sorted` parameter in `SearchColorNoIdx`
was annotated with `__counted_by(num_colors)` in both the definition
(src/utils/palette.c:68) and declaration (src/utils/palette.h:40).
This change led to cascading errors during testing.
These errors occurred because callers
passed pointers that were considered `__unsafe_indexable` under this
setting. To resolve this, the following functions were also
annotated:
- `PrepareMapToPalette` (src/utils/palette.c:85, src/utils/palette.h:46):
`palette`, `sorted`, and `idx_map` were annotated with
`__counted_by(num_colors)`.
The pointer `cooccurrence` in `src/utils/palette.c` is used as a
flattened 2D array of size `num_colors * num_colors` in functions
`CoOccurrenceFindMax` (lines 226-252) and `CoOccurrenceBuild` (lines
255-300). The parameters in these functions were annotated with
`WEBP_COUNTED_BY(num_colors * num_colors)` to reflect this usage and
fix the original bounds safety errors during indexing.
In the caller function `PaletteSortModifiedZeng` (lines 307-391),
`cooccurrence` is a local variable allocated using `WebPSafeCalloc`.
To ensure compatibility with the annotated parameters, especially
during test builds where local variables appeared to be treated as
unsafe pointers, `WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used when
passing `cooccurrence` to `CoOccurrenceBuild` (line 327) and
`CoOccurrenceFindMax` (line 333). This ensures a pointer with the
correct bounds is passed to the callees in all build configurations.
Bug: 432511821
Change-Id: I7540968ecca67645c5ca57e542433971b235e582
Previously, WebPAnimEncoder::prev_rect was set in
SetFrame()>PickBestCandidate(), and SetFrame() may be called twice in
a row in CacheFrame(), making prev_rect value wrong in the second
SetFrame() call. Fortunately, the second call is always for a
keyframe, and in that case SetFrame() does not use the value of
prev_rect.
Since commit b1feaa4 that may have been a bigger issue as
PickBestCandidate() was moved from the end of SetFrame() to
SetFrame()>GenerateCandidates()>PickBestCandidate(), effectively
changing the value of prev_rect for non-keyframe candidates within the
same SetFrame() call. Fortunately, prev_rect is only used at the
beginning of SetFrame(), before any call to GenerateCandidates(),
hence the no-op change.
Change-Id: I693dbf9d264b8768d25a29d75b511e616a3db56b
Reasoning:
The `data` parameter in `QuantizeLevels` (defined in
`src/utils/quant_levels_utils.c` line 33) was causing bounds safety
errors because it was accessed using array subscripts (e.g., `data[n]`
at lines 60-63 and 137) but was typed as a single pointer. The size of
the buffer pointed to by `data` is determined by the `width` and
`height` parameters, calculated as `data_size = height * width` at
line 39. The loops accessing `data` iterate up to `data_size`. To fix
this, the `data` parameter in both the function definition and its
declaration (`src/utils/quant_levels_utils.h` line 30) was annotated
with `WEBP_COUNTED_BY((size_t)width * height)`.
Bug: 432511821
Change-Id: Idfe8810eaaf1239d86e38eb661a1a987d817127c
Reasoning:
The function `NextTableBitSize` is only called by `BuildHuffmanTable`
(src/utils/huffman_utils.c:196). In `BuildHuffmanTable`, `count` is
defined as a local array `int count[MAX_ALLOWED_CODE_LENGTH + 1]`
(line 93). This array decays to a pointer when passed to
`NextTableBitSize`. The maximum index accessed within `NextTableBitSize`
is `MAX_ALLOWED_CODE_LENGTH - 1` (line 75, inside a loop where `len`
starts <= `MAX_ALLOWED_CODE_LENGTH` and increments up to
`MAX_ALLOWED_CODE_LENGTH`).
Since the caller provides an array of size `MAX_ALLOWED_CODE_LENGTH + 1`
and the accesses are within bounds, the `count` parameter in
`NextTableBitSize` is annotated with
`WEBP_COUNTED_BY(MAX_ALLOWED_CODE_LENGTH + 1)`. This resolves the
compiler error.
Bug: 432511821
Change-Id: I29a55dd7f09c04aa3a2394996cf0e123d985fcd0
Reasoning:
The `histogram` parameter in `GenerateOptimalTree`
(src/utils/huffman_encode_utils.c:170) is used as an array indexed up
to `histogram_size`. It was annotated with
`__counted_by(histogram_size)` to fix bounds-safety errors. This
required annotating the caller `VP8LCreateHuffmanTree`
(src/utils/huffman_encode_utils.c:411). However, the size of
`histogram` in `VP8LCreateHuffmanTree` depends on another parameter
(`huff_code->num_symbols`), preventing direct annotation in the
signature. Instead, a local `__bidi_indexable` pointer
(`bounded_histogram`) was forged inside `VP8LCreateHuffmanTree` using
`__unsafe_forge_bidi_indexable` with the correct size and passed to
`GenerateOptimalTree`. This also required annotating parameters
`good_for_rle` and `counts` in `OptimizeHuffmanForRle`
(src/utils/huffman_encode_utils.c:37) and forging a bounded pointer
for `buf_rle` (`bounded_buf_rle`) in `VP8LCreateHuffmanTree` to pass
to `OptimizeHuffmanForRle`.
Bug: 432511821
Change-Id: I5905ff93f4cc0a9ff03c5786b5ac20c6bfa965ff
Reasoning:
The `palette` parameter in `GetColorPalette` (src/utils/palette.c:100)
was annotated with `WEBP_COUNTED_BY_OR_NULL(MAX_PALETTE_SIZE)` to fix
an array subscript error at src/utils/palette.c:146. This annotation
reflects the function's contract, documented in src/utils/palette.h,
which states that if `palette` is not NULL, it must point to memory
allocated for at least `MAX_PALETTE_SIZE` elements. To make
`MAX_PALETTE_SIZE` visible, `src/webp/format_constants.h` was included
in `src/utils/palette.h`. Consequently, the wrapper function
`WebPGetColorPalette` (src/utils/utils.c:273) and its declaration in
`src/utils/utils.h` were also annotated similarly, requiring the
inclusion of `src/webp/format_constants.h` in `src/utils/utils.h` as
well. This resolved a compilation error caused by the type mismatch
when calling the annotated `GetColorPalette` from `WebPGetColorPalette`.
Bug: 432511821
Change-Id: Iceebf2facf9558dd49889f056f028d9a3fb22d41
Reasoning:
The struct `VP8LBitWriter` in `src/utils/bit_writer_utils.h` uses
`buf`, `cur`, and `end` pointers to manage a dynamic buffer. Pointer
arithmetic on `buf` and `cur` caused bounds safety errors.
Initial attempts using `WEBP_ENDED_BY(end)` for both `buf` and `cur`
failed due to compiler limitations (one field cannot be the upper
bound for multiple other fields). Using `WEBP_INDEXABLE` for `buf` and
`cur` resolved the arithmetic errors but caused ABI incompatibility
issues, as `VP8LBitWriter` is used in multiple compilation units with
different settings.
The final approach annotates `buf` with `WEBP_ENDED_BY(end)` and `cur`
with `WEBP_UNSAFE_INDEXABLE`. This resolves the arithmetic errors for
both pointers without changing the ABI. `WEBP_UNSAFE_INDEXABLE` is
used for `cur` as a workaround for the `WEBP_ENDED_BY` limitation and
ABI constraints.
Additionally, the `VP8LBitWriterResize` function in
`src/utils/bit_writer_utils.c` was modified:
- The assignments to `bw->buf`, `bw->end`, and `bw->cur` (lines
231-233) were reordered and updated to use the local
`allocated_buf` variable on the right-hand side. This satisfies the
consecutive assignment requirement imposed by `WEBP_ENDED_BY(end)`
on `bw->buf`.
- The local variable `allocated_buf` (line 207) was annotated with
`WEBP_BIDI_INDEXABLE`.
- The allocation for `allocated_buf` (line 222) now uses
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` to create a safe, bounded
pointer from the result of `WebPSafeMalloc`, fixing potential type
mismatches when `suppress_fbounds_errors=yes`.
Bug: 432511821
Change-Id: I603a6a7d3ff3bf2a8edf9749a9898ea227c32982
Reasoning:
Analysis of `GetLE32` and its called function `GetLE16`
(src/utils/utils.h:95) showed that `GetLE32` requires access to 4
bytes starting from `data`. `GetLE16` accesses 2 bytes.
Therefore, `WEBP_COUNTED_BY(4)` was added to the `data` parameter of
`GetLE32`. Similarly, `WEBP_COUNTED_BY(2)` was added to `GetLE16`.
During this analysis, related functions `GetLE24`, `PutLE16`,
`PutLE24`, and `PutLE32` (src/utils/utils.h lines 99-123) were also
identified as needing similar annotations. They were annotated with
`WEBP_COUNTED_BY(3)`, `WEBP_COUNTED_BY(2)`, `WEBP_COUNTED_BY(3)`, and
`WEBP_COUNTED_BY(4)` respectively, based on the number of bytes they
access or modify.
Bug: 432511821
Change-Id: I5783392bc8dcaa2f346a81928ef496fc52da3a30
Reasoning:
The `pool` parameter in the static function `SetBitDepths`
(`src/utils/huffman_encode_utils.c:140`) is indexed as an array. Since
the function is static and the exact size is not available in the
signature, `pool` was annotated as `__bidi_indexable`. This required
updating the caller, `GenerateOptimalTree` (also static), where the
corresponding argument `tree_pool` is derived from the `tree` parameter.
Consequently, both the `tree` parameter and the local variable
`tree_pool` in `GenerateOptimalTree`
(`src/utils/huffman_encode_utils.c:170`) were also annotated as
`__bidi_indexable`. Finally, the call to `GenerateOptimalTree` from the
ABI-visible function `VP8LCreateHuffmanTree`
(`src/utils/huffman_encode_utils.c:417`) required adapting the
`huff_tree` pointer. Tracing the allocation in `src/enc/vp8l_enc.c:473`
revealed that `huff_tree` is allocated with enough space for
`3 * max_num_symbols`. Therefore, `__unsafe_forge_bidi_indexable` was
used to cast `huff_tree` to `__bidi_indexable` with the calculated
bounds (`3 * num_symbols`).
Bug: 432511821
Change-Id: I53c6965c3fd708256241a225ec9223085a1ec2d4
Reasoning:
The errors reported for `tokens` in `CodeRepeatedValues`
(src/utils/huffman_encode_utils.c, lines 274, 283, 289, 294)
indicated pointer arithmetic on a pointer assumed to be `__single`.
Since `CodeRepeatedValues` is static and uses `tokens` as an iterator,
its signature was changed to use `HuffmanTreeToken* __indexable` for
both the parameter and return type. A similar static function,
`CodeRepeatedZeros`, was also updated to use `__indexable` pointers.
The caller, `VP8LCreateCompressedHuffmanTree`, passes a buffer `tokens`
with size `max_tokens`. Its signature (in .c and .h files) was
annotated to reflect this using `__counted_by(max_tokens)`. Because
`__counted_by` requires the size parameter to be updated alongside the
pointer if the pointer is modified, the implementation of
`VP8LCreateCompressedHuffmanTree` was refactored. Instead of modifying
the `tokens` parameter directly, a local iterator variable
`current_token` was introduced and explicitly annotated as
`__indexable` to ensure correct type propagation, especially when
`WEBP_ASSUME_UNSAFE_INDEXABLE_ABI` is active. This local iterator is
used for calls to `CodeRepeatedValues` and `CodeRepeatedZeros`.
Bug: 432511821
Change-Id: I07fe553341a613a0ea4d5284817098c31f3aefeb
Reasoning:
Analysis of `VP8LColorCacheInit`
(src/utils/color_cache_utils.c:29) revealed that `colors` is allocated
using `WebPSafeCalloc` with a size of `1 << hash_bits`. Therefore,
`colors` was annotated with `WEBP_COUNTED_BY_OR_NULL(1u << hash_bits)`.
To support this, `WebPSafeCalloc` (src/utils/utils.h:59,
src/utils/utils.c:214) was annotated to return
`WEBP_SIZED_BY_OR_NULL(nmemb * size)`. Since `WebPSafeCalloc` returns a
local pointer that defaults to unsafe when bounds safety is suppressed,
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` was used on the return value
(src/utils/utils.c:222). Similarly, `VP8LColorCacheInit` required
`WEBP_UNSAFE_FORGE_BIDI_INDEXABLE` when assigning the allocated pointer
to the struct field (src/utils/color_cache_utils.c:47). Finally,
`VP8LColorCacheInit` and `VP8LColorCacheClear` were modified to perform
side-by-side assignments to `colors` and `hash_bits` as required by the
`WEBP_COUNTED_BY_OR_NULL` annotation, using self-assignment for
`hash_bits` when necessary to maintain functional equivalence with the
original code.
Bug: 432511821
Change-Id: I63cb46909d883a2e8932043ac3117b05b37e8d40
