Adding cbr support for Opus

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 528 matching lines @@
     }
 
     // Should encode now.
     states.encoder->Encode(rtp_timestamp, audio_frames->GetNextBlock(),
                            &encoded);
     EXPECT_GT(encoded.size(), 0u);
     encoded.Clear();
   }
 }
 
+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