Be less pessimistic about turning "default" receive streams into signaled streams.

BUG=webrtc:7179, b/34746131

Review-Url: https://codereview.webrtc.org/2685573003
Cr-Commit-Position: refs/heads/master@{#16673}

webrtc/media/engine/fakewebrtccall.cc

@@ -21,7 +21,8 @@
 
 namespace cricket {
 FakeAudioSendStream::FakeAudioSendStream(
-    const webrtc::AudioSendStream::Config& config) : config_(config) {
+    int id, const webrtc::AudioSendStream::Config& config)
+    : id_(id), config_(config) {
   RTC_DCHECK(config.voe_channel_id != -1);
 }
 
@@ -59,8 +60,8 @@ webrtc::AudioSendStream::Stats FakeAudioSendStream::GetStats() const {
 }
 
 FakeAudioReceiveStream::FakeAudioReceiveStream(
-    const webrtc::AudioReceiveStream::Config& config)
-    : config_(config) {
+    int id, const webrtc::AudioReceiveStream::Config& config)
+    : id_(id), config_(config) {
   RTC_DCHECK(config.voe_channel_id != -1);
 }
 
@@ -404,7 +405,8 @@ webrtc::NetworkState FakeCall::GetNetworkState(webrtc::MediaType media) const {
 
 webrtc::AudioSendStream* FakeCall::CreateAudioSendStream(
     const webrtc::AudioSendStream::Config& config) {
-  FakeAudioSendStream* fake_stream = new FakeAudioSendStream(config);
+  FakeAudioSendStream* fake_stream = new FakeAudioSendStream(next_stream_id_++,
+                                                             config);
   audio_send_streams_.push_back(fake_stream);
   ++num_created_send_streams_;
   return fake_stream;
@@ -424,7 +426,8 @@ void FakeCall::DestroyAudioSendStream(webrtc::AudioSendStream* send_stream) {
 
 webrtc::AudioReceiveStream* FakeCall::CreateAudioReceiveStream(
     const webrtc::AudioReceiveStream::Config& config) {
-  audio_receive_streams_.push_back(new FakeAudioReceiveStream(config));
+  audio_receive_streams_.push_back(new FakeAudioReceiveStream(next_stream_id_++,
+                                                              config));
   ++num_created_receive_streams_;
   return audio_receive_streams_.back();
 }

webrtc/media/engine/fakewebrtccall.h

@@ -43,8 +43,10 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream {
     int duration_ms = 0;
   };
 
-  explicit FakeAudioSendStream(const webrtc::AudioSendStream::Config& config);
+  explicit FakeAudioSendStream(
+      int id, const webrtc::AudioSendStream::Config& config);
 
+  int id() const { return id_; }
   const webrtc::AudioSendStream::Config& GetConfig() const;
   void SetStats(const webrtc::AudioSendStream::Stats& stats);
   TelephoneEvent GetLatestTelephoneEvent() const;
@@ -61,6 +63,7 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream {
   void SetMuted(bool muted) override;
   webrtc::AudioSendStream::Stats GetStats() const override;
 
+  int id_ = -1;
   TelephoneEvent latest_telephone_event_;
   webrtc::AudioSendStream::Config config_;
   webrtc::AudioSendStream::Stats stats_;
@@ -71,8 +74,9 @@ class FakeAudioSendStream final : public webrtc::AudioSendStream {
 class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream {
  public:
   explicit FakeAudioReceiveStream(
-      const webrtc::AudioReceiveStream::Config& config);
+      int id, const webrtc::AudioReceiveStream::Config& config);
 
+  int id() const { return id_; }
   const webrtc::AudioReceiveStream::Config& GetConfig() const;
   void SetStats(const webrtc::AudioReceiveStream::Stats& stats);
   int received_packets() const { return received_packets_; }
@@ -93,6 +97,7 @@ class FakeAudioReceiveStream final : public webrtc::AudioReceiveStream {
   void SetSink(std::unique_ptr<webrtc::AudioSinkInterface> sink) override;
   void SetGain(float gain) override;
 
+  int id_ = -1;
   webrtc::AudioReceiveStream::Config config_;
   webrtc::AudioReceiveStream::Stats stats_;
   int received_packets_ = 0;
@@ -293,6 +298,7 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver {
   webrtc::NetworkState video_network_state_;
   rtc::SentPacket last_sent_packet_;
   int last_sent_nonnegative_packet_id_ = -1;
+  int next_stream_id_ = 665;
   webrtc::Call::Stats stats_;
   std::vector<FakeVideoSendStream*> video_send_streams_;
   std::vector<FakeAudioSendStream*> audio_send_streams_;

webrtc/media/engine/webrtcvoiceengine.cc

@@ -1609,6 +1609,14 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
     RecreateAudioReceiveStream();
   }
 
+  void MaybeRecreateAudioReceiveStream(const std::string& sync_group) {
+    RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+    if (config_.sync_group != sync_group) {
+      config_.sync_group = sync_group;
+      RecreateAudioReceiveStream();
+    }
+  }
+
   webrtc::AudioReceiveStream::Stats GetStats() const {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     RTC_DCHECK(stream_);
@@ -2276,10 +2284,13 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
     return false;
   }
 
-  // Remove the default receive stream if one had been created with this ssrc;
-  // we'll recreate it then.
+  // If the default receive stream was created with this ssrc, we unmark it as
+  // being the default stream, and possibly recreate the AudioReceiveStream, if
+  // sync_label has changed.
   if (IsDefaultRecvStream(ssrc)) {
-    RemoveRecvStream(ssrc);
+    recv_streams_[ssrc]->MaybeRecreateAudioReceiveStream(sp.sync_label);
+    default_recv_ssrc_ = -1;
+    return true;
   }
 
   if (GetReceiveChannelId(ssrc) != -1) {

webrtc/media/engine/webrtcvoiceengine_unittest.cc

@@ -2814,6 +2814,50 @@ TEST_F(WebRtcVoiceEngineTestFake, RecvUnsignalledAfterSignalled) {
   EXPECT_EQ(2, call_.GetAudioReceiveStreams().size());
 }
 
+// Two tests to verify that adding a receive stream with the same SSRC as a
+// previously added unsignaled stream will only recreate underlying stream
+// objects if the stream parameters have changed.
+TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_NoRecreate) {
+  EXPECT_TRUE(SetupChannel());
+
+  // Spawn unsignaled stream with SSRC=1.
+  DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
+  EXPECT_EQ(1, call_.GetAudioReceiveStreams().size());
+  EXPECT_TRUE(GetRecvStream(1).VerifyLastPacket(kPcmuFrame,
+                                                sizeof(kPcmuFrame)));
+
+  // Verify that the underlying stream object in Call is not recreated when a
+  // stream with SSRC=1 is added.
+  const auto& streams = call_.GetAudioReceiveStreams();
+  EXPECT_EQ(1, streams.size());
+  int audio_receive_stream_id = streams.front()->id();
+  EXPECT_TRUE(AddRecvStream(1));
+  EXPECT_EQ(1, streams.size());
+  EXPECT_EQ(audio_receive_stream_id, streams.front()->id());
+}
+
+TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamAfterUnsignaled_Recreate) {
+  EXPECT_TRUE(SetupChannel());
+
+  // Spawn unsignaled stream with SSRC=1.
+  DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
+  EXPECT_EQ(1, call_.GetAudioReceiveStreams().size());
+  EXPECT_TRUE(GetRecvStream(1).VerifyLastPacket(kPcmuFrame,
+                                                sizeof(kPcmuFrame)));
+
+  // Verify that the underlying stream object in Call *is* recreated when a
+  // stream with SSRC=1 is added, and which has changed stream parameters.
+  const auto& streams = call_.GetAudioReceiveStreams();
+  EXPECT_EQ(1, streams.size());
+  int audio_receive_stream_id = streams.front()->id();
+  cricket::StreamParams stream_params;
+  stream_params.ssrcs.push_back(1);
+  stream_params.sync_label = "sync_label";
+  EXPECT_TRUE(channel_->AddRecvStream(stream_params));
+  EXPECT_EQ(1, streams.size());
+  EXPECT_NE(audio_receive_stream_id, streams.front()->id());
+}
+
 // Test that we properly handle failures to add a receive stream.
 TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamFail) {
   EXPECT_TRUE(SetupChannel());