Open
Conversation
|
The documentation preview is available at https://preview.netcord.dev/168. |
…into feature/e2ee
Refactored encryption/decryption APIs to accept media type and return detailed result codes. Updated error handling in VoiceOutStream and VoiceClient for retries and exceptions. Changed native library reference from "dave.dll" to "libdave.dll" and removed the old DLL. Improves flexibility and error reporting for voice operations.
Add session.OnSpeaking call on Speaking event to notify DaveSession of active speakers. Refactor ClientDisconnect handling to ensure user disconnect event is awaited before calling session.OnClientDisconnect, improving event flow and consistency.
Refactored Dave interop to use BufferHandle<T> SafeHandle for native buffer management, replacing raw pointer outputs with safer handles and spans. Updated P/Invoke signatures to use Span/ReadOnlySpan where possible. Added new EncryptorHasKeyRatchet and EncryptorIsPassthroughMode APIs. Updated VoiceClient and VoiceCommands to use new buffer patterns and float PCM audio. Updated libdave.dll for compatibility. Improves memory safety and modernizes interop usage.
…ctor voice encryption
Introduce TryEncrypt and TryDecrypt methods to IVoiceEncryption, allowing encryption and decryption to signal failure via return value instead of exceptions. Refactor Aes256GcmRtpSizeEncryption and XChaCha20Poly1305RtpSizeEncryption to implement these methods. Update VoiceClient to use TryDecrypt, improving error handling and memory management. Simplify LibsodiumException and adjust WebSocketClient and tests to accommodate the new API.
Refactor VoiceOutStream to encapsulate encryption in a new EncryptDave method, introducing DaveEncryptionResult for buffer/resource management. Replace manual array pooling and in-line encryption with a safer, clearer pattern using using statements and IDisposable. Update to use ReadOnlyMemory/Span for buffers, rename GetMaxCiphertextByteSize to GetMaxCiphertextSize, and switch to async sending with cancellation support. Improve error handling, code clarity, and resource safety throughout the voice pipeline.
Refactored DaveEncryptorException into a separate file under the NetCord.Gateway.Voice namespace. Removed DaveDecryptorException. Updated VoiceOutStream to throw DaveEncryptorException via a local static method for improved clarity and static analysis support.
Refactored the echo logic in VoiceModule to use voiceClient.SendVoice instead of writing to an output stream. Added handling to skip packets with null timestamps, ensuring that packet loss is mirrored to echo recipients. Removed outStream creation and usage.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 56 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
SourceGenerators/WebSocketClientEventsGenerator/WebSocketClientEventsGenerator.cs
Show resolved
Hide resolved
Refactored delay computation in SpeedNormalizingStream for clarity and correctness, passing pre-calculated millisecond values to Task.Delay and Thread.Sleep. Updated VoiceCommands to specify frame duration when creating OpusEncodeStream, ensuring consistent audio encoding configuration.
Replaced GetValueOrDefault with the null-coalescing operator (??) for assigning _externalSocketAddressDiscoveryTimeout, simplifying the default value logic when ExternalSocketAddressDiscoveryTimeout is null.
- Make OpusEncodeStream and helper classes sealed for clarity. - Separate Opus and PCM buffer sizes in OpusEncodeStreamInternal. - Use correct buffer sizes for encoding to improve safety and efficiency. - Override Flush/FlushAsync to send silence frames for clean stream end. - Simplify VoiceOutStream flush methods; silence now handled upstream. - Add 2s delay after playback loop in VoiceCommands. - Minor code style improvements and test path comment.
Marked OpusDecodeStream, SpeedNormalizingStream, and VoiceOutStream as sealed to prevent inheritance. Renamed _bufferSize to _pcmBufferSize in OpusDecodeStream and updated all related usages for improved clarity and consistency. These changes enhance code readability and enforce immutability of the stream classes.
Expanded PlayAsync to accept pcmFormat, voiceChannels, opusApplication, and frameDuration for customizable audio streaming. Updated OpusEncodeStream and ffmpeg invocation to use these parameters, supporting both float and short PCM formats. Refactored related code for consistency.
RecordAsync now accepts optional pcmFormat and voiceChannels parameters, allowing callers to customize the audio format and number of channels. ffmpeg arguments and OpusDecodeStream construction have been updated to use these new options.
Removed commented-out code related to audio streaming and Opus encoding/decoding in the VoiceCommands class. This cleanup does not affect current functionality.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 55 out of 56 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
NetCord/Gateway/WebSocketClient.cs:1037
InvokeEventWithDisposalAsyncdisposesdata(viadisposeData(data)) before awaiting the returnedValueTasks. If any handler returns an incomplete task and relies on resources owned bydata(e.g., pooled buffers), this can lead to use-after-return/data corruption. Dispose should happen only after all handler tasks have completed (e.g., in afinallyafterawait HandleTasksAsync(...)).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Several P/Invoke method declarations in Dave.cs have been commented out, disabling their usage while preserving the original code for future reference. These methods include native interop functions related to session, commit, welcome, encryptor, and decryptor operations. No other logic or structure was modified.
Replaced hardcoded "opus" library names with a DllName constant in LibraryImport attributes for maintainability. Removed [UnmanagedCallConv] attributes to rely on default calling conventions. No functional changes.
Simplify interop signatures by removing SuppressGCTransition and UnmanagedCallConv attributes. Introduce DllName constant for "libsodium" to avoid hardcoding. No functional changes to method signatures.
Replaced hardcoded "libzstd" strings with a DllName constant for all native library imports. Removed [UnmanagedCallConv] attributes and added [SuppressGCTransition] to IsError for improved performance and maintainability.
The [UnmanagedCallConv] attribute was removed from the CryptoSignEd25519VerifyDetached method declaration, so the calling convention for the unmanaged import is no longer explicitly specified.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 58 out of 59 changed files in this pull request and generated 13 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
No description provided.