Support N unsignaled audio streams

webrtc/media/engine/webrtcvoiceengine_unittest.cc

 /*
  *  Copyright (c) 2008 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 2722 matching lines @@
   EXPECT_EQ(s1.received_packets(), 1);
   EXPECT_EQ(s2.received_packets(), 1);
   EXPECT_EQ(s3.received_packets(), 1);
   EXPECT_TRUE(s3.VerifyLastPacket(packets[3], sizeof(packets[3])));
 
   EXPECT_TRUE(channel_->RemoveRecvStream(ssrc3));
   EXPECT_TRUE(channel_->RemoveRecvStream(ssrc2));
   EXPECT_TRUE(channel_->RemoveRecvStream(ssrc1));
 }
 
-// Test that receiving on an unsignalled stream works (default channel will be
-// created).
-TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignalled) {
+// Test that receiving on an unsignaled stream works (a stream is created).
+TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignaled) {
   EXPECT_TRUE(SetupChannel());
   EXPECT_EQ(0, call_.GetAudioReceiveStreams().size());
 
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
 
   EXPECT_EQ(1, call_.GetAudioReceiveStreams().size());
   EXPECT_TRUE(GetRecvStream(1).VerifyLastPacket(kPcmuFrame,
                                                 sizeof(kPcmuFrame)));
 }
 
-// Test that receiving on an unsignalled stream works (default channel will be
-// created), and that packets will be forwarded to the default channel
-// regardless of their SSRCs.
-TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignalledWithSsrcSwitch) {
+// Test that receiving N unsignaled stream works (streams will be created), and
+// that packets are forwarded to them all.
+TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignaledWithSsrcSwitch) {
   EXPECT_TRUE(SetupChannel());
   unsigned char packet[sizeof(kPcmuFrame)];
   memcpy(packet, kPcmuFrame, sizeof(kPcmuFrame));
 
-  // Note that ssrc = 0 is not supported.
-  uint32_t ssrc = 1;
-  for (; ssrc < 10; ++ssrc) {
+  constexpr uint32_t kMaxUnsignaledCount = 50;
+
+  // Note that SSRC = 0 is not supported.
+  for (uint32_t ssrc = 1; ssrc < (1 + kMaxUnsignaledCount); ++ssrc) {
     rtc::SetBE32(&packet[8], ssrc);
     DeliverPacket(packet, sizeof(packet));
 
-    // Verify we only have one default stream.
-    EXPECT_EQ(1, call_.GetAudioReceiveStreams().size());
+    // Verify we have one new stream for each loop iteration.
+    EXPECT_EQ(ssrc, call_.GetAudioReceiveStreams().size());
     EXPECT_EQ(1, GetRecvStream(ssrc).received_packets());
     EXPECT_TRUE(GetRecvStream(ssrc).VerifyLastPacket(packet, sizeof(packet)));
   }
 
-  // Sending the same ssrc again should not create a new stream.
-  --ssrc;
+  // Send on the same SSRCs again should not create new streams.

[Taylor Brandstetter, 2017/02/17 08:36:15]

nit: "Sending"

[the sun, 2017/02/17 10:10:57]

Done.

+  for (uint32_t ssrc = 1; ssrc < (1 + kMaxUnsignaledCount); ++ssrc) {
+    rtc::SetBE32(&packet[8], ssrc);
+    DeliverPacket(packet, sizeof(packet));
+
+    EXPECT_EQ(kMaxUnsignaledCount, call_.GetAudioReceiveStreams().size());
+    EXPECT_EQ(2, GetRecvStream(ssrc).received_packets());
+    EXPECT_TRUE(GetRecvStream(ssrc).VerifyLastPacket(packet, sizeof(packet)));
+  }
+
+  // Send on another SSRC, the oldest unsignaled stream (SSRC=1) is replaced.
+  constexpr uint32_t kAnotherSsrc = 667;
+  rtc::SetBE32(&packet[8], kAnotherSsrc);
   DeliverPacket(packet, sizeof(packet));
-  EXPECT_EQ(1, call_.GetAudioReceiveStreams().size());
-  EXPECT_EQ(2, GetRecvStream(ssrc).received_packets());
-  EXPECT_TRUE(GetRecvStream(ssrc).VerifyLastPacket(packet, sizeof(packet)));
+
+  const auto& streams = call_.GetAudioReceiveStreams();
+  EXPECT_EQ(kMaxUnsignaledCount, streams.size());
+  size_t i = 0;
+  for (uint32_t ssrc = 2; ssrc < (1 + kMaxUnsignaledCount); ++ssrc, ++i) {
+    EXPECT_EQ(ssrc, streams[i]->GetConfig().rtp.remote_ssrc);
+    EXPECT_EQ(2, streams[i]->received_packets());
+  }
+  EXPECT_EQ(kAnotherSsrc, streams[i]->GetConfig().rtp.remote_ssrc);
+  EXPECT_EQ(1, streams[i]->received_packets());
+  ++i;
+  EXPECT_EQ(kMaxUnsignaledCount, i);

[Taylor Brandstetter, 2017/02/17 08:36:15]

What's the purpose of the final ++i and EXPECT_EQ? Is it a just a sanity check
that every stream was counted? If so could you leave a comment saying that?

[the sun, 2017/02/17 10:10:57]

Yes, basically to "complete" the loop

 }
 
 // Test that a default channel is created even after a signalled stream has been
 // added, and that this stream will get any packets for unknown SSRCs.
-TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignalledAfterSignalled) {
+TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignaledAfterSignalled) {

[Taylor Brandstetter, 2017/02/17 08:36:15]

nit: This uses both "Unsignaled" and "Unsignalled"

[the sun, 2017/02/17 10:10:57]

Done.

   EXPECT_TRUE(SetupChannel());
   unsigned char packet[sizeof(kPcmuFrame)];
   memcpy(packet, kPcmuFrame, sizeof(kPcmuFrame));
 
   // Add a known stream, send packet and verify we got it.
   const uint32_t signaled_ssrc = 1;
   rtc::SetBE32(&packet[8], signaled_ssrc);
   EXPECT_TRUE(AddRecvStream(signaled_ssrc));
   DeliverPacket(packet, sizeof(packet));
   EXPECT_TRUE(GetRecvStream(signaled_ssrc).VerifyLastPacket(
@@ skipping 487 matching lines @@
   EXPECT_TRUE(SetupChannel());
   EXPECT_FALSE(channel_->SetOutputVolume(kSsrc2, 0.5));
   cricket::StreamParams stream;
   stream.ssrcs.push_back(kSsrc2);
   EXPECT_TRUE(channel_->AddRecvStream(stream));
   EXPECT_DOUBLE_EQ(1, GetRecvStream(kSsrc2).gain());
   EXPECT_TRUE(channel_->SetOutputVolume(kSsrc2, 3));
   EXPECT_DOUBLE_EQ(3, GetRecvStream(kSsrc2).gain());
 }
 
-TEST_F(WebRtcVoiceEngineTestFake, SetOutputVolumeDefaultRecvStream) {
+TEST_F(WebRtcVoiceEngineTestFake, SetOutputVolumeUnsignaledRecvStream) {
   EXPECT_TRUE(SetupChannel());
+
+  unsigned char pcmuFrame2[sizeof(kPcmuFrame)];
+  memcpy(pcmuFrame2, kPcmuFrame, sizeof(kPcmuFrame));
+  rtc::SetBE32(&pcmuFrame2[8], kSsrc4);
+
+  // Spawn an unsignaled stream by sending a packet - gain should be 1.
+  DeliverPacket(pcmuFrame2, sizeof(pcmuFrame2));
+  EXPECT_DOUBLE_EQ(1, GetRecvStream(kSsrc4).gain());
+
+  // Should remember the volume "2" which will be set on new unsignaled streams,
+  // and also set the gain to 2 on existing unsignaled streams.
   EXPECT_TRUE(channel_->SetOutputVolume(0, 2));
+  EXPECT_DOUBLE_EQ(2, GetRecvStream(kSsrc4).gain());
+
+  // Spawn an unsignaled stream by sending a packet - gain should be 2.
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));

[Taylor Brandstetter, 2017/02/17 08:36:14]

nit: May be more readable if "kPcmuFrame2" was delivered after "kPcmuFrame"

[the sun, 2017/02/17 10:10:57]

Done.

   EXPECT_DOUBLE_EQ(2, GetRecvStream(1).gain());
+
+  // Setting gain with SSRC=0 should affect all unsignaled streams.
   EXPECT_TRUE(channel_->SetOutputVolume(0, 3));
+  EXPECT_DOUBLE_EQ(3, GetRecvStream(kSsrc4).gain());
   EXPECT_DOUBLE_EQ(3, GetRecvStream(1).gain());
+
+  // Setting gain on an individual stream affects only that.
   EXPECT_TRUE(channel_->SetOutputVolume(1, 4));
+  EXPECT_DOUBLE_EQ(3, GetRecvStream(kSsrc4).gain());
   EXPECT_DOUBLE_EQ(4, GetRecvStream(1).gain());
 }
 
 TEST_F(WebRtcVoiceEngineTestFake, SetsSyncGroupFromSyncLabel) {
   const uint32_t kAudioSsrc = 123;
   const std::string kSyncLabel = "AvSyncLabel";
 
   EXPECT_TRUE(SetupSendStream());
   cricket::StreamParams sp = cricket::StreamParams::CreateLegacy(kAudioSsrc);
   sp.sync_label = kSyncLabel;
@@ skipping 132 matching lines @@
 
   // Now try actually setting the sink.
   channel_->SetRawAudioSink(kSsrc1, std::move(fake_sink_2));
   EXPECT_NE(nullptr, GetRecvStream(kSsrc1).sink());
 
   // Now try resetting it.
   channel_->SetRawAudioSink(kSsrc1, nullptr);
   EXPECT_EQ(nullptr, GetRecvStream(kSsrc1).sink());
 }
 
-TEST_F(WebRtcVoiceEngineTestFake, SetRawAudioSinkDefaultRecvStream) {
+TEST_F(WebRtcVoiceEngineTestFake, SetRawAudioSinkUnsignaledRecvStream) {
   EXPECT_TRUE(SetupChannel());
   std::unique_ptr<FakeAudioSink> fake_sink_1(new FakeAudioSink());
   std::unique_ptr<FakeAudioSink> fake_sink_2(new FakeAudioSink());
 
   // Should be able to set a default sink even when no stream exists.
   channel_->SetRawAudioSink(0, std::move(fake_sink_1));
 
   // Create default channel and ensure it's assigned the default sink.
   DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
   EXPECT_NE(nullptr, GetRecvStream(0x01).sink());
@@ skipping 290 matching lines @@
   // Without this cast, the comparison turned unsigned and, thus, failed for -1.
   const int num_specs = static_cast<int>(specs.size());
   EXPECT_GE(find_codec({"cn", 8000, 1}), num_specs);
   EXPECT_GE(find_codec({"cn", 16000, 1}), num_specs);
   EXPECT_EQ(find_codec({"cn", 32000, 1}), -1);
   EXPECT_GE(find_codec({"telephone-event", 8000, 1}), num_specs);
   EXPECT_GE(find_codec({"telephone-event", 16000, 1}), num_specs);
   EXPECT_GE(find_codec({"telephone-event", 32000, 1}), num_specs);
   EXPECT_GE(find_codec({"telephone-event", 48000, 1}), num_specs);
 }