fix(moq-video): make decode Frame/Consumer Send on macOS#2162
Merged
Conversation
The NVDEC merge (#2145) reshaped the public `decode::Frame` to hold the internal `frame::Frame` enum so a GPU-decoded frame can flow zero-copy into NVENC. On macOS that enum includes a `Surface` variant wrapping `CFRetained<CVPixelBuffer>`, which objc2 leaves `!Send`. That propagated up through `decode::Consumer` and broke `libmoq`, whose `tokio::spawn` of the decode loop requires a Send future. Linux-only CI never exercised the `Surface` variant so it slipped through. Move the `Send` assertion onto `macos::Surface` itself (the capture path already relied on this via a local `SendFrame` newtype, now removed) and add a compile-time Send check for the decode surface so a future regression fails per-platform in `cargo test` rather than downstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
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.
Summary
The NVDEC merge (#2145) broke
cargo check -p libmoqon macOS with "future cannot be sent between threads safely". Linux-only CI didn't catch it.That PR reshaped the public
decode::Frameto hold the internalframe::Frameenum so a GPU-decoded frame can flow zero-copy into NVENC. On macOS that enum has aSurfacevariant wrappingCFRetained<CVPixelBuffer>, which objc2 conservatively leaves!Send. The non-Send-ness propagated up throughdecode::Consumer, and libmoq'stokio::spawnof its decode loop requires aSendfuture, so the crate stopped compiling on macOS.Changes
frame.rs:unsafe impl Send for macos::Surface, with the safety justification the crate already relied on (CVPixelBuffer is a refcounted IOSurface wrapper; retain/release are thread-safe; pixels are only touched underCVPixelBufferLockBaseAddress).capture/channel.rs: removed theSendFramenewtype that asserted the same fact locally at the capture boundary. Theunsafenow lives once, on the type itself, and both the capture and decode paths inherit it.decode/mod.rs: added a compile-timeSendassertion test fordecode::Frameanddecode::Consumer, so a future non-Send variant failscargo teston the affected platform instead of only surfacing downstream.Public API changes
None.
macos::SurfaceandSendFrameare private (pub(crate)/ private); no exported signatures change. Not a breaking change.Test plan
cargo check -p libmoq(macOS)cargo check -p moq-video --all-features(macOS)decode::tests::frame_and_consumer_are_sendpassesnix develop --command just check(exit 0)Targets
devbecause it sits on top of the dev-only NVDEC work.(Written by Claude Fable 5)