-
Notifications
You must be signed in to change notification settings - Fork 284
Fix NaN bypass of AVIF_CLAMP in gain map pixel clamping #3189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
jortles
wants to merge
4
commits into
AOMediaCodec:main
Choose a base branch
from
jortles:fix/nan-safe-clamp-gainmap
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+165
−3
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
efa7c28
Fix NaN bypass of AVIF_CLAMP in gain map pixel clamping
jortles 1a816c3
Add CHANGELOG entry for NaN-safe clamp fix in gain map
jortles 1d65932
Add standalone reproducer for NaN crash in gain map tone mapping
jortles 608ca50
Fix reproducer: use LINEAR transfer to expose NaN crash
jortles File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| /* | ||
| * reproduce_gainmap_nan.c — Demonstrates NaN crash in gain map tone mapping. | ||
| * | ||
| * Without the fminf/fmaxf fix, this triggers an assertion failure | ||
| * in avifSetRGBAPixel() (debug builds) or undefined float-to-int | ||
| * conversion (release builds). | ||
| * | ||
| * The NaN arises from IEEE 754 indeterminate form 0 * Inf: | ||
| * - baseOffset = 0, so (baseLinear + baseOffset) = 0 for a black pixel | ||
| * - gainMapMax = 1000, so exp2f(lerp(0, 1000, 1.0) * 1.0) = +Inf | ||
| * - 0.0f * +Inf = NaN | ||
| * - AVIF_CLAMP(NaN, 0, 1) = NaN (ternary comparisons with NaN are false) | ||
| * | ||
| * We use LINEAR output transfer because sRGB's avifToGammaSRGB() absorbs | ||
| * NaN: all branch conditions (< 0, < 0.003, < 1.0) are false for NaN, | ||
| * so it falls through to "return 1.0f" — masking the bug as silent data | ||
| * corruption (black becomes white) instead of a crash. LINEAR's transfer | ||
| * function uses AVIF_CLAMP which passes NaN through, allowing it to reach | ||
| * the assertion in avifSetRGBAPixel(). | ||
| * | ||
| * Build (from libavif root): | ||
| * | ||
| * mkdir build && cd build | ||
| * cmake .. -DAVIF_CODEC_AOM=LOCAL -DAVIF_LIBYUV=LOCAL \ | ||
| * -DCMAKE_BUILD_TYPE=Debug -DBUILD_SHARED_LIBS=OFF | ||
| * cmake --build . --target avif -j$(nproc) | ||
| * cd .. | ||
| * | ||
| * cc -g -O1 -I include tests/reproduce_gainmap_nan.c \ | ||
| * build/libavif_internal.a build/_deps/libyuv-build/libyuv.a \ | ||
| * build/_deps/libaom-build/libaom.a -lstdc++ -lm -lpthread \ | ||
| * -o reproduce_gainmap_nan | ||
| * | ||
| * ./reproduce_gainmap_nan | ||
| * | ||
| * Expected without fix: assertion failure in avifSetRGBAPixel | ||
| * Expected with fix: "PASS: no crash" | ||
| */ | ||
|
|
||
| #include <avif/avif.h> | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
| #include <string.h> | ||
|
|
||
| int main(void) { | ||
| /* 2x2 black base image (sRGB, BT.709) */ | ||
| avifImage *base = avifImageCreate(2, 2, 8, AVIF_PIXEL_FORMAT_YUV444); | ||
| if (!base) { | ||
| fprintf(stderr, "Failed to create base image\n"); | ||
| return 1; | ||
| } | ||
| base->colorPrimaries = AVIF_COLOR_PRIMARIES_SRGB; | ||
| base->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB; | ||
| base->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_BT709; | ||
| base->yuvRange = AVIF_RANGE_FULL; | ||
| if (avifImageAllocatePlanes(base, AVIF_PLANES_YUV) != AVIF_RESULT_OK) { | ||
| fprintf(stderr, "Failed to allocate base planes\n"); | ||
| avifImageDestroy(base); | ||
| return 1; | ||
| } | ||
| /* Y=0 (black), U=128/V=128 (neutral chroma) */ | ||
| memset(base->yuvPlanes[0], 0, (size_t)base->yuvRowBytes[0] * 2); | ||
| memset(base->yuvPlanes[1], 128, (size_t)base->yuvRowBytes[1] * 2); | ||
| memset(base->yuvPlanes[2], 128, (size_t)base->yuvRowBytes[2] * 2); | ||
|
|
||
| /* 2x2 gain map image — all pixels at maximum (255 -> 1.0 normalized) */ | ||
| avifGainMap *gainMap = avifGainMapCreate(); | ||
| if (!gainMap) { | ||
| fprintf(stderr, "Failed to create gain map\n"); | ||
| avifImageDestroy(base); | ||
| return 1; | ||
| } | ||
| gainMap->image = avifImageCreate(2, 2, 8, AVIF_PIXEL_FORMAT_YUV444); | ||
| if (!gainMap->image) { | ||
| fprintf(stderr, "Failed to create gain map image\n"); | ||
| avifGainMapDestroy(gainMap); | ||
| avifImageDestroy(base); | ||
| return 1; | ||
| } | ||
| gainMap->image->yuvRange = AVIF_RANGE_FULL; | ||
| gainMap->image->matrixCoefficients = AVIF_MATRIX_COEFFICIENTS_IDENTITY; | ||
| if (avifImageAllocatePlanes(gainMap->image, AVIF_PLANES_YUV) != AVIF_RESULT_OK) { | ||
| fprintf(stderr, "Failed to allocate gain map planes\n"); | ||
| avifGainMapDestroy(gainMap); | ||
| avifImageDestroy(base); | ||
| return 1; | ||
| } | ||
| memset(gainMap->image->yuvPlanes[0], 255, (size_t)gainMap->image->yuvRowBytes[0] * 2); | ||
| memset(gainMap->image->yuvPlanes[1], 255, (size_t)gainMap->image->yuvRowBytes[1] * 2); | ||
| memset(gainMap->image->yuvPlanes[2], 255, (size_t)gainMap->image->yuvRowBytes[2] * 2); | ||
|
|
||
| /* | ||
| * Gain map metadata crafted to trigger NaN: | ||
| * gainMapMin = 0 -> lerp lower bound | ||
| * gainMapMax = 1000 -> lerp upper bound | ||
| * gamma = 1 -> no gamma distortion | ||
| * baseOffset = 0 -> (baseLinear + 0) = 0 for black pixels | ||
| * altOffset = 0 | ||
| * | ||
| * The math: lerp(0, 1000, powf(1.0, 1.0)) = 1000 | ||
| * exp2f(1000 * weight) = +Inf | ||
| * (0.0 + 0.0) * +Inf = NaN (IEEE 754) | ||
| */ | ||
| for (int c = 0; c < 3; ++c) { | ||
| gainMap->gainMapMin[c] = (avifSignedFraction){ 0, 1 }; | ||
| gainMap->gainMapMax[c] = (avifSignedFraction){ 1000, 1 }; | ||
| gainMap->gainMapGamma[c] = (avifUnsignedFraction){ 1, 1 }; | ||
| gainMap->baseOffset[c] = (avifSignedFraction){ 0, 1 }; | ||
| gainMap->alternateOffset[c] = (avifSignedFraction){ 0, 1 }; | ||
| } | ||
| gainMap->baseHdrHeadroom = (avifUnsignedFraction){ 0, 1 }; | ||
| gainMap->alternateHdrHeadroom = (avifUnsignedFraction){ 6, 1 }; | ||
| gainMap->useBaseColorSpace = 1; | ||
| gainMap->altColorPrimaries = AVIF_COLOR_PRIMARIES_SRGB; | ||
| gainMap->altTransferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB; | ||
| gainMap->altMatrixCoefficients = AVIF_MATRIX_COEFFICIENTS_BT709; | ||
| gainMap->altYUVRange = AVIF_RANGE_FULL; | ||
| gainMap->altDepth = 8; | ||
| gainMap->altPlaneCount = 3; | ||
|
|
||
| /* Output tone-mapped image — set format/depth only. | ||
| * avifRGBImageApplyGainMap sets width/height and allocates pixels internally. */ | ||
| avifRGBImage toneMap; | ||
| memset(&toneMap, 0, sizeof(toneMap)); | ||
| toneMap.depth = 8; | ||
| toneMap.format = AVIF_RGB_FORMAT_RGBA; | ||
|
|
||
| avifContentLightLevelInformationBox clli; | ||
| memset(&clli, 0, sizeof(clli)); | ||
| avifDiagnostics diag; | ||
| avifDiagnosticsClearError(&diag); | ||
|
|
||
| /* Apply with full HDR headroom (weight = 1.0). | ||
| * Use LINEAR transfer so NaN propagates through to avifSetRGBAPixel. | ||
| * (sRGB's gamma function absorbs NaN to 1.0f, hiding the crash.) */ | ||
| avifResult result = avifImageApplyGainMap(base, gainMap, 6.0f, | ||
| AVIF_COLOR_PRIMARIES_SRGB, | ||
| AVIF_TRANSFER_CHARACTERISTICS_LINEAR, | ||
| &toneMap, &clli, &diag); | ||
|
|
||
| if (result == AVIF_RESULT_OK) { | ||
| printf("Result: OK\n"); | ||
| } else { | ||
| printf("Result: %s (%s)\n", avifResultToString(result), diag.error); | ||
| } | ||
| printf("PASS: no crash\n"); | ||
|
|
||
| avifRGBImageFreePixels(&toneMap); | ||
| avifGainMapDestroy(gainMap); | ||
| avifImageDestroy(base); | ||
| return 0; | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anthony: Thank you for the pull request. Please add an entry for this fix to the "Changed since 1.4.1" section in CHANGELOG.md.