Repository navigation
Send a wide fixture's real colors over RGB-only outputs - #227
Open
AnjinMeili wants to merge 1 commit into
Open
AnjinMeili wants to merge 1 commit into
AnjinMeili wants to merge 1 commit into
Conversation
NDI, HLS and RTSP carry RGB alone. With a fixture profile wider than RGB, such as white first, they sent the profile's first three channels, so white showed as red. They now send red, green and blue from where the profile puts them. **Light domain** - DriverBase.h: `correctToRgb` corrects one light and reads red, green and blue by the profile's offsets, 0 for a role it lacks. `lightScratch_` holds that one light, and `sizeLightScratch()` sizes it off the hot path, saying "not enough memory for one light of this fixture" in the status when it fails. - NdiDriver.h, HlsDriver.h, RtspDriver.h: each uses the base's helper and scratch; their own `corrScratch_` goes. **Tests** - unit_NdiDriver: sends a white-first profile's colors, not its white; the driver and layer live on the heap, outside a test's stack frame. HLS and RTSP share the helper and have no test of their own for it. - unit_ModuleContract: the NdiDriver known-list text names `lightScratch_`; the measurement comment is one line, within docgen's rules, since the file was touched. - test_desktop: 2,851 passed on the Pi 5 at this step. - build_esp32 esp32s3-n16r8: clean, no warnings. - Scenarios skipped: every driver grows by one ScratchBuffer handle, 32 bytes on a 64-bit host and 16 on an ESP32, and nothing changes per frame for an RGB wiring. **Docs/CI** - check_prose: NegatedHeading 6 to 8 and SelfReference 16 to 17 are main's baseline: the committed tree measures the same with this change absent. **Reviews** - Wide profiles took their first three channels on an RGB-only output: done, `correctToRgb` reads by role. - A failed scratch allocation went silent: done, `sizeLightScratch` names it in the status. - HLS and RTSP kept the first-three-channels copy after NDI moved: done, both on the helper. - The ModuleContract known-list text named the removed `corrScratch_`: done. - The ModuleContract measurement comment merged two cases into one claim: done, restored as three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
There was a problem hiding this comment.
🟡 Changes recommended
The new scratch-allocation failure path is untested, leaving shortage reporting and suppression of frame sending unverified.
1 open issue
What changed in this PR
This PR fixes color extraction for wide fixture profiles on RGB-only outputs, so NDI, HLS, and RTSP send the profile’s red, green, and blue channels rather than its first three channels.
Changes:
- Centralizes RGB extraction and one-light scratch allocation in
DriverBase, with an explicit allocation-failure status. - Updates all three streaming drivers to use the shared helper.
- Adds a white-first NDI regression test and refreshes test comments.
| File | Description |
|---|---|
test/unit/light/unit_NdiDriver.cpp |
Adds coverage for RGB extraction from a white-first profile. |
test/unit/core/unit_ModuleContract.cpp |
Updates the scratch-buffer name and measurement comment. |
src/light/drivers/RtspDriver.h |
Uses shared scratch storage and RGB extraction. |
src/light/drivers/NdiDriver.h |
Uses the shared helper and closes the sender on scratch allocation failure. |
src/light/drivers/HlsDriver.h |
Uses shared scratch storage and RGB extraction. |
src/light/drivers/DriverBase.h |
Adds role-based RGB extraction and checked scratch allocation. |
Review effort: Balanced
Comment on lines
+157
to
+159
| if (correction_.outChannels <= 3 || lightScratch_.resize(correction_.outChannels)) return true; | ||
| setStatus("not enough memory for one light of this fixture", Severity::Error); | ||
| return false; |
This branch has not been deployed
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.
NDI, HLS and RTSP carry RGB alone. With a fixture profile wider than RGB, such as white first, they sent the profile's first three channels, so white showed as red. They now send red, green and blue from where the profile puts them.
Light domain
correctToRgbcorrects one light and reads red, green and blue by the profile's offsets, 0 for a role it lacks.lightScratch_holds that one light, andsizeLightScratch()sizes it off the hot path, saying "not enough memory for one light of this fixture" in the status when it fails.corrScratch_goes.Tests
lightScratch_; the measurement comment is one line, within docgen's rules, since the file was touched.Docs/CI
Reviews
correctToRgbreads by role.sizeLightScratchnames it in the status.corrScratch_: done.What an agent wrote: Claude Code (Opus) wrote the code, the tests and this text from my direction; I reviewed every line and vouch for it.
Tested: On this branch on macOS, 2849 of 2850 desktop tests pass, the new NDI test among them; the one failure is main's discovery test injecting a WLED from 192.168.1.186, which this Mac holds, the bug #225 fixes.
scenario_Drivers_output_and_brightnesspasses; the ESP32-S3 build is clean. The same helper drives the ESP32 HUB75 driver in a follow-up, verified there on an Adafruit MatrixPortal S3 panel.check_coderan on this branch before the push: 1636 findings, 2 duplicated blocks fewer than main's 1638.Skipped checks:
🤖 Generated with Claude Code