AudioEncoderOpusTest.PacketLossRateOptimized: Fix bug and make prettier

webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc

 /*
  *  Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 79 matching lines @@
   EXPECT_EQ(kMaxBitrateBps, encoder_->GetTargetBitrate());
   // Set rates from 1000 up to 32000 bps.
   for (int rate = 1000; rate <= 32000; rate += 1000) {
     encoder_->SetTargetBitrate(rate);
     EXPECT_EQ(rate, encoder_->GetTargetBitrate());
   }
 }
 
 namespace {
 
-// These constants correspond to those used in
-// AudioEncoderOpus::SetProjectedPacketLossRate.
-const double kPacketLossRate20 = 0.20;
-const double kPacketLossRate10 = 0.10;
-const double kPacketLossRate5 = 0.05;
-const double kPacketLossRate1 = 0.01;
-const double kLossRate20Margin = 0.02;
-const double kLossRate10Margin = 0.01;
-const double kLossRate5Margin = 0.01;
+// If a < b, returns a vector with the n evenly-spaced numbers min(a,b) + eps,

[minyue-webrtc, 2015/09/09 11:40:29]

do you add/minus eps in this function?

[kwiberg-webrtc, 2015/09/09 11:51:42]

I used to, but then I changed it. Patch set #2 changes this comment to match.

+// ..., max(a,b) - eps. If a > b, returns the same vector reversed.
+std::vector<double> IntervalSteps(double a, double b, size_t n) {
+  DCHECK_GT(n, 1u);
+  if (a > b) {
+    auto r = IntervalSteps(b, a, n);
+    std::reverse(r.begin(), r.end());
+    return r;
+  }
+  DCHECK_LT(a, b);
+  const double step = (b - a) / (n - 1);
+  DCHECK_GT(step, 0.0);
+  std::vector<double> points;
+  for (size_t i = 0; i < n; ++i)
+    points.push_back(std::min(b, std::max(a, a + i * step)));
+  return points;

[minyue-webrtc, 2015/09/09 11:40:29]

does't returning std::vector involves a copy ctor? which might be less
efficient.

[kwiberg-webrtc, 2015/09/09 11:51:42]

Yes, in principle. But (1) this is a test, so we care much more about
readability than performance; (2) the lists only contain 10 doubles each, so
copying them isn't that expensive anyway; and (3) the compiler will use NVRO and
eliminate the copy (see e.g.
http://en.cppreference.com/w/cpp/language/copy_elision).

[minyue-webrtc, 2015/09/09 12:13:39]

Acknowledged.

+}
 
-// Repeatedly sets packet loss rates in the range [from, to], increasing by
-// 0.01 in each step. The function verifies that the actual loss rate is
-// |expected_return|.
+// Sets the packet loss rate to each number in the vector in turn, and verifies
+// that the loss rate as reported by the encoder is |expected_return| for all
+// of them.
 void TestSetPacketLossRate(AudioEncoderOpus* encoder,
-                           double from,
-                           double to,
+                           const std::vector<double>& losses,
                            double expected_return) {
-  for (double loss = from; loss <= to;

[minyue-webrtc, 2015/09/09 11:40:29]

all right, this was silly. Nice pick!

-       (to >= from) ? loss += 0.01 : loss -= 0.01) {
+  for (double loss : losses) {
     encoder->SetProjectedPacketLossRate(loss);
     EXPECT_DOUBLE_EQ(expected_return, encoder->packet_loss_rate());
   }
 }
 
 }  // namespace
 
 TEST_F(AudioEncoderOpusTest, PacketLossRateOptimized) {
   CreateCodec(1);
+  auto I = [](double a, double b) { return IntervalSteps(a, b, 10); };
+  const double eps = 1e-15;
 
   // Note that the order of the following calls is critical.
-  TestSetPacketLossRate(encoder_.get(), 0.0, 0.0, 0.0);
-  TestSetPacketLossRate(encoder_.get(), kPacketLossRate1,
-                        kPacketLossRate5 + kLossRate5Margin - 0.01,
-                        kPacketLossRate1);
-  TestSetPacketLossRate(encoder_.get(), kPacketLossRate5 + kLossRate5Margin,
-                        kPacketLossRate10 + kLossRate10Margin - 0.01,
-                        kPacketLossRate5);
-  TestSetPacketLossRate(encoder_.get(), kPacketLossRate10 + kLossRate10Margin,
-                        kPacketLossRate20 + kLossRate20Margin - 0.01,
-                        kPacketLossRate10);
-  TestSetPacketLossRate(encoder_.get(), kPacketLossRate20 + kLossRate20Margin,
-                        1.0, kPacketLossRate20);
-  TestSetPacketLossRate(encoder_.get(), kPacketLossRate20 + kLossRate20Margin,
-                        kPacketLossRate20 - kLossRate20Margin,
-                        kPacketLossRate20);
-  TestSetPacketLossRate(
-      encoder_.get(), kPacketLossRate20 - kLossRate20Margin - 0.01,
-      kPacketLossRate10 - kLossRate10Margin, kPacketLossRate10);
-  TestSetPacketLossRate(encoder_.get(),
-                        kPacketLossRate10 - kLossRate10Margin - 0.01,
-                        kPacketLossRate5 - kLossRate5Margin, kPacketLossRate5);
-  TestSetPacketLossRate(encoder_.get(),
-                        kPacketLossRate5 - kLossRate5Margin - 0.01,
-                        kPacketLossRate1, kPacketLossRate1);
-  TestSetPacketLossRate(encoder_.get(), 0.0, 0.0, 0.0);
+
+  // clang-format off
+  TestSetPacketLossRate(encoder_.get(), I(0.00      , 0.01 - eps), 0.00);
+  TestSetPacketLossRate(encoder_.get(), I(0.01 + eps, 0.06 - eps), 0.01);
+  TestSetPacketLossRate(encoder_.get(), I(0.06 + eps, 0.11 - eps), 0.05);
+  TestSetPacketLossRate(encoder_.get(), I(0.11 + eps, 0.22 - eps), 0.10);
+  TestSetPacketLossRate(encoder_.get(), I(0.22 + eps, 1.00      ), 0.20);
+
+  TestSetPacketLossRate(encoder_.get(), I(1.00      , 0.18 + eps), 0.20);
+  TestSetPacketLossRate(encoder_.get(), I(0.18 - eps, 0.09 + eps), 0.10);
+  TestSetPacketLossRate(encoder_.get(), I(0.09 - eps, 0.04 + eps), 0.05);
+  TestSetPacketLossRate(encoder_.get(), I(0.04 - eps, 0.01 + eps), 0.01);
+  TestSetPacketLossRate(encoder_.get(), I(0.01 - eps, 0.00      ), 0.00);
+  // clang-format on
 }
 
 }  // namespace webrtc