Base A/V synchronization on sync_labels.

talk/media/webrtc/webrtcvoiceengine_unittest.cc

Index: talk/media/webrtc/webrtcvoiceengine_unittest.cc
diff --git a/talk/media/webrtc/webrtcvoiceengine_unittest.cc b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
index 0dc7b510eea7d6eaa62c9deee2f35a8d313b514a..77f437c60a615746a2be74789bec0322795915a9 100644
--- a/talk/media/webrtc/webrtcvoiceengine_unittest.cc
+++ b/talk/media/webrtc/webrtcvoiceengine_unittest.cc
@@ -3353,6 +3353,34 @@ TEST(WebRtcVoiceEngineTest, CoInitialize) {
 }
 #endif
 
+TEST_F(WebRtcVoiceEngineTestFake, SetsSyncGroupFromSyncLabel) {
+  cricket::FakeCall call(webrtc::Call::Config(nullptr));
+  const uint32 kAudioSsrc = 123;
+  const std::string kSyncLabel = "AvSyncLabel";
+
+  EXPECT_TRUE(SetupEngine());
+  cricket::WebRtcVoiceMediaChannel* media_channel =
+      static_cast<cricket::WebRtcVoiceMediaChannel*>(channel_);
+  media_channel->SetCall(&call);
+  cricket::StreamParams sp = cricket::StreamParams::CreateLegacy(kAudioSsrc);
+  sp.sync_label = kSyncLabel;
+  // Creating two channels to make sure that sync label is set properly for both
+  // the default voice channel and following ones.
+  EXPECT_TRUE(channel_->AddRecvStream(sp));
+  sp.ssrcs[0] += 1;
+  EXPECT_TRUE(channel_->AddRecvStream(sp));
+
+  ASSERT_EQ(2, call.GetAudioReceiveStreams().size());
+  EXPECT_EQ(kSyncLabel,
+            call.GetAudioReceiveStream(kAudioSsrc)->GetConfig().sync_group)
+      << "SyncGroup should be set based on sync_label";
+  EXPECT_EQ(kSyncLabel,
+            call.GetAudioReceiveStream(kAudioSsrc + 1)->GetConfig().sync_group)
+      << "SyncGroup should be set based on sync_label";
+
+  media_channel->SetCall(nullptr);
+}
+
 TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedBweOption_Call) {
   // Test that changing the combined_audio_video_bwe option results in the
   // expected state changes on an associated Call.
@@ -3363,27 +3391,45 @@ TEST_F(WebRtcVoiceEngineTestFake, ChangeCombinedBweOption_Call) {
   EXPECT_TRUE(SetupEngine());
   cricket::WebRtcVoiceMediaChannel* media_channel =
       static_cast<cricket::WebRtcVoiceMediaChannel*>(channel_);
+  const auto& rtp_extensions = engine_.rtp_header_extensions();
+  media_channel->SetRecvRtpHeaderExtensions(rtp_extensions);
   media_channel->SetCall(&call);
   EXPECT_TRUE(media_channel->AddRecvStream(
       cricket::StreamParams::CreateLegacy(kAudioSsrc1)));
   EXPECT_TRUE(media_channel->AddRecvStream(
       cricket::StreamParams::CreateLegacy(kAudioSsrc2)));
 
-  // Combined BWE should not be set up yet.
-  EXPECT_EQ(0, call.GetAudioReceiveStreams().size());
+  // Combined BWE should not be set up yet (no RTP extensions).
+  EXPECT_EQ(2, call.GetAudioReceiveStreams().size());
+  EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc1)
+                  ->GetConfig()
+                  .rtp.extensions.empty());
+  EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc2)
+                  ->GetConfig()
+                  .rtp.extensions.empty());
 
   // Enable combined BWE option - now it should be set up.
   cricket::AudioOptions options;
   options.combined_audio_video_bwe.Set(true);
   EXPECT_TRUE(media_channel->SetOptions(options));
   EXPECT_EQ(2, call.GetAudioReceiveStreams().size());
-  EXPECT_NE(nullptr, call.GetAudioReceiveStream(kAudioSsrc1));
-  EXPECT_NE(nullptr, call.GetAudioReceiveStream(kAudioSsrc2));
+  EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc1)
+                   ->GetConfig()
+                   .rtp.extensions.empty());
+  EXPECT_FALSE(call.GetAudioReceiveStream(kAudioSsrc2)
+                   ->GetConfig()
+                   .rtp.extensions.empty());
 
   // Disable combined BWE option - should be disabled again.
   options.combined_audio_video_bwe.Set(false);
   EXPECT_TRUE(media_channel->SetOptions(options));
-  EXPECT_EQ(0, call.GetAudioReceiveStreams().size());
+  EXPECT_EQ(2, call.GetAudioReceiveStreams().size());
+  EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc1)
+                  ->GetConfig()
+                  .rtp.extensions.empty());
+  EXPECT_TRUE(call.GetAudioReceiveStream(kAudioSsrc2)
+                  ->GetConfig()
+                  .rtp.extensions.empty());

[hta-webrtc, 2015/07/06 19:04:00]

Drive-by comment: This test doesn't seem very future-proof. The combined BWE
option may be the only RTP extension we use *today*, but what happens the day
the "mid" RTP option goes live?

[pbos-webrtc, 2015/07/09 10:21:49]

Yes, this isn't meant to be future proof, it's supposed to be non-optional.
Before this CL we never created audio receive streams if there wasn't combined
BWE, so this was a way to get stream-based A/V sync in at all.

 
   media_channel->SetCall(nullptr);
 }
@@ -3620,4 +3666,3 @@ TEST_F(WebRtcVoiceEngineTestFake, AssociateChannelResetUponDeleteChannnel) {
   EXPECT_TRUE(channel_->RemoveSendStream(2));
   EXPECT_EQ(voe_.GetAssociateSendChannel(recv_ch), -1);
 }
-