-
-
Notifications
You must be signed in to change notification settings - Fork 60
Fix: Resolve compilation errors for callback feature (builds on #151) #171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix: Resolve compilation errors for callback feature (builds on #151) #171
Conversation
Co-authored-by: alphacr792 <[email protected]>
Co-authored-by: alphacr792 <[email protected]>
…/windows-capture into feature/callback-with-fixes
|
Thank you for the pull request. However, I noticed a few issues:
I would love to hear your thoughts on this. @codex, review this PR. |
|
Codex Review: Here are some suggestions. windows-capture/src/encoder.rs Lines 1463 to 1468 in 9a76168
[P1] Stop treating raw sample buffers as encoded video The callback sends whatever https://github.com/NiiightmareXD/windows-capture/blob/9a76168ea80061519c7eb8c52111be9b147456cf/test_implementation#L1 The commit adds Reply with About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex Review: Here are some suggestions.
https://github.com/NiiightmareXD/windows-capture/blob/9a76168ea80061519c7eb8c52111be9b147456cf/test_implementation#L1
[P2] Drop committed test binaries
The commit adds test_implementation and test_windows_integration binaries (~3.6 MB each). These compiled artifacts bloat the repository and will quickly become stale; they should be generated as part of CI or downloaded on demand instead of being versioned.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| // Extract encoded video data and send to callback | ||
| let video_data = sample.Buffer()?; | ||
| let mut video_array: Array<u8> = Array::new(); | ||
| CryptographicBuffer::CopyToByteArray(&video_data, &mut video_array)?; | ||
|
|
||
| let encoded_frame = EncodedFrame { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Stop treating raw sample buffers as encoded video
The callback sends whatever MediaStreamSample::Buffer returns as an EncodedFrame. When the sample was created from Direct3D11 surfaces this property is typically empty or fails, so the handler will error out before the transcoder runs. Even when it succeeds, the bytes are the uncompressed BGRA frame, not the H264/AAC output generated by the MediaTranscoder, so the network layer sends multi‑megabyte raw frames and downstream decoders never receive keyframes. To stream encoded data you need to consume the transcoder’s output stream (or another post‑encode hook) instead of the pre‑encode sample.
Useful? React with 👍 / 👎.
Hi there!
I'm working on a project that needs high-performance screen capture, and the callback feature from PR #151 is perfect for it.
I noticed the original PR was failing to build with the latest dependencies, so I dug in and patched the compilation errors. I'm opening this new PR so everyone can get it working without any issues.
This combines all the work from #151 with the necessary fixes. Hopefully, this helps anyone else who stumbles upon the same issue!
Changes:
callbackargument in thestart()method from PR Add Real-Time Encoded Frame Streaming and Network Transmission Support #151.src/encoder.rsandsrc/network.rscaused by dependency updates.