Merge pull request #1670 from private-octopus/media-test-down

Fix BBR probe UP and Probe RTT
private-octopus · Apr 13, 2024 · 0a8731b · 0a8731b
2 parents 75a85d6 + f6eb300
commit 0a8731b
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 8 deletions.
diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp
@@ -2255,6 +2255,12 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(mediatest_video2_back) {
+            int ret = mediatest_video2_back_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(mediatest_wifi) {
             int ret = mediatest_wifi_test();
 

diff --git a/picoquic/bbr.c b/picoquic/bbr.c
@@ -1251,6 +1251,7 @@ static void BBRUpdateMinRTT(picoquic_bbr_state_t* bbr_state, picoquic_path_t* pa
 static void BBRExitProbeRTT(picoquic_bbr_state_t* bbr_state, picoquic_path_t * path_x, uint64_t current_time)
 {
     BBRResetLowerBounds(bbr_state);
+    path_x->rtt_min = bbr_state->min_rtt;
     if (bbr_state->filled_pipe) {
         BBRStartProbeBW_DOWN(bbr_state, path_x, current_time);
         BBRStartProbeBW_CRUISE(bbr_state);
@@ -1303,7 +1304,6 @@ static void BBRHandleProbeRTT(picoquic_bbr_state_t* bbr_state, picoquic_path_t *
     }
 }
 
-
 static void BBREnterProbeRTT(picoquic_bbr_state_t* bbr_state)
 {
     bbr_state->state = picoquic_bbr_alg_probe_rtt;
@@ -1317,6 +1317,7 @@ static void BBRCheckProbeRTT(picoquic_bbr_state_t* bbr_state, picoquic_path_t *
         bbr_state->probe_rtt_expired &&
         !bbr_state->idle_restart) {
         BBREnterProbeRTT(bbr_state);
+        bbr_state->min_rtt = rs->rtt_sample;
         bbr_state->prior_cwnd = BBRSaveCwnd(bbr_state, path_x);
         bbr_state->probe_rtt_done_stamp = 0;
         bbr_state->ack_phase = picoquic_bbr_acks_probe_stopping;
@@ -1579,6 +1580,7 @@ static void BBRStartProbeBW_DOWN(picoquic_bbr_state_t* bbr_state, picoquic_path_
     bbr_state->ack_phase = picoquic_bbr_acks_probe_stopping;
     BBRStartRound(bbr_state, path_x);
     bbr_state->state = picoquic_bbr_alg_probe_bw_down;
+    bbr_state->nb_rtt_excess = 0;
 }
 
 static void BBRStartProbeBW_CRUISE(picoquic_bbr_state_t* bbr_state)
@@ -1603,6 +1605,7 @@ static void BBRStartProbeBW_REFILL(picoquic_bbr_state_t* bbr_state, picoquic_pat
 
 static void BBRStartProbeBW_UP(picoquic_bbr_state_t* bbr_state, picoquic_path_t * path_x, uint64_t current_time)
 {
+    bbr_state->nb_rtt_excess = 0;
     bbr_state->pacing_gain = BBRProbeBwUpPacingGain;  /* pace at rate */
     bbr_state->cwnd_gain = BBRProbeBwUpCwndGain;   /* maintain cwnd */
     bbr_state->ack_phase = picoquic_bbr_acks_probe_starting;
@@ -1669,7 +1672,8 @@ static void BBRUpdateProbeBWCyclePhase(picoquic_bbr_state_t* bbr_state, picoquic
 
     case picoquic_bbr_alg_probe_bw_up:
         if (BBRHasElapsedInPhase(bbr_state, bbr_state->min_rtt, current_time) &&
-            path_x->bytes_in_transit > BBRInflightWithBw(bbr_state, path_x, 1.25, bbr_state->max_bw)) {
+            (bbr_state->nb_rtt_excess > 0 ||
+            path_x->bytes_in_transit > BBRInflightWithBw(bbr_state, path_x, 1.25, bbr_state->max_bw))) {
             BBRStartProbeBW_DOWN(bbr_state, path_x, current_time);
         }
         break;

diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c
@@ -394,6 +394,7 @@ static const picoquic_test_def_t test_table[] = {
     { "mediatest_video_audio", mediatest_video_audio_test },
     { "mediatest_video_data_audio", mediatest_video_data_audio_test },
     { "mediatest_video2_down", mediatest_video2_down_test },
+    { "mediatest_video2_back", mediatest_video2_back_test },
     { "mediatest_wifi", mediatest_wifi_test },
     { "mediatest_worst", mediatest_worst_test },
     { "warptest_video", warptest_video_test },

diff --git a/picoquictest/mediatest.c b/picoquictest/mediatest.c
@@ -67,7 +67,8 @@ typedef enum {
     mediatest_video_data_audio = 3,
     mediatest_worst = 4,
     mediatest_video2_down = 5,
-    mediatest_wifi = 6
+    mediatest_wifi = 6,
+    mediatest_video2_back = 7
 } mediatest_id_enum;
 
 typedef enum {
@@ -347,14 +348,20 @@ int mediatest_check_stats(mediatest_ctx_t* mt_ctx, mediatest_spec_t * spec, medi
             if (spec->latency_average == 0)
             {
                 if (average > 25000 || sigma > 12500 || stats->max_delay > 100000) {
+                    DBG_PRINTF("Latency average: %" PRIu64 ", sigma: %" PRIu64 ", max: %" PRIu64,
+                        average, sigma, stats->max_delay);
                     ret = -1;
                 }
             }
             else {
                 if (average > spec->latency_average) {
+                    DBG_PRINTF("Average latency expected: %" PRIu64 ", got %" PRIu64,
+                        spec->latency_average, average);
                     ret = -1;
                 }
                 else if (spec->latency_max > 0 && stats->max_delay > spec->latency_max) {
+                    DBG_PRINTF("Max latency expected: %" PRIu64 ", got %" PRIu64,
+                        spec->latency_max, stats->max_delay);
                     ret = -1;
                 }
             }
@@ -1154,7 +1161,7 @@ int mediatest_one(mediatest_id_enum media_test_id, mediatest_spec_t * spec)
     if (mt_ctx == NULL) {
         ret = -1;
     }
-    /* Three special cases in which we manipualte the configuration
+    /* Three special cases in which we manipulate the configuration
     * to simulate various downgrade or suspension patterns.
      */
     if (media_test_id == mediatest_worst) {
@@ -1168,20 +1175,23 @@ int mediatest_one(mediatest_id_enum media_test_id, mediatest_spec_t * spec)
         }
     }
 
-    if (media_test_id == mediatest_video2_down) {
+    if (media_test_id == mediatest_video2_down ||
+        media_test_id == mediatest_video2_back) {
         uint64_t picosec_per_byte_ref[2];
         uint64_t latency_ref[2];
+        uint64_t down_time = (media_test_id == mediatest_video2_down) ? 4000000 : 2000000;
+        uint64_t back_time = (media_test_id == mediatest_video2_down) ? 24000000 : 4000000;
 
         /* Run the simulation for 2 second. */
-        ret = mediatest_loop(mt_ctx, 2000000, 0, &is_finished);
-        /* Drop the bandwidth and increase latency for 4 seconds */
+        ret = mediatest_loop(mt_ctx, down_time, 0, &is_finished);
+        /* Drop the bandwidth and increase latency for specified down time */
         for (int i = 0; i < 2; i++) {
             picosec_per_byte_ref[i] = mt_ctx->link[i]->picosec_per_byte;
             mt_ctx->link[i]->picosec_per_byte = 8000000; /* 8 us per byte, i.e., 1Mbps*/
             latency_ref[i] = mt_ctx->link[i]->microsec_latency;
         }
         if (ret == 0) {
-            ret = mediatest_loop(mt_ctx, 6000000, 0, &is_finished);
+            ret = mediatest_loop(mt_ctx, back_time, 0, &is_finished);
         }
         /* restore the bandwidth */
         for (int i = 0; i < 2; i++) {
@@ -1324,6 +1334,24 @@ int mediatest_video2_down_test()
     return ret;
 }
 
+int mediatest_video2_back_test()
+{
+    int ret;
+    mediatest_spec_t spec = { 0 };
+    spec.ccalgo = picoquic_bbr_algorithm;
+    spec.bandwidth = 0.01;
+    spec.do_video = 1;
+    spec.do_video2 = 1;
+    spec.do_audio = 1;
+    spec.data_size = 0;
+    spec.latency_average = 110000;
+    spec.latency_max = 600000;
+    spec.do_not_check_video2 = 1;
+    ret = mediatest_one(mediatest_video2_back, &spec);
+
+    return ret;
+}
+
 int mediatest_worst_test()
 {
     int ret;

diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h
@@ -388,6 +388,7 @@ int mediatest_video_test();
 int mediatest_video_audio_test();
 int mediatest_video_data_audio_test();
 int mediatest_video2_down_test();
+int mediatest_video2_back_test();
 int mediatest_wifi_test();
 int mediatest_worst_test();
 int warptest_video_test();