Repository navigation
ntsc filter: clamp the left-edge taps with signed arithmetic - #19765
Merged
Merged
Conversation
ntsc_process_line clamped each chroma tap with "x + t < 0", but x is unsigned, so the test never fired. At the left edge x + t wrapped, the upper clamp took it to the last sample of the line, and the right edge's colour bled onto the left edge of every scanline. The wrapped sum also gave those taps the wrong phase whenever the phase count does not divide 2^32, which the default 12 does not. Take the tap position as a signed int, clamp it, and fold a negative phase back into range. Tests: an ASan+UBSan harness that runs ntsc_create and ntsc_packets on one line, black but for four white pixels on the right. Before: the left-most six output pixels are purple and blue. After: all black, right edge unchanged. Without the phase fold, UBSan reports index -6 into lut_cos. ntsc.so builds with -Wall -Wsign-compare and no warnings, against 2 before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
ntsc_process_lineclamped each chroma tap withx + t < 0, butxis unsigned, so the test never fired. At the left edgex + twrapped, the upper clamp took it to the last sample of the line, and the right edge's colour bled onto the left edge of every scanline. The wrapped sum also gave those taps the wrong phase whenever the phase count does not divide 2^32, which the default 12 does not.The tap position is now a signed
int, clamped, and a negative phase is folded back into range.Testing: an ASan+UBSan harness that runs
ntsc_createandntsc_packetson one line, black but for four white pixels on the right. Before: the left-most six output pixels are purple and blue. After: all black, right edge unchanged. Without the phase fold, UBSan reports index -6 intolut_cos.ntsc.sobuilt withgfx/video_filters/Makefile;-Wall -Wsign-comparegives no warnings, against 2 before.Found by a static-analysis pass over the tree.
🤖 Generated with Claude Code