code review

docpages/advanced_reference/unit_tests.md

@@ -24,6 +24,13 @@ export TEST_USER_ID="826535422381391913"
 export TEST_EVENT_ID="909928577951203360"
 ```
 
+You may also optionally set:
+```bash
+export TEST_DATA_DIR="/path/to/test/data"
+```
+If you wish to have test data (Robot.pcm etc) in a different location than two directories above the unit test program. If you do not specify
+this environment variable the default will be used.
+
 Then, after cloning and building DPP, run `cd build && ctest -VV` for unit test cases. 
 
 If you do not specify the `DPP_UNIT_TEST_TOKEN` environment variable, a subset of the tests will run which do not require discord connectivity.

docpages/advanced_reference/voice_model.md

@@ -26,7 +26,7 @@ during the MLS negotiation which was carried out earlier. All valid participants
 their ratchets, to decrypt the OPUS audio.
 
 Regardless of if DAVE is enabled or not, the OPUS stream (encrypted by DAVE, or "plaintext") is placed into an RTP packet, and then encrypted using the shared secret
-given by the websocket, known only to you and Discord, using the chacha20 poly1305 cipher.
+given by the websocket, known only to you and Discord, using the xchacha20 poly1305 cipher.
 
 The completed packet, potentially with two separate layers of encryption (one with a key only you and Discord know, and one with a key only you and participants in the
 voice chat know!), plus opus encoded audio is sent on its way via UDP to the RTP server, where Discord promptly distribute it to all participants in the chat.

include/dpp/discordclient.h

@@ -481,6 +481,7 @@ class DPP_EXPORT discord_client : public websocket_client
 	/**
 	 * @brief Handle JSON from the websocket.
 	 * @param buffer The entire buffer content from the websocket client
+	 * @param opcode The type of frame, e.g. text or binary
 	 * @returns True if a frame has been handled
 	 */
 	virtual bool handle_frame(const std::string &buffer, ws_opcode opcode);

include/dpp/discordvoiceclient.h

@@ -854,6 +854,7 @@ class DPP_EXPORT discord_voice_client : public websocket_client
 	/**
 	 * @brief Handle JSON from the websocket.
 	 * @param buffer The entire buffer content from the websocket client
+	 * @param opcode Frame type, e.g. OP_TEXT, OP_BINARY
 	 * @return bool True if a frame has been handled
 	 * @throw dpp::exception If there was an error processing the frame, or connection to UDP socket failed
 	 */
@@ -1132,7 +1133,7 @@ class DPP_EXPORT discord_voice_client : public websocket_client
 	std::string discover_ip();
 
 	/**
-	 * Returns true if end-to-end encryption is enabled
+	 * @brief Returns true if end-to-end encryption is enabled
 	 * for the active voice call (Discord Audio Visual
 	 * Encryption, a.k.a. DAVE).
 	 *
@@ -1141,7 +1142,7 @@ class DPP_EXPORT discord_voice_client : public websocket_client
 	bool is_end_to_end_encrypted() const;
 
 	/**
-	 * Returns the privacy code for the end to end encryption
+	 * @brief Returns the privacy code for the end to end encryption
 	 * scheme ("DAVE"). if end-to-end encryption is not active,
 	 * or is not yet established, this will return an empty
 	 * string.
@@ -1153,7 +1154,7 @@ class DPP_EXPORT discord_voice_client : public websocket_client
 	std::string get_privacy_code() const;
 
 	/**
-	 * Returns the privacy code for a given user by id,
+	 * @brief Returns the privacy code for a given user by id,
 	 * if they are in the voice call, and enc-to-end encryption
 	 * is enabled.
 	 *

include/dpp/wsclient.h

@@ -54,7 +54,7 @@ enum ws_state : uint8_t {
 	HTTP_HEADERS,
 
 	/**
-	 * @brief Connected as a websocket, and "upgraded". now talking using binary frames.
+	 * @brief Connected as a websocket, and "upgraded". Now talking using binary frames.
 	 */
 	CONNECTED
 };
@@ -193,6 +193,8 @@ class DPP_EXPORT websocket_client : public ssl_client {
 	/**
 	 * @brief Write to websocket. Encapsulates data in frames if the status is CONNECTED.
 	 * @param data The data to send.
+	 * @param _opcode The opcode of the data to send, either binary or text. The default
+	 * is to use the socket's opcode as set in the constructor.
 	 */
 	virtual void write(const std::string_view data, ws_opcode _opcode = OP_AUTO);
 

library-vcpkg/CMakeLists.txt

@@ -56,7 +56,7 @@ if(WIN32)
 	add_compile_definitions(WIN32_LEAN_AND_MEAN)
 	add_compile_definitions(_CRT_SECURE_NO_WARNINGS)
 	add_compile_definitions(_CRT_NONSTDC_NO_DEPRECATE)
-	# For MLSPP
+	# Fake an ssl version number to satisfy MLSPP
 	set(OPENSSL_VERSION "1.1.1f")
 endif()
 

library/CMakeLists.txt

@@ -49,7 +49,7 @@ add_compile_definitions(AVX_TYPE=${AVX_TYPE})
 add_compile_options(${AVX_FLAG})
 
 if(WIN32 AND NOT MINGW)
-	# For MLSPP
+	# Fake an ssl version number to satisfy MLSPP
 	set(OPENSSL_VERSION "1.1.1f")
 	if (NOT WINDOWS_32_BIT)
 		message("-- Building for windows with precompiled packaged dependencies")

src/davetest/dave.cpp

@@ -21,9 +21,7 @@
  ************************************************************************************/
 #include <dpp/dpp.h>
 #include <iostream>
-#include <cstring>
 #include <string>
-#include "../../src/dpp/dave/common.h"
 
 std::string get_testdata_dir() {
 	char *env_var = getenv("TEST_DATA_DIR");

src/dpp/dave/clock.h

@@ -66,7 +66,7 @@ class clock_interface {
 class clock : public clock_interface {
 public:
 	/**
-	 * Get current time
+	 * @brief Get current time
 	 * @return current time
 	 */
 	time_point now() const override {

src/dpp/dave/codec_utils.cpp

@@ -90,9 +90,7 @@ constexpr uint8_t kH26XNaluShortStartSequenceSize = 3;
 
 using IndexStartCodeSizePair = std::pair<size_t, size_t>;
 
-std::optional<IndexStartCodeSizePair> FindNextH26XNaluIndex(const uint8_t* buffer,
-															const size_t bufferSize,
-															const size_t searchStartIndex = 0)
+std::optional<IndexStartCodeSizePair> FindNextH26XNaluIndex(const uint8_t* buffer, const size_t bufferSize, const size_t searchStartIndex = 0)
 {
 	constexpr uint8_t kH26XStartCodeHighestPossibleValue = 1;
 	constexpr uint8_t kH26XStartCodeEndByteValue = 1;

src/dpp/dave/codec_utils.h

@@ -36,55 +36,55 @@
 namespace dpp::dave::codec_utils {
 
 /**
- * process opus audio frame
+ * @brief process opus audio frame
  * @param processor outbound frame processor
  * @param frame frame bytes
  * @return true if frame could be processed
  */
 bool process_frame_opus(outbound_frame_processor & processor, array_view<const uint8_t> frame);
 
 /**
- * process VP8 video frame
+ * @brief process VP8 video frame
  * @param processor outbound frame processor
  * @param frame frame bytes
  * @return true if frame could be processed
  */
 bool process_frame_vp8(outbound_frame_processor & processor, array_view<const uint8_t> frame);
 
 /**
- * process VP9 video frame
+ * @brief process VP9 video frame
  * @param processor outbound frame processor
  * @param frame frame bytes
  * @return true if frame could be processed
  */
 bool process_frame_vp9(outbound_frame_processor & processor, array_view<const uint8_t> frame);
 
 /**
- * process H264 video frame
+ * @brief process H264 video frame
  * @param processor outbound frame processor
  * @param frame frame bytes
  * @return true if frame could be processed
  */
 bool process_frame_h264(outbound_frame_processor & processor, array_view<const uint8_t> frame);
 
 /**
- * process opus frame
+ * @brief process opus frame
  * @param processor outbound frame processor
  * @param frame frame bytes
  * @return true if frame could be processed
  */
 bool process_frame_h265(outbound_frame_processor & processor, array_view<const uint8_t> frame);
 
 /**
- * process opus frame
+ * @brief process opus frame
  * @param processor outbound frame processor
  * @param frame frame bytes
  * @return true if frame could be processed
  */
 bool process_frame_av1(outbound_frame_processor & processor, array_view<const uint8_t> frame);
 
 /**
- * Check if encrypted frame is valid
+ * @brief Check if encrypted frame is valid
  * @param processor outbound frame processor
  * @param frame frame to validate
  * @return true if frame could be validated

src/dpp/dave/cryptor_manager.h

@@ -60,7 +60,7 @@ big_nonce compute_wrapped_big_nonce(key_generation generation, truncated_sync_no
 /**
  * @brief A manager to handle whichever cipher is best for the current version of DAVE
  *
- * his will currently instantiate an AES 128 GCM AEAD cipher.
+ * This will currently instantiate an AES 128 GCM AEAD cipher.
  */
 class aead_cipher_manager {
 public:

src/dpp/dave/decryptor.h

@@ -103,7 +103,7 @@ class decryptor {
 					    duration transitionExpiry = DEFAULT_TRANSITION_EXPIRY);
 
 	/**
-	 * Decrypt a frame
+	 * @brief Decrypt a frame
 	 *
 	 * @param mediaType type of media, audio or video
 	 * @param encryptedFrame encrypted frame bytes
@@ -123,7 +123,7 @@ class decryptor {
 	size_t get_max_plaintext_byte_size(media_type mediaType, size_t encryptedFrameSize);
 
 	/**
-	 * Get decryption stats
+	 * @brief Get decryption stats
 	 * @param mediaType media type, audio or video
 	 * @return decryption stats
 	 */
@@ -144,10 +144,7 @@ class decryptor {
 	 * @param frame decrypted frame data
 	 * @return True if decryption succeeded
 	 */
-	bool decrypt_impl(aead_cipher_manager& cipher_manager,
-			  media_type mediaType,
-			  inbound_frame_processor& encryptedFrame,
-			  array_view<uint8_t> frame);
+	bool decrypt_impl(aead_cipher_manager& cipher_manager, media_type mediaType, inbound_frame_processor& encryptedFrame, array_view<uint8_t> frame);
 
 	/**
 	 * @brief Update expiry for an instance of the manager