Gate Syphon validity on actual support; minor follow-ups#2
Merged
Conversation
Without this, a config imported on Linux/Windows (or a macOS host without node-syphon installed) with `Output2SinkSyphonEnabled: true` and both NDI and FFmpeg disabled would pass valid() and start capture, but the worker would silently drop every frame because SyphonMetalServer is null. The renderer-side check in vingester-control.js had the same bug — the UI would happily show the instance as ready to start. Also: short comment in the worker clarifying that FFmpeg is intentionally absent from the BGRA endianness normalization (it gets a re-encoded JPEG from nativeImage and handles pixel format itself). Addresses codemouseai PR #1 review comments. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
![codemouseai[bot]](https://avatars.githubusercontent.com/in/2993891?s=60&v=4){kind=small}
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
1 participant
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.
Follow-up to #1 addressing the substantive review comments.
Real bug fixed
valid()(main process,vingester-browser.js:154) and the renderer-side mirror (vingester-control.js:336) both treatedcfg.s = trueas satisfying the Output 2 sink requirement, without checking whether Syphon is actually available on this host.Effect: a YAML config imported on Linux/Windows — or on a macOS host where
node-syphonis not installed — withOutput2SinkSyphonEnabled: trueand both NDI and FFmpeg disabled would:valid()→ instance startssyphonServer === null→ every frame is silently droppedFix:
vingester-browser.js: try-requirenode-syphonat module load (mirroring the worker's pattern), set asyphonAvailableflag, and require it invalid().vingester-control.js: gatebrowser.sonthis.support.syphon(already populated from main viaipcRenderer.invoke("support")).Trivial cleanup
vingester-browser-worker.js: short comment on the BGRA endianness normalization block explaining that FFmpeg is intentionally excluded — it receives a JPEG fromnativeImage.toJPEG()and handles pixel format itself, so the raw buffer endianness is irrelevant for it. (codemouseai Add Syphon sink (macOS) alongside NDI/FFmpeg #1.)Skipped
log.errorflooding on Syphon publish failures: NDI has the same pattern. Fixing only Syphon would create the opposite asymmetry. Left as-is; a real fix would be symmetric across all sinks and is out of scope here.Verified
support.syphon=true,syphonAvailable=true, validity behavior unchanged on this platform. Smoke run with the same Syphon-only test config (s: true,n: false,m: false) still autostarts, spawns the worker, creates aSyphonMetalServer, and publishes frames.node-syphonwas not installed.