feat(macos): experimental ProRes VideoToolbox encoder (opt-in, additive protocol)#5202
Open
Nottlespike wants to merge 12 commits into
Open
feat(macos): experimental ProRes VideoToolbox encoder (opt-in, additive protocol)#5202Nottlespike wants to merge 12 commits into
Nottlespike wants to merge 12 commits into
Conversation
…12.3+
AVCaptureScreenInput was deprecated in macOS 13 (October 2022) and is
fundamentally limited to 8-bit BGRA, blocking any honest HDR or 10-bit
work on the macOS capture path. ScreenCaptureKit has been available
since macOS 12.3 (March 2022) and is the only forward path; this
commit lays the foundation by adding a drop-in SCK-based backend that
preserves behaviour exactly (same pixel format, frame rate, display
selection) so it can be reviewed independently of the HDR work that
builds on top.
Changes:
* Add SunshineVideoCapture protocol in av_video.h declaring the
capture-side surface both backends expose.
* Make AVVideo conform to the protocol (no behaviour change; pure
declaration).
* Add SCVideo (sc_video.h / sc_video.m) implementing the same
protocol against SCStream + SCContentFilter + SCStreamConfiguration.
Built with -fobjc-arc for SCK's block-heavy API surface; objects
cross the MRC boundary via the standard +1-retain alloc/init
convention so display.mm continues to work in MRC.
* Drop incomplete frames from SCK output by inspecting
SCStreamFrameInfoStatus on each sample-buffer attachment, matching
the reliability the legacy path got for free from AVCaptureSession.
* display.mm now holds an id<SunshineVideoCapture> and branches at
construction via @available(macOS 12.3, *): SCVideo on supported
systems, AVVideo as fallback for older macOS.
* Wire ScreenCaptureKit framework into cmake/dependencies/macos.cmake
and cmake/compile_definitions/macos.cmake; set ARC compile flag on
sc_video.m only.
Pixel format stays 32BGRA for this commit; 10-bit + EDR metadata
follow in a subsequent change.
…pixel formats
With AVCaptureScreenInput, asking the capture surface for a 10-bit
pixel format silently produced 8-bit BGRA — the OS-level lie that
made HEVC Main10 / AV1 Main10 / ProRes 10-bit profiles on macOS into
fake HDR (color-tagged 8-bit data). With ScreenCaptureKit landing in
the previous commit, 10-bit pixel formats are actually honoured, but
SCK needs an explicit signal to attach HDR metadata to those buffers
instead of treating them as 10-bit Rec.709.
This commit wires SCStreamConfiguration.captureDynamicRange:
* Add +pixelFormatIsHighBitDepth: classifier covering the YUV 4:2:0,
4:2:2 and 4:4:4 10-bit BiPlanar formats plus ARGB2101010 packed
and 64-bit RGBA formats.
* On the synchronous init path, set captureDynamicRange immediately
if the starting pixel format is high bit depth so the very first
sample buffer carries HDR metadata.
* On the setPixelFormat: path (called by nv12_zero_device when the
encoder selects p010), also update captureDynamicRange and push
the new config to a running stream via -updateConfiguration:.
* Use SCCaptureDynamicRangeHDRLocalDisplay rather than canonical
HDR: game streaming wants the host display's actual HDR
characteristics (peak luminance, primaries) so the receiver shows
what a local user would see, not Apple's idealised reference.
* Guard the whole block behind @available(macOS 14.0, *); on
12.3-13.x SCK still honours the 10-bit pixel format request but
doesn't auto-tag buffers, so Sunshine's existing colorspace logic
continues to drive the encoder's color fields.
Validated on M4 Max: Sunshine's encoder probe matrix now includes
successful 10-bit HEVC and 10-bit ProRes entries that previously
could not have validated because the capture surface couldn't
deliver matching pixel data. ProRes-specific VideoToolbox color tags
land in a separate follow-up commit.
…session HDR Two improvements to the macOS ScreenCaptureKit backend that landed in the upstream review cycle (LizardByte#5190) and never made it back onto our fork's dev: 1. **Lifecycle hardening**: - Register the SCStreamOutput exactly once in -init, not on every -capture: call. SCStream retains outputs across stop/start cycles, so re-registering would fail or silently duplicate delivery. - Bound all SCK completion-handler waits to 5s. SCK should always invoke them, but a misbehaving system service must not hang the whole startup path. - @synchronized(self) around all reads/writes of currentCallback / currentSignal / streamRunning. The sample-handler queue, the -capture: caller, and the SCStream delegate all touch these from different threads. - dispatch_queue_attr_make_with_qos_class's third argument is a RELATIVE priority (range -15..0), not one of the legacy DISPATCH_QUEUE_PRIORITY_* constants. Using 0 keeps the queue at its QoS class's nominal priority. - CGDisplayBounds fallback when CGDisplayCopyDisplayMode returns NULL (display reconfiguration races). 2. **EDR gating fix**: The previous EDR code flipped captureDynamicRange = HDRLocalDisplay whenever the chosen CVPixelBuffer format was 10-bit. That's necessary but not sufficient: a 10-bit format may be selected for codec reasons (e.g., a ProRes profile that requires 4:4:4 10-bit input) without the client ever requesting HDR ingest. Without gating, Sunshine would tell the client "HDR mode false" in the SDP while emitting BT.2020 PQ-tagged buffers — a silent control/data-plane mismatch. Now EDR requires BOTH 10-bit pixel format AND the negotiated session's enable_hdr (plumbed from launch_session_t via the existing config.dynamicRange field on video::config_t). Default is SDR; HDR is opt-in per session. New init signature: initWithDisplay:frameRate:hdrAllowed: The old initializer is preserved as a convenience that passes NO. New log line at session start makes the gating visible: "Using ScreenCaptureKit capture backend (HDR allowed|blocked)"
FIND_LIBRARY(SCREEN_CAPTURE_KIT_LIBRARY ScreenCaptureKit REQUIRED) so configure fails fast on environments without the SDK rather than later at header-lookup time. sc_video.m is compiled unconditionally; the REQUIRED keyword ensures the build prerequisites are surfaced clearly when the build host's Xcode/SDK is older than 13.3 / 12.3 (well past routine compatibility). Addresses Copilot inline feedback from the closed upstream PR cycle.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds disabled-by-default experimental ProRes (VideoToolbox / macOS) support end-to-end, including server-side protocol advertisement, configuration/UI plumbing, and a new ScreenCaptureKit-based capture backend for modern macOS.
Changes:
- Introduces ProRes as a new video format with config gating (
prores_mode,prores_profile) and RTSP/NVHTTP advertisement. - Extends encoder probing/capabilities and pixel-format plumbing (NV24/P410) to support ProRes input requirements.
- Adds macOS ScreenCaptureKit capture backend and wires it into display selection for macOS 12.3+.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_video.cpp | Adds unit tests for ProRes config parsing and format gating helpers. |
| src_assets/common/assets/web/public/assets/locale/en.json | Adds UI strings for ProRes mode/profile settings. |
| src_assets/common/assets/web/configs/tabs/encoders/VideotoolboxEncoder.vue | Adds ProRes mode/profile controls to VideoToolbox encoder tab. |
| src_assets/common/assets/web/config.html | Adds default config values for prores_mode and prores_profile. |
| src/video.h | Defines format constants, adds ProRes codec slot, and helper predicates. |
| src/video.cpp | Implements ProRes profile mapping, probing, encoder selection constraints, and new pixel formats. |
| src/rtsp.cpp | Advertises ProRes support via SDP and validates/gates client videoFormat requests. |
| src/platform/windows/display_vram.cpp | Updates format comparisons to named constants. |
| src/platform/macos/sc_video.m | Adds ScreenCaptureKit-based capture implementation with HDR gating support. |
| src/platform/macos/sc_video.h | Declares SCVideo and its capture protocol conformance. |
| src/platform/macos/nv12_zero_device.cpp | Maps new NV24/P410 pixel formats to CVPixelBuffer types for macOS capture. |
| src/platform/macos/display.mm | Switches capture backend to SCK on macOS 12.3+ and generalizes capture pointer type. |
| src/platform/macos/av_video.h | Introduces SunshineVideoCapture protocol shared by AVVideo/SCVideo. |
| src/platform/common.h | Adds nv24/p410 to pix_fmt enumeration and string conversion. |
| src/nvhttp.cpp | Adds optional NVHTTP fields for experimental ProRes capability/profile. |
| src/nvenc/nvenc_base.cpp | Extends codec string logging to include ProRes. |
| src/config.h | Adds apply_config(...) declaration and new ProRes config fields. |
| src/config.cpp | Adds ProRes profile normalization and parses prores_mode/prores_profile. |
| docs/configuration.md | Documents prores_mode and prores_profile configuration options. |
| docs/changelog.md | Notes experimental macOS ProRes support addition in Unreleased section. |
| cmake/dependencies/macos.cmake | Requires ScreenCaptureKit framework at configure time. |
| cmake/compile_definitions/macos.cmake | Links ScreenCaptureKit and compiles sc_video.m with ARC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+2936
to
+2944
| active_prores_mode = config::video.prores_mode; | ||
| // Bind `require_prores` to the user-configured value, NOT to the | ||
| // mutable `active_prores_mode` global. `adjust_encoder_constraints` | ||
| // below may demote `active_prores_mode` to 0 when no encoder supports | ||
| // ProRes; reading from the immutable config source keeps the | ||
| // "user explicitly asked for forced ProRes" intent intact across that | ||
| // demotion, so we can fail loudly instead of silently picking a | ||
| // non-ProRes encoder. | ||
| const bool require_prores = config::video.prores_mode >= 2; |
Comment on lines
+818
to
+822
| if (video::active_prores_mode > 0) { | ||
| ss << "a=x-sunshine-prores:1"sv << std::endl; | ||
| ss << "a=x-sunshine-prores-profile:"sv << config::video.prores_profile << std::endl; | ||
| ss << "a=rtpmap:99 prores/90000"sv << std::endl; | ||
| } |
Comment on lines
+323
to
+342
| - (void)dealloc { | ||
| BOOL running; | ||
| SCStream *stream; | ||
| @synchronized(self) { | ||
| running = self.streamRunning; | ||
| stream = self.stream; | ||
| self.streamRunning = NO; | ||
| self.currentCallback = nil; | ||
| } | ||
| if (running && stream) { | ||
| // Best-effort synchronous stop with a bounded wait so a | ||
| // misbehaving SCK doesn't hang teardown. | ||
| dispatch_semaphore_t stopped = dispatch_semaphore_create(0); | ||
| [stream stopCaptureWithCompletionHandler:^(NSError *_Nullable error) { | ||
| (void) error; | ||
| dispatch_semaphore_signal(stopped); | ||
| }]; | ||
| dispatch_semaphore_wait(stopped, dispatch_time(DISPATCH_TIME_NOW, 2 * NSEC_PER_SEC)); | ||
| } | ||
| } |
| auto device = std::make_unique<nv12_zero_device>(); | ||
|
|
||
| device->init(static_cast<void *>(av_capture), pix_fmt, setResolution, setPixelFormat); | ||
| device->init((void *) av_capture, pix_fmt, setResolution, setPixelFormat); |
Comment on lines
+10
to
+15
| static_assert(video::SUNSHINE_FORMAT_H264 == 0); | ||
| static_assert(video::SUNSHINE_FORMAT_HEVC == 1); | ||
| static_assert(video::SUNSHINE_FORMAT_AV1 == 2); | ||
| static_assert(video::SUNSHINE_FORMAT_PRORES == 3); | ||
| static_assert(video::SUNSHINE_FORMAT_COUNT == 4); | ||
|
|
ScreenCaptureKit became available in macOS 12.3; Sunshine's deployment target (MACOSX_DEPLOYMENT_TARGET=14.2) is well above that. The @available(macOS 12.3, *) runtime branch in display.mm and the entire AVCaptureScreenInput-based AVVideo class were therefore dead code on every supported build. Changes: - Remove @available(macOS 12.3, *) check in display.mm; SCK is the only branch. - Replace `id<SunshineVideoCapture>` with `SCVideo *` directly — the protocol existed to abstract over both AVVideo and SCVideo, and is no longer needed with a single concrete capture class. - Move the small bits we still need (FrameCallbackBlock typedef, +displayNames / +getDisplayName: helpers) from av_video.{h,m} into sc_video.{h,m}. - Delete src/platform/macos/av_video.{h,m} (208 lines). - Drop both from PLATFORM_TARGET_FILES. Addresses andygrundman + ReenigneArcher review feedback on the original PR: "shouldn't keep workarounds for versions older than what we support."
Nottlespike
force-pushed
the
feat/macos/prores-videotoolbox
branch
from
208f61f to
f89e868
Compare
Contributor
Author
|
@andygrundman re: "how are you viewing this stream on the client side?" — testing with a custom Moonlight build that has the ProRes decode path wired (stock Moonlight has no ProRes decoder, which is why this PR is opt-in via `prores_mode` and the protocol negotiation is additive — a stock Moonlight client never sees ProRes signaled). The intent is to leave the door open for custom clients that want lossless-ish desktop streaming on LAN — capture workflows, A/B tests against HEVC, etc. — without affecting the default streaming path at all. |
21 tasks
Two Copilot findings on the SCK PR: 1. **dealloc signals pending semaphore.** -dealloc was clearing currentCallback but never signalling currentSignal. If the stream stopped without firing -stream:didStopWithError:, any caller still waiting on the semaphore returned by -capture: would stall forever. Snapshot the pending signal in the @synchronized block, then send it after clearing the callback so the waiter wakes up to observe their callback is nil and exits. 2. **Drop unused streamOutputAdded property.** Set in -init but never read. Removed both the @Property declaration and the assignment. The "register output exactly once at init" invariant is now structural (the call is in -init, not gated on a flag) and the dead state can't drift.
- Introduced `prores_mode` configuration option to enable custom clients to request ProRes video streams. - Added `prores_profile` configuration option to set the FFmpeg ProRes profile. - Updated changelog to reflect the addition of ProRes encoder plumbing. - Enhanced configuration documentation with details on ProRes options. - Implemented necessary changes in the codebase to support ProRes encoding, including updates to video format handling and encoder capabilities. - Added tests to validate ProRes configuration and protocol behavior.
…olbox tab The experimental ProRes mode selector landed in the generic Advanced tab while prores_profile (its inseparable companion — the mode toggle only makes sense if you also pick a profile) lived on the VideoToolbox Encoder tab. Splitting the two knobs across tabs hurts discoverability: users configuring VideoToolbox don't realise ProRes exists, and users on the Advanced tab can't see the profile that controls bit depth and chroma subsampling. Move prores_mode into VideoToolboxEncoder.vue directly above prores_profile so both controls sit together in their codec-relevant section. The macOS-only platform guard previously around prores_mode in Advanced.vue is implicit on the VideoToolbox tab (which is itself macOS-only), so it can be dropped. No backend or config change; pure UI relocation.
ProRes profiles are intrinsically 10-bit (proxy / lt / standard / hq)
or 12-bit (4444 / 4444 XQ); FFmpeg's prores_videotoolbox supported
input pix_fmt list does not include the 4:2:0 BiPlanar formats (NV12 /
P010) Sunshine has historically delivered for H.264 / HEVC / AV1. The
422 family wants 4:2:2 or higher chroma and the 4444 family wants
4:4:4 natively. Until now, the macOS capture path could only produce
4:2:0 buffers, so the ProRes encoder probe failed at avcodec_open2
with "Couldn't open" regardless of profile selection or colorspace
configuration.
Wire the 4:4:4 BiPlanar capture path end-to-end:
* Add nv24 / p410 to platf::pix_fmt_e and the from_pix_fmt switch.
* Map AV_PIX_FMT_NV24 / AV_PIX_FMT_P410 in video::map_pix_fmt.
* Populate the previously-NONE 4:4:4 pix_fmt slots on the
videotoolbox encoder declaration and add YUV444_SUPPORT to its
flags. H.264 and HEVC VideoToolbox don't gain anything new
functionally — they remain 4:2:0 only on Apple Silicon hardware —
but the 4:4:4 probe runs against them harmlessly and falls
through with their YUV444 capability bit set false, which is
correct.
* Route nv24 / p410 through nv12_zero_device in
display.mm::make_avcodec_encode_device, alongside the existing
nv12 / p010 paths.
* Set the matching CVPixelBufferType
(kCVPixelFormatType_444YpCbCr*BiPlanarVideoRange) per pix_fmt in
nv12_zero_device::init.
Update the ProRes encoder probe in test_prores to use a ProRes-shaped
config:
* dynamicRange = 1 (10-bit pix_fmt slot instead of 8-bit).
* encoderCscMode = 3 (full range, BT.709 instead of BT.601).
* chromaSamplingType = 1 (4:4:4 P410 instead of 4:2:0 P010).
Any ProRes probe that succeeds inherently uses 10-bit input, so
DYNAMIC_RANGE is promoted eagerly here rather than relying on
test_hdr_and_yuv444 below (which gates on PASSED and only sets
DYNAMIC_RANGE itself).
Verified end-to-end on M4 Max: the startup probe now produces
"Found ProRes encoder: prores_videotoolbox [videotoolbox]", where
previously the encoder failed to open at every config permutation
attempted.
This is the runtime-side companion to the build-deps change that
adds prores_videotoolbox to the FFmpeg configure's --enable-encoder
flag (LizardByte/build-deps#693). Without that, the encoder symbol
is missing from libavcodec.a's static codec list and
avcodec_find_encoder_by_name returns null regardless of the wiring
in this commit.
video.h: is_known_video_format() now uses `< SUNSHINE_FORMAT_COUNT` instead of the hardcoded `<= SUNSHINE_FORMAT_PRORES` upper bound. Future codecs are now a one-enum-bump change. video.cpp: require_prores binds to the immutable config::video.prores_mode rather than the mutable active_prores_mode. adjust_encoder_constraints() may demote active_prores_mode to 0; reading the immutable source keeps the "user explicitly asked for forced ProRes" intent intact so we fail loudly instead of silently picking a non-ProRes encoder. cmake/dependencies/macos.cmake: Pull in the SCREEN_CAPTURE_KIT_LIBRARY REQUIRED change too — this PR stacks on the SCK backend PR and needs the same SDK enforcement. Addresses Copilot inline feedback from the closed upstream PR cycle.
…evice
Upstream Sunshine on macOS has no host-side gamepad support: every
alloc_gamepad/gamepad_update/free_gamepad in src/platform/macos/input.cpp
is a stub that logs "Gamepad not yet implemented for MacOS" and returns
failure, and get_capabilities() returns 0. Moonlight clients send
gamepad events over the wire that the host silently drops.
This ports Lumen's working approach (MIT-licensed; trollzem/Lumen):
publish a virtual gamepad as an IOHIDUserDevice with a complete HID
report descriptor (16 buttons, 4-bit hat switch, 2x 8-bit triggers,
4x 16-bit signed stick axes). macOS's HID matching publishes the
device to userspace; games, emulators, GameController.framework, SDL,
and Steam all see it as a real USB Xbox-style controller (VID 0x1209,
PID 0x5853 — pid.codes open-source range, intentionally not in SDL's
known-controller database so it routes through generic mapping).
The IOHIDUserDevice creation path requires AMFI to be bypassed at
boot: `sudo nvram boot-args="amfi_get_out_of_my_way=1"` and reboot.
Without that, IOHIDUserDeviceCreateWithProperties fails the entitlement
check and the probe at input() construction time logs a clear
instruction. SIP can stay on. The DriverKit DEXT path (no AMFI flag,
requires Apple DriverKit entitlement application + signed
system-extension distribution) is the longer-term shipping story but
is multi-week work; this gets us to a working gamepad host today.
Changes:
- src/platform/macos/hid_gamepad.{h,m} (new): MRC Obj-C wrapper around
IOHIDUserDevice publishing the Xbox-style gamepad HID descriptor.
Maps Sunshine's 32-bit buttonFlags to the 16-bit HID button field +
4-bit hat switch + axes/triggers. Bounded 2s teardown via
IOHIDUserDeviceSetCancelHandler. QOS_CLASS_USER_INTERACTIVE serial
queue for report dispatch.
- src/platform/macos/input.{cpp -> mm}: renamed to Obj-C++ so the
gamepad path holds HIDGamepad strong references directly. Mouse/
keyboard injection paths unchanged. macos_input_t gains a 16-slot
HIDGamepad array + hid_gamepad_available flag (probed once at init).
alloc_gamepad finds the lowest free slot, createDevice's a virtual
gamepad, returns the slot index. free_gamepad disconnects +
releases. gamepad_update forwards state to the HIDGamepad's
updateState:. supported_gamepads now reports macos_hid /
macos_amfi_required depending on probe result.
- cmake/dependencies/macos.cmake: FIND_LIBRARY(IO_KIT_LIBRARY IOKit)
for IOHIDUserDevice* symbols.
- cmake/compile_definitions/macos.cmake: hid_gamepad.{h,m} added to
PLATFORM_TARGET_FILES, input.cpp -> input.mm, IO_KIT_LIBRARY linked.
- cmake/dependencies/ffmpeg.cmake: stage a small allow-list of FFmpeg
internal headers (h2645_parse.h, get_bits.h, etc.) into the dist
include tree at configure time. src/cbs.cpp uses libavcodec internals
that `make install` doesn't export; surfacing them inline avoids the
alternative of adding the FFmpeg source tree to the include path
wholesale (which collides with libc++'s "thread.h" and friends).
What this does not do:
- Rumble / haptic feedback (Sunshine's feedback_queue_t is accepted but
not forwarded — virtual HID has no force-feedback motor to drive).
- DualSense touchpad, gyro, adaptive triggers, LED.
- Battery state reporting.
These can be added later under the same HIDGamepad class if needed.
Verified locally: hid_gamepad.m compiles, _OBJC_CLASS_$_HIDGamepad is
linked into Sunshine.app, the AMFI-disabled probe runs at startup and
emits the right instruction line for users not yet on the bypass.
Functional end-to-end test (gamepad events from Moonlight client
actually reaching a game) requires AMFI bypass + reboot.
Lumen attribution: Lumen is MIT-licensed; this port preserves the
underlying design and copies the HID report descriptor verbatim.
Nottlespike
force-pushed
the
feat/macos/prores-videotoolbox
branch
from
f89e868 to
eedcf8f
Compare
Contributor
Author
|
Force-pushed ( Re Copilot's specific findings on this PR:
Let me know on the platform guard / SDP whitelist / test cleanup preferences; happy to do them in a follow-up commit on this PR. |
Per Copilot review: tests/unit/test_video.cpp:10-13 duplicated the exact static_asserts that already live next to the enumerator declarations in src/video.h:28-31. Renumbering a format would require keeping the two lists in sync. Replace with a single test-relevant invariant — SUNSHINE_FORMAT_COUNT must equal PRORES + 1 — and link the comment back to where the authoritative assertions live. The H264/HEVC/AV1/PRORES==N value constraints stay in src/video.h where they belong (next to the constants themselves).
|
ReenigneArcher
added
the
ai
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.
Reopens #5192 (closed prematurely; GitHub blocked direct reopen due to regenerated branch SHAs). All review feedback from the original cycle is addressed in additional commits. See the original PR for full discussion; rebased on current master.