Bitrate limits changed to be resolution-specific

- Absolute bitrate limits changed to be bitrate_per_pixel so that its dependent on resolution
- Starting bitrate message is sent from client via TCP to ensure server starts at a specific bitrate
- Removed the logic for minQP, since min_bitrate_per_pixel will ensure that there is no unbearable pixelation
- Removed the VBV based throttling logic on the video encode since the minQP is now removed
- Video bitrate to consider NUM_PREV_AUDIO_FRAMES_RESEND
- NUM_PREV_AUDIO_FRAMES_RESEND reduced from 2 to 1, to reduce audio bitrate. Anyways 1 serves the purpose of redundancy.

protocol/client/client_utils.c

@@ -48,8 +48,8 @@ Bad Globals [TODO: Remove these or give them `static`!]
 volatile char client_binary_aes_private_key[16];
 volatile char client_hex_aes_private_key[33];
 volatile char *server_ip;
-volatile int output_width;
-volatile int output_height;
+extern volatile int output_width;
+extern volatile int output_height;
 volatile char *program_name = NULL;
 volatile SDL_Window *window;
 

protocol/client/network.c

@@ -272,7 +272,7 @@ int send_wcmsg(WhistClientMessage *wcmsg) {
     // before adding/removing from this list
     if (wcmsg->type == MESSAGE_DISCOVERY_REQUEST || wcmsg->type == CMESSAGE_FILE_DATA ||
         wcmsg->type == CMESSAGE_FILE_METADATA || wcmsg->type == CMESSAGE_CLIPBOARD ||
-        (size_t)wcmsg_size > sizeof(*wcmsg)) {
+        wcmsg->type == MESSAGE_DIMENSIONS || (size_t)wcmsg_size > sizeof(*wcmsg)) {
         return send_packet(&packet_tcp_context, PACKET_MESSAGE, wcmsg, wcmsg_size, -1, false);
     } else {
         if ((size_t)wcmsg_size > MAX_PACKET_SIZE) {

protocol/server/audio.c

@@ -42,12 +42,6 @@ Includes
 #include "state.h"
 #include "server_statistic.h"
 
-// Number of audio previous frames that will be resent along with the current frame.
-// Resending of audio is done pro-actively as audio packet loss doesn't have enough time for client
-// to send nack and recover. Since audio bitrate is just 128Kbps, the extra bandwidth used for
-// resending audio packets is still acceptable.
-#define NUM_PREVIOUS_FRAMES_RESEND 2
-
 #ifdef _WIN32
 #pragma comment(lib, "ws2_32.lib")
 #endif
@@ -137,7 +131,7 @@ int32_t multithreaded_send_audio(void* opaque) {
                                 id, false);
                             // Simulate nacks to trigger re-sending of previous frames.
                             // TODO: Move into udp.c
-                            for (int i = 1; i <= NUM_PREVIOUS_FRAMES_RESEND && id - i > 0; i++) {
+                            for (int i = 1; i <= NUM_PREV_AUDIO_FRAMES_RESEND && id - i > 0; i++) {
                                 // Audio is always only one UDP packet per audio frame.
                                 // Average bytes per audio frame = (Samples_per_frame * Bitrate) /
                                 //                                 (BITS_IN_BYTE * Sampling freq)

protocol/server/video.c

@@ -489,19 +489,12 @@ int32_t multithreaded_send_video(void* opaque) {
 
     NetworkSettings last_network_settings = {0};
 
-    NetworkThrottleContext* network_throttler =
-        network_throttler_create((double)VBV_BUF_SIZE_IN_MS, true);
-    // Don't bitrate limit in the beginning
-    network_throttler_set_burst_bitrate(network_throttler, 100000000);
-
     // Create producer-consumer semaphore pair with queue size of 1
     producer = whist_create_semaphore(0);
     consumer = whist_create_semaphore(1);
     WhistThread video_send_packets = whist_create_thread(multithreaded_send_video_packets,
                                                          "multithreaded_send_video_packets", state);
 
-    int last_frame_size = 0;
-
     int consecutive_identical_frames = 0;
 
     bool assuming_client_active = false;
@@ -510,16 +503,12 @@ int32_t multithreaded_send_video(void* opaque) {
            (state->client_width == -1 || state->client_height == -1 || state->client_dpi == -1)) {
         whist_sleep(1);
     }
+    udp_handle_network_settings(
+        state->client.udp_context.context,
+        get_default_network_settings(state->client_width, state->client_height));
     bool encoder_running = false;
 
     while (!state->exiting) {
-        // If we sent a frame, throttle the bytes of the last frame,
-        // before doing anything else
-        if (last_frame_size > 0) {
-            network_throttler_wait_byte_allocation(network_throttler, last_frame_size);
-            last_frame_size = 0;
-        }
-
         update_client_active_status(&state->client, &assuming_client_active);
 
         if (!assuming_client_active) {
@@ -554,12 +543,11 @@ int32_t multithreaded_send_video(void* opaque) {
             }
         }
 
-        NetworkSettings network_settings =
-            state->client.is_active ? udp_get_network_settings(&state->client.udp_context)
-                                    : default_network_settings;
+        NetworkSettings network_settings = udp_get_network_settings(&state->client.udp_context);
 
         int video_bitrate =
-            (network_settings.bitrate - AUDIO_BITRATE) * (1.0 - network_settings.video_fec_ratio);
+            (network_settings.bitrate - (NUM_PREV_AUDIO_FRAMES_RESEND + 1) * AUDIO_BITRATE) *
+            (1.0 - network_settings.video_fec_ratio);
         FATAL_ASSERT(video_bitrate > 0);
         CodecType video_codec = network_settings.desired_codec;
         // TODO: Use video_fps instead of max_fps, also see update_video_encoder when doing this
@@ -576,8 +564,6 @@ int32_t multithreaded_send_video(void* opaque) {
             start_timer(&statistics_timer);
             encoder =
                 update_video_encoder(state, encoder, device, video_bitrate, video_codec, video_fps);
-            // Update throttler bitrate too
-            network_throttler_set_burst_bitrate(network_throttler, video_bitrate);
             log_double_statistic(VIDEO_ENCODER_UPDATE_TIME,
                                  get_timer(&statistics_timer) * MS_IN_SECOND);
         }
@@ -735,8 +721,6 @@ int32_t multithreaded_send_video(void* opaque) {
                         send_populated_frames(state, &statistics_timer, &server_frame_timer, device,
                                               encoder, id, client_input_timestamp,
                                               server_timestamp);
-                        // Remember the last frame size, so that we can throttle the bitrate
-                        last_frame_size = encoder->encoded_frame_size;
 
                         log_double_statistic(VIDEO_FPS_SENT, 1.0);
                         log_double_statistic(VIDEO_FRAME_SIZE, encoder->encoded_frame_size);
@@ -766,7 +750,6 @@ int32_t multithreaded_send_video(void* opaque) {
         destroy_capture_device(device);
         device = NULL;
     }
-    network_throttler_destroy(network_throttler);
 
     return 0;
 }

protocol/whist/core/whist.c

@@ -323,6 +323,7 @@ int get_wcmsg_size(WhistClientMessage *wcmsg) {
         return (int)(sizeof(*wcmsg) + strlen((const char *)&wcmsg->url_to_open) + 1);
     } else {
         // Send small wcmsg's when we don't need unnecessarily large ones
+        // TODO : Remove this hardcoded value.
         return sizeof(wcmsg->type) + sizeof(wcmsg->id) + 40;
     }
 }

protocol/whist/core/whist.h

@@ -163,9 +163,6 @@ Defines
 // VBV Buffer size in milliseconds
 #define VBV_BUF_SIZE_IN_MS 1000
 
-#define MAX_QP 40                  // Max QP value of video frames
-#define MAX_INTRA_QP (MAX_QP + 6)  // Higher QP value for intra frames if supported by the encoder
-
 #define OUTPUT_WIDTH 1280
 #define OUTPUT_HEIGHT 720
 
@@ -189,6 +186,12 @@ Defines
 
 #define AUDIO_FREQUENCY 48000
 
+// Number of audio previous frames that will be resent along with the current frame.
+// Resending of audio is done pro-actively as audio packet loss doesn't have enough time for client
+// to send nack and recover. Since audio bitrate is just 128Kbps, the extra bandwidth used for
+// resending audio packets is still acceptable.
+#define NUM_PREV_AUDIO_FRAMES_RESEND 1
+
 /*
 ============================
 Constants

protocol/whist/network/network_algorithm.c

@@ -27,25 +27,57 @@ Includes
 
 #include "network_algorithm.h"
 
+/*
+============================
+Globals
+============================
+*/
+
+volatile int output_width;
+volatile int output_height;
+
 /*
 ============================
 Defines
 ============================
 */
 
-const NetworkSettings default_network_settings = {
-    .bitrate = STARTING_BITRATE,
-    .burst_bitrate = STARTING_BURST_BITRATE,
+// bitrate and burst_bitrate doesn't have any compile-time default values as it depends on the
+// resolution
+static NetworkSettings default_network_settings = {
     .desired_codec = CODEC_TYPE_H264,
     .audio_fec_ratio = AUDIO_FEC_RATIO,
     .video_fec_ratio = VIDEO_FEC_RATIO,
     .fps = 60,
 };
 
-#define BAD_BITRATE 10400000
-#define BAD_BURST_BITRATE 31800000
 #define EWMA_STATS_SECONDS 5
 
+// The below values are roughly based on the "Low Bitrate" and "High Bitrate" values recommended in
+// https://www.videoproc.com/media-converter/bitrate-setting-for-h264.htm. Also confirmed visually
+// that these values produce reasonable output quality.
+#define MINIMUM_BITRATE_PER_PIXEL 1.0
+#define MAXIMUM_BITRATE_PER_PIXEL 4.0
+#define STARTING_BITRATE_PER_PIXEL 3.0
+
+// The value of 6 is chosen based on previously used constant values of STARTING_BURST_BITRATE_RAW,
+// STARTING_BITRATE_RAW. This is chosen, just for the sake of maintaining the status-quo.
+#define BURST_BITRATE_RATIO 6
+
+#define TOTAL_AUDIO_BITRATE ((NUM_PREV_AUDIO_FRAMES_RESEND + 1) * AUDIO_BITRATE)
+
+#define MINIMUM_VIDEO_BITRATE (output_width * output_height * MINIMUM_BITRATE_PER_PIXEL)
+#define MAXIMUM_VIDEO_BITRATE (output_width * output_height * MAXIMUM_BITRATE_PER_PIXEL)
+#define STARTING_VIDEO_BITRATE (output_width * output_height * STARTING_BITRATE_PER_PIXEL)
+
+#define MINIMUM_BITRATE (MINIMUM_VIDEO_BITRATE + TOTAL_AUDIO_BITRATE)
+#define MAXIMUM_BITRATE (MAXIMUM_VIDEO_BITRATE + TOTAL_AUDIO_BITRATE)
+#define STARTING_BITRATE (STARTING_VIDEO_BITRATE + TOTAL_AUDIO_BITRATE)
+
+#define MINIMUM_BURST_BITRATE (MINIMUM_BITRATE * BURST_BITRATE_RATIO)
+#define MAXIMUM_BURST_BITRATE (MAXIMUM_BITRATE * BURST_BITRATE_RATIO)
+#define STARTING_BURST_BITRATE (STARTING_BITRATE * BURST_BITRATE_RATIO)
+
 /*
 ============================
 Private Function Declarations
@@ -55,8 +87,6 @@ Private Function Declarations
 // These functions involve various potential
 // implementations of `calculate_new_bitrate`
 
-NetworkSettings fallback_bitrate(NetworkStatistics stats);
-NetworkSettings ewma_bitrate(NetworkStatistics stats);
 NetworkSettings ewma_ratio_bitrate(NetworkStatistics stats);
 NetworkSettings timed_ewma_ratio_bitrate(NetworkStatistics stats);
 
@@ -66,6 +96,15 @@ Public Function Implementations
 ============================
 */
 
+NetworkSettings get_default_network_settings(int width, int height) {
+    FATAL_ASSERT(width > 0 && height > 0);
+    int video_bitrate = width * height * STARTING_BITRATE_PER_PIXEL;
+    int starting_bitrate = video_bitrate + TOTAL_AUDIO_BITRATE;
+    default_network_settings.bitrate = starting_bitrate;
+    default_network_settings.burst_bitrate = starting_bitrate * BURST_BITRATE_RATIO;
+    return default_network_settings;
+}
+
 NetworkSettings get_desired_network_settings(NetworkStatistics stats) {
     // Get the network settings we want, based on those statistics
     NetworkSettings network_settings = timed_ewma_ratio_bitrate(stats);
@@ -98,48 +137,6 @@ Private Function Implementations
 ============================
 */
 
-NetworkSettings fallback_bitrate(NetworkStatistics stats) {
-    /*
-        Switches between two sets of bitrate/burst bitrate: the default of 16mbps/100mbps and a
-        fallback of 10mbps/30mbps. We fall back if we've nacked a lot in the last second.
-    */
-    static NetworkSettings network_settings;
-    if (stats.num_nacks_per_second > 6) {
-        network_settings.bitrate = BAD_BITRATE;
-        network_settings.burst_bitrate = BAD_BURST_BITRATE;
-    } else {
-        network_settings.bitrate = STARTING_BITRATE;
-        network_settings.burst_bitrate = STARTING_BURST_BITRATE;
-    }
-    return network_settings;
-}
-
-NetworkSettings ewma_bitrate(NetworkStatistics stats) {
-    /*
-        Keeps an exponentially weighted moving average of the throughput per second the client is
-        getting, and uses that to predict a good bitrate to ask the server for.
-    */
-
-    // Constants
-    const double alpha = 0.8;
-    const double bitrate_throughput_ratio = 1.25;
-
-    FATAL_ASSERT(stats.throughput_per_second >= 0);
-
-    // Calculate throughput
-    static int throughput = -1;
-    if (throughput == -1) {
-        throughput = (int)(STARTING_BITRATE / bitrate_throughput_ratio);
-    }
-    throughput = (int)(alpha * throughput + (1 - alpha) * stats.throughput_per_second);
-
-    // Set network settings
-    NetworkSettings network_settings = default_network_settings;
-    network_settings.bitrate = (int)(bitrate_throughput_ratio * throughput);
-    network_settings.burst_bitrate = STARTING_BURST_BITRATE;
-    return network_settings;
-}
-
 NetworkSettings timed_ewma_ratio_bitrate(NetworkStatistics stats) {
     /* This is a stopgap to account for a recent change that calls the ewma_ratio_bitrate
        at a frequency for which it was not designed for. This should eventually be replaced
@@ -149,11 +146,11 @@ NetworkSettings timed_ewma_ratio_bitrate(NetworkStatistics stats) {
 
     static WhistTimer interval_timer;
     static bool timer_initialized = false;
-    static NetworkSettings network_settings = {.bitrate = STARTING_BITRATE,
-                                               .burst_bitrate = STARTING_BURST_BITRATE};
+    static NetworkSettings network_settings;
 
     if (!timer_initialized) {
         start_timer(&interval_timer);
+        network_settings = get_default_network_settings(output_width, output_height);
         timer_initialized = true;
     }
 

protocol/whist/network/network_algorithm.h

@@ -27,19 +27,6 @@ Defines
 ============================
 */
 
-// Max/Min/Starting Bitrates/Burst Bitrates
-
-#define MAXIMUM_BITRATE 30000000
-#define MINIMUM_BITRATE 2000000
-#define STARTING_BITRATE_RAW 15400000
-#define STARTING_BITRATE (min(max(STARTING_BITRATE_RAW, MINIMUM_BITRATE), MAXIMUM_BITRATE))
-
-#define MAXIMUM_BURST_BITRATE 200000000
-#define MINIMUM_BURST_BITRATE 4000000
-#define STARTING_BURST_BITRATE_RAW 100000000
-#define STARTING_BURST_BITRATE \
-    (min(max(STARTING_BURST_BITRATE_RAW, MINIMUM_BURST_BITRATE), MAXIMUM_BURST_BITRATE))
-
 // The FEC Ratio to use on video/audio packets respectively
 // (Only used for testing phase of FEC)
 // This refers to the percentage of packets that will be FEC packets
@@ -54,8 +41,6 @@ typedef struct {
     int throughput_per_second;
 } NetworkStatistics;
 
-extern const NetworkSettings default_network_settings;
-
 /*
 ============================
 Public Functions
@@ -73,4 +58,16 @@ Public Functions
  */
 NetworkSettings get_desired_network_settings(NetworkStatistics stats);
 
+/**
+ * @brief               This function will return the default network settings for a given video
+ *                      resolution
+ *
+ * @param width         Video width
+ *
+ * @param height        Video height
+ *
+ * @returns             A network settings struct
+ */
+NetworkSettings get_default_network_settings(int width, int height);
+
 #endif

protocol/whist/network/throttle.c

@@ -9,6 +9,10 @@
 
 #define BITS_IN_BYTE 8.0
 
+// Set this to something very low. Throttler will work as expected only if
+// network_throttler_set_burst_bitrate() is called with the required bitrate
+#define STARTING_THROTTLER_BITRATE 1000000
+
 struct NetworkThrottleContext {
     double coin_bucket_ms;   //<<< The size of the coin bucket in milliseconds.
     size_t coin_bucket_max;  //<<< The maximum size of the coin bucket for the current burst bitrate
@@ -34,14 +38,14 @@ NetworkThrottleContext* network_throttler_create(double coin_bucket_ms,
     */
     NetworkThrottleContext* ctx = safe_malloc(sizeof(NetworkThrottleContext));
     ctx->coin_bucket_ms = coin_bucket_ms;
-    ctx->coin_bucket_max =
-        (size_t)((ctx->coin_bucket_ms / MS_IN_SECOND) * (STARTING_BURST_BITRATE / BITS_IN_BYTE));
+    ctx->coin_bucket_max = (size_t)((ctx->coin_bucket_ms / MS_IN_SECOND) *
+                                    (STARTING_THROTTLER_BITRATE / BITS_IN_BYTE));
     if (fill_bucket_initially) {
         ctx->coin_bucket = ctx->coin_bucket_max;
     } else {
         ctx->coin_bucket = 0;
     }
-    ctx->burst_bitrate = STARTING_BURST_BITRATE;
+    ctx->burst_bitrate = STARTING_THROTTLER_BITRATE;
     ctx->queue_lock = whist_create_mutex();
     ctx->queue_cond = whist_create_cond();
     atomic_init(&ctx->next_queue_id, 0);