Adding cbr support for Opus

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

Index: webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
index 1d4fc2e7813c9c3b2eee1bc6596be2ace20e47f6..50df9152901c0d632aa4e2af9fe3fc087378af84 100644
--- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
+++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc
@@ -546,4 +546,55 @@ TEST(AudioEncoderOpusTest, EncodeAtMinBitrate) {
   }
 }
 
+TEST(AudioEncoderOpusTest, EncodeCbr) {

[minyue-webrtc, 2017/03/29 20:45:26]

I am not sure that we have to test Cbr is Cbr in AudioEncoderOpusTest.

On AudioEncoderOpus level, we want to make sure that SetCbr can be called
properly, Take ToggleDtx as an example.

But definitely need this test on the opus_interface level. See opus_unittest.cc

[squashingskak, 2017/03/30 08:25:54]

Main reason to do the test is to check that enable actually does enable cbr. The
first time I ran the test I saw that I had reversed the logic so SetCbr(true)
was disabeling cbr. Maybe it is not needed in a unit test. It is your call if
you think it can be removed.

+  auto states = CreateCodec(1);
+  constexpr int kNumRuns = 4;
+  constexpr int kNumPacketsPerRun = 20;
+  constexpr int kNumPacketsToEncode = kNumPacketsPerRun * kNumRuns;
+  auto audio_frames =
+  Create10msAudioBlocks(states.encoder, kNumPacketsToEncode * 20);

[minyue-webrtc, 2017/03/29 20:45:26]

needs indent. you can do

git cl format

+  ASSERT_TRUE(audio_frames) << "Create10msAudioBlocks failed";
+  rtc::Buffer encoded;
+  uint32_t rtp_timestamp = 12345;  // Just a number not important to this test.
+
+  for(int run = 0; run < kNumRuns; run++){
+    int32_t max_pkt_size_diff = 0;
+    int32_t prev_pkt_size = 0;
+    bool use_cbr = (run & 1) ? true : false;

[minyue-webrtc, 2017/03/29 20:45:26]

are you trying combinations of {use_cbr, bitrate}?

I hope you do it more explicitly

say (ignore syntax)

set<bool> cbr_set = {true, false}
set<int> bitrate_set = {0, 10000}
for (bool cbr : cbr_set) {
  for (int bitrate : bitrate_set) {
  }
}

[kwiberg-webrtc, 2017/03/29 21:54:17]

Nowadays, it should be legal to do simply

  for (const bool cbr : {true, false}) {
    for (const int bitrate : {0, 10000}) {
      ...
    }
  }

+    int bitrate = (run & 2) ? 100000 : 0;
+
+    states.encoder->SetCbr(use_cbr);
+    states.encoder->OnReceivedUplinkBandwidth(bitrate,
+                                              rtc::Optional<int64_t>());
+    for (int packet_index = 0; packet_index < kNumPacketsPerRun;
+         packet_index++) {
+      // Make sure we are not encoding before we have enough data for
+      // a 20ms packet.
+      for (int index = 0; index < 1; index++) {
+        states.encoder->Encode(rtp_timestamp, audio_frames->GetNextBlock(),
+                               &encoded);
+        EXPECT_EQ(0u, encoded.size());
+      }
+
+      // Should encode now.
+      states.encoder->Encode(rtp_timestamp, audio_frames->GetNextBlock(),
+                                &encoded);
+      EXPECT_GT(encoded.size(), 0u);
+
+      if(prev_pkt_size > 0){
+        int32_t diff = std::abs((int32_t)encoded.size() - prev_pkt_size);
+        max_pkt_size_diff = std::max(max_pkt_size_diff, diff);
+      }
+      prev_pkt_size = encoded.size();
+
+      encoded.Clear();
+    }
+    if(use_cbr){
+      EXPECT_EQ(max_pkt_size_diff, 0);
+    } else {
+      EXPECT_GT(max_pkt_size_diff, 0);
+    }
+  }
+}
+
 }  // namespace webrtc