Repository navigation
Conversation
Lights around a display now glow the color of the picture nearest them: a video source publishes one decoded RGB frame per tick, and an effect gives each light the mean of the rectangle mapping to it. The frame comes from a synthesized test pattern or a binary PPM, so the whole path is reviewable and testable on a desktop build. Capturing a real picture from an HDMI grabber over USB follows in its own change. **Core** - VideoService: two sources, a strict Netpbm P6 header parser, and a one-active-source seat so a service added or removed at runtime hands over rather than going dark - VideoFrame: the frame contract, borrowing the producer's buffer for one tick - Every frame carries its tone curve, sRGB here, so a consumer averages light rather than encoded bytes **Light domain** - AmbilightEffect: a per-light box filter averaged in LINEAR light, then encoded once per light, so a part-lit zone reads as bright rather than as a dim mush - Positions reaching no LED are skipped, which on a border layout is most of the box, so a backlight strip costs its perimeter rather than its bounding box - edgeDepth widens the outermost zones past the overscan junk; detectBlackBars moves them inside a letterbox, which edgeDepth cannot do - Smoothing keeps 8.8 fixed-point state per channel, with snapAbove letting a cut land whole - MappingLUT: hasDestination(), the one question the effect asks per position **Tests** - unit_VideoService: 13 cases over the header grammar, the seat election and the pattern's sweep rate - unit_AmbilightEffect: 23 cases over orientation, coverage, the linear-light mean, smoothing in both directions, bar detection and the sparse-mapping skip, with a hollow-rectangle layout local to the file - scenario_Video_source: add, configure and remove the service and its consumer under a running pipeline, which also pins the type registry, since the runner constructs every module through src/module_types.cpp **Docs/CI** - A Video card under core/services.md and an Ambilight card under light/effects.md, each with its details section - screenshot_modules captures both A strip is placed on a wall's edge pixels by the MoonLive library's backlight.mll, so this adds no layout. docgen: appendix lines 5617 to 5643, for the three new headers, which is the one rule that rises. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The video source gains a third option: an HDMI grabber presenting itself as a UVC webcam, streaming MJPEG that the P4's JPEG hardware decodes. A console or a PC on the grabber's input now drives the lights directly. Every other target links a stub whose init fails, which the service reports as a status rather than an error. **Platform** - A UVC capture seam in platform.h: the device handle, the advertised format rows, the frame accessor and the drop counters - platform_esp32_usbvideo.cpp: the UVC stream and the hardware JPEG decode across three threads, with the decode off the render tick and the newest frame winning when one is still pending - Buffers belong to the platform for the life of one open, so a published frame borrows rather than copies, and the renderer drops it before deinit - Bulk payload headers are stripped by a stride read off the first header, which usb_host_uvc leaves in when a device's payload is a multiple of the packet size - Gated on the chip AND both SoC capabilities, since only the P4 has a High-Speed USB PHY and a JPEG decoder **Core** - VideoService: the usb source, the offered-format picker, staleMs, and hdr with the PQ and HLG curves, since MJPEG carries no HDR metadata and a grabber strips what the console sends - A hotplug is noticed by one atomic load a second, answered by a tree rebuild on the thread that may open the device **Tests** - unit_VideoService: the capture-capability pair and the PQ curve recovering a measured source's linear ratio **Docs/CI** - The Video card gains its four capture controls; details gain the P4 requirement, format choice, HDR and staleness - esp32: the usb_host_uvc component for the P4 alone, and a 2 KB control-transfer buffer, without which an MS2130 never enumerates - .clangd points the ESP32 platform directory at a firmware compile database Built for esp32p4rev3-eth-wifi, which links the UVC driver, and for esp32s3-n16r8, which links stubs and no component. docgen: appendix lines 5643 to 5669, four for VideoService's restored capture sections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An LED's three emitters are not equally efficient, so a neutral triple can render with a cast: on an SK6812 the green die runs hot and grey scenes come out tinted. Four trims correct it at the output, once per strip, rather than leaving every effect to guess. **Light domain** - balanceRed, balanceGreen and balanceBlue pull one die down to meet the others. Trim DOWN only, since there is no headroom above 255 and raising a channel clips rather than balances - whiteLevel trims the white die, which the RGB trims cannot reach: a white phosphor is separate hardware, often brighter than the trio beside it - Folded into the SAME table fill that applies the curve, as a linear pre-scale, so the hot path stays one lookup per channel and the curve still lands last - briLut carries a row per trim, and the 16-bit table a driver allocates for fine channels carries the same rows, so a 16-bit output is balanced exactly as an 8-bit one is - Amber and UV read an untrimmed row, being their own dies that an RGB trim says nothing about - FixtureProfilesModule: profileHasWhiteDie(), narrower than profileHasSynthChannel, so a profile carrying amber but no white does not offer a slider that changes nothing **Tests** - Six cases: one trim leaves the others untouched, a trim does not reach the white die, the curve and a trim compose into one table, whiteLevel moves the white alone, and whiteLevel of 0 puts the die out rather than leaving it at the never-black floor **Docs/CI** - The Shared driver card gains the four controls, with the reasoning in its details section Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shipped backlight layout placed its lights clockwise from the top-left corner and nothing else, so a strip entering anywhere else was mapped wrong: the picture arrives rotated by however many lights the real wiring is out by, which reads as a wrong picture rather than as a misconfiguration. **Light domain** - backlight.mll gains startCorner, clockwise and offset, so one strand serves any corner, either direction, and an entry part-way along an edge, which is what a gap for a TV stand leaves - placeLights walks one canonical perimeter through mod(), rather than four hard-coded segments, so the controls reorder INDICES only and the emitted coordinates stay a property of the box **Tests** - Four cases in unit_MoonLiveLayout, run against the shipped script rather than a copy, so a change to the file is a change to the test - The eight wirings, four corners by two directions, emit one coordinate set in seven distinct orders; offset is checked as a rotation of the plain walk; and a strand coarser than the wall still reaches its edge pixels Verified against the 60x34, 184-light wiring this was written for: same count, same cells, same order as the C++ layout it replaces. The docgen report is regenerated here, so it carries the whole branch rather than the base it was resolved to during the rebase. Two totals rise and neither is this change: appendix lines 5460 to 5512, for the file-level rationale of the four modules the branch adds, and split paragraphs 1791 to 1799, a rule newer than the comments it measures. This change adds no finding of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Gohnnyman
changed the title
This was referenced Oct 13, 2026
Collaborator
|
This one doesn't depend on video, and its own change is 3 files, the size I can review in one go. Please hold off on updating it for now, like #138 and #139, until I've done the groundwork; then a rebase onto main will show only the backlight change. (As in my comment on #138: one topic per PR, roughly 400 lines and ten files, so each can be reviewed properly. This one is a good example.) For reference only, the review points so far:
The shape is the textbook one, one walk around the frame with corner and direction as a rotation and a reversal, and the defaults reproduce the old order exactly. Nice. |
Collaborator
|
Hi @Gohnnyman, an update to my comment above: this one doesn't need to wait for the groundwork after all. It extends the So feel free to resubmit it now on its own, based on current main, with just its own commit ("Say where a backlight strand enters and which way it runs"): If you want to go one step further later: Hyperion also counts each edge separately (top, bottom, left, right), which allows a gap at the bottom for a TV stand. Using Hyperion's terms makes it easy for users coming from there. One naming note for the parts from #138: we won't use the name "Ambilight", since it's Philips' trademark. In the streams plan, the screen-following part becomes the area sampler of a picture effect. Those parts stay on hold until that groundwork is in. Thanks again! |
Contributor
Author
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
2 participants
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.
What's new
backlight.mllplaced its lights clockwise from the top-left corner and nothing else, so a strip entering anywhere else was mapped wrong — the picture arrives rotated by however many lights the wiring is out by, which reads as a wrong picture rather than a misconfiguration.startCorner,clockwiseandoffset, so one strand serves any corner, either direction, and an entry part-way along an edge — which is what a gap for a TV stand leaves.placeLightswalks one canonical perimeter throughmod()instead of four hard-coded segments, so the controls reorder indices only and the coordinates stay a property of the box.Verified on a 60x34, 184-light strip
Note
This PR is a part of #138 and #139
This PR Stacked on #139, so that PR should be merged first