Conversation
DRSchlaubi
left a comment
There was a problem hiding this comment.
please run ./gradlew apiDump
|
i'm not very familiar with voice connections, can you explain a bit why this is needed? |
It kinda is and kinda isn't. I initially wanted to implement AES because it was faster than xsalsa but after doing a couple benchmarks it looks like xsalsa is a little bit more consistent and a couple microseconds faster. This isn't really that surprising considering Kord Voice isn't the most efficient voice library, it doesn't help that Java relies on |
| val encryption = if ((receiveVoice || streams != null) && encryption?.supportsDecryption != true) { | ||
| VoiceEncryption.XSalsaPoly1305() | ||
| } else { | ||
| encryption ?: VoiceEncryption.AeadAes256Gcm | ||
| } |
There was a problem hiding this comment.
It might be useful to log if the user's chosen encryption method ends up not being used.
There was a problem hiding this comment.
or even throw an exception? but logging a warning is probably best.
|
Functionality-wise LGTM. It likely will make adding/working with encryption a bit easier in the future too, if (or rather when) Discord decides to change things up. |
| val encryption = if ((receiveVoice || streams != null) && encryption?.supportsDecryption != true) { | ||
| VoiceEncryption.XSalsaPoly1305() | ||
| } else { | ||
| encryption ?: VoiceEncryption.AeadAes256Gcm | ||
| } |
There was a problem hiding this comment.
or even throw an exception? but logging a warning is probably best.
| */ | ||
| public fun append(nonce: ByteArrayView, cursor: MutableByteArrayCursor) | ||
|
|
||
| public interface Factory { |
There was a problem hiding this comment.
why do we need this factory? i'd rather not add it if not needed.
Not sure if removing |
I tested voice send/receive for all 3 xsalsa20 poly1305 nonce strategies, all work as expected.