Reconfigure default streams on AddRecvStream.

Makes sure RTX can be used for streams that have received early media
before being properly configured.

BUG=1788
R=mflodman@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/46499004

Cr-Commit-Position: refs/heads/master@{#8634}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8634 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/media/webrtc/webrtcvideoengine2.cc

@@ -276,7 +276,7 @@ DefaultUnsignalledSsrcHandler::DefaultUnsignalledSsrcHandler()
     : default_recv_ssrc_(0), default_renderer_(NULL) {}
 
 UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc(
-    VideoMediaChannel* channel,
+    WebRtcVideoChannel2* channel,
     uint32_t ssrc) {
   if (default_recv_ssrc_ != 0) {  // Already one default stream.
     LOG(LS_WARNING) << "Unknown SSRC, but default receive stream already set.";
@@ -286,7 +286,7 @@ UnsignalledSsrcHandler::Action DefaultUnsignalledSsrcHandler::OnUnsignalledSsrc(
   StreamParams sp;
   sp.ssrcs.push_back(ssrc);
   LOG(LS_INFO) << "Creating default receive stream for SSRC=" << ssrc << ".";
-  if (!channel->AddRecvStream(sp)) {
+  if (!channel->AddRecvStream(sp, true)) {
     LOG(LS_WARNING) << "Could not create default receive stream.";
   }
 
@@ -801,7 +801,7 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) {
   // ssrc.
   rtc::CritScope stream_lock(&stream_crit_);
   if (send_streams_.find(ssrc) != send_streams_.end()) {
-    LOG(LS_ERROR) << "Send stream with ssrc '" << ssrc << "' already exists.";
+    LOG(LS_ERROR) << "Send stream with SSRC '" << ssrc << "' already exists.";
     return false;
   }
 
@@ -875,6 +875,11 @@ bool WebRtcVideoChannel2::RemoveSendStream(uint32 ssrc) {
 }
 
 bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) {
+  return AddRecvStream(sp, false);
+}
+
+bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
+                                        bool default_stream) {
   LOG(LS_INFO) << "AddRecvStream: " << sp.ToString();
   assert(sp.ssrcs.size() > 0);
 
@@ -883,9 +888,17 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) {
 
   // TODO(pbos): Check if any of the SSRCs overlap.
   rtc::CritScope stream_lock(&stream_crit_);
-  if (receive_streams_.find(ssrc) != receive_streams_.end()) {
+  {
-    LOG(LS_ERROR) << "Receive stream for SSRC " << ssrc << "already exists.";
+    auto it = receive_streams_.find(ssrc);
-    return false;
+    if (it != receive_streams_.end()) {
+      if (default_stream || !it->second->IsDefaultStream()) {
+        LOG(LS_ERROR) << "Receive stream for SSRC '" << ssrc
+                      << "' already exists.";
+        return false;
+      }
+      delete it->second;
+      receive_streams_.erase(it);
+    }
   }
 
   webrtc::VideoReceiveStream::Config config;
@@ -902,8 +915,9 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) {
         static_cast<WebRtcVoiceMediaChannel*>(voice_channel_)->voe_channel();
   }
 
-  receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
+  receive_streams_[ssrc] =
-      call_.get(), external_decoder_factory_, config, recv_codecs_);
+      new WebRtcVideoReceiveStream(call_.get(), external_decoder_factory_,
+                                   default_stream, config, recv_codecs_);
 
   return true;
 }
@@ -1098,8 +1112,9 @@ void WebRtcVideoChannel2::OnPacketReceived(
     return;
   }
 
-  // TODO(pbos): Make sure that the unsignalled SSRC uses the video payload.
+  // TODO(pbos): Ignore unsignalled packets that don't use the video payload
-  // Also figure out whether RTX needs to be handled.
+  // (prevent creating default receivers for RTX configured as if it would
+  // receive media payloads on those SSRCs).
   switch (unsignalled_ssrc_handler_->OnUnsignalledSsrc(this, ssrc)) {
     case UnsignalledSsrcHandler::kDropPacket:
       return;
@@ -1857,10 +1872,12 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::RecreateWebRtcStream() {
 WebRtcVideoChannel2::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
     webrtc::Call* call,
     WebRtcVideoDecoderFactory* external_decoder_factory,
+    bool default_stream,
     const webrtc::VideoReceiveStream::Config& config,
     const std::vector<VideoCodecSettings>& recv_codecs)
     : call_(call),
       stream_(NULL),
+      default_stream_(default_stream),
       config_(config),
       external_decoder_factory_(external_decoder_factory),
       renderer_(NULL),
@@ -2004,6 +2021,10 @@ bool WebRtcVideoChannel2::WebRtcVideoReceiveStream::IsTextureSupported() const {
   return true;
 }
 
+bool WebRtcVideoChannel2::WebRtcVideoReceiveStream::IsDefaultStream() const {
+  return default_stream_;
+}
+
 void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRenderer(
     cricket::VideoRenderer* renderer) {
   rtc::CritScope crit(&renderer_lock_);

talk/media/webrtc/webrtcvideoengine2.h

@@ -81,7 +81,7 @@ class UnsignalledSsrcHandler {
     kDropPacket,
     kDeliverPacket,
   };
-  virtual Action OnUnsignalledSsrc(VideoMediaChannel* engine,
+  virtual Action OnUnsignalledSsrc(WebRtcVideoChannel2* channel,
                                    uint32_t ssrc) = 0;
 };
 
@@ -89,7 +89,8 @@ class UnsignalledSsrcHandler {
 class DefaultUnsignalledSsrcHandler : public UnsignalledSsrcHandler {
  public:
   DefaultUnsignalledSsrcHandler();
-  Action OnUnsignalledSsrc(VideoMediaChannel* engine, uint32_t ssrc) override;
+  Action OnUnsignalledSsrc(WebRtcVideoChannel2* channel,
+                           uint32_t ssrc) override;
 
   VideoRenderer* GetDefaultRenderer() const;
   void SetDefaultRenderer(VideoMediaChannel* channel, VideoRenderer* renderer);
@@ -197,6 +198,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
   bool AddSendStream(const StreamParams& sp) override;
   bool RemoveSendStream(uint32 ssrc) override;
   bool AddRecvStream(const StreamParams& sp) override;
+  bool AddRecvStream(const StreamParams& sp, bool default_stream);
   bool RemoveRecvStream(uint32 ssrc) override;
   bool SetRenderer(uint32 ssrc, VideoRenderer* renderer) override;
   bool GetStats(VideoMediaInfo* info) override;
@@ -387,6 +389,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
     WebRtcVideoReceiveStream(
         webrtc::Call*,
         WebRtcVideoDecoderFactory* external_decoder_factory,
+        bool default_stream,
         const webrtc::VideoReceiveStream::Config& config,
         const std::vector<VideoCodecSettings>& recv_codecs);
     ~WebRtcVideoReceiveStream();
@@ -397,6 +400,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
     void RenderFrame(const webrtc::I420VideoFrame& frame,
                      int time_to_render_ms) override;
     bool IsTextureSupported() const override;
+    bool IsDefaultStream() const;
 
     void SetRenderer(cricket::VideoRenderer* renderer);
     cricket::VideoRenderer* GetRenderer();
@@ -425,6 +429,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
     webrtc::Call* const call_;
 
     webrtc::VideoReceiveStream* stream_;
+    const bool default_stream_;
     webrtc::VideoReceiveStream::Config config_;
 
     WebRtcVideoDecoderFactory* const external_decoder_factory_;

talk/media/webrtc/webrtcvideoengine2_unittest.cc

@@ -333,8 +333,21 @@ void FakeCall::DestroyVideoReceiveStream(
 }
 
 webrtc::PacketReceiver* FakeCall::Receiver() {
-  // TODO(pbos): Fix this.
+  return this;
-  return NULL;
+}
+
+FakeCall::DeliveryStatus FakeCall::DeliverPacket(const uint8_t* packet,
+                                                 size_t length) {
+  CHECK(length >= 12);
+  uint32_t ssrc;
+  if (!GetRtpSsrc(packet, length, &ssrc))
+    return DELIVERY_PACKET_ERROR;
+
+  for (auto& receiver: video_receive_streams_) {
+    if (receiver->GetConfig().rtp.remote_ssrc == ssrc)
+        return DELIVERY_OK;
+  }
+  return DELIVERY_UNKNOWN_SSRC;
 }
 
 void FakeCall::SetStats(const webrtc::Call::Stats& stats) {
@@ -2349,6 +2362,37 @@ TEST_F(WebRtcVideoChannel2Test, TranslatesSenderBitrateStatsCorrectly) {
       << "Bandwidth stats should take all streams into account.";
 }
 
+TEST_F(WebRtcVideoChannel2Test, DefaultReceiveStreamReconfiguresToUseRtx) {
+  EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
+
+  const std::vector<uint32> ssrcs = MAKE_VECTOR(kSsrcs1);
+  const std::vector<uint32> rtx_ssrcs = MAKE_VECTOR(kRtxSsrcs1);
+
+  ASSERT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
+  const size_t kDataLength = 12;
+  uint8_t data[kDataLength];
+  memset(data, 0, sizeof(data));
+  rtc::SetBE32(&data[8], ssrcs[0]);
+  rtc::Buffer packet(data, kDataLength);
+  rtc::PacketTime packet_time;
+  channel_->OnPacketReceived(&packet, packet_time);
+
+  ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size())
+      << "No default receive stream created.";
+  FakeVideoReceiveStream* recv_stream = fake_call_->GetVideoReceiveStreams()[0];
+  EXPECT_EQ(0u, recv_stream->GetConfig().rtp.rtx.size())
+      << "Default receive stream should not have configured RTX";
+
+  EXPECT_TRUE(channel_->AddRecvStream(
+      cricket::CreateSimWithRtxStreamParams("cname", ssrcs, rtx_ssrcs)));
+  ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size())
+      << "AddRecvStream should've reconfigured, not added a new receiver.";
+  recv_stream = fake_call_->GetVideoReceiveStreams()[0];
+  ASSERT_EQ(1u, recv_stream->GetConfig().rtp.rtx.size());
+  EXPECT_EQ(rtx_ssrcs[0],
+            recv_stream->GetConfig().rtp.rtx.begin()->second.ssrc);
+}
+
 class WebRtcVideoEngine2SimulcastTest : public testing::Test {
  public:
   WebRtcVideoEngine2SimulcastTest()

talk/media/webrtc/webrtcvideoengine2_unittest.h

@@ -99,7 +99,7 @@ class FakeVideoReceiveStream : public webrtc::VideoReceiveStream {
   webrtc::VideoReceiveStream::Stats stats_;
 };
 
-class FakeCall : public webrtc::Call {
+class FakeCall : public webrtc::Call, public webrtc::PacketReceiver {
  public:
   FakeCall(const webrtc::Call::Config& config);
   ~FakeCall();
@@ -135,6 +135,7 @@ class FakeCall : public webrtc::Call {
   void DestroyVideoReceiveStream(
       webrtc::VideoReceiveStream* receive_stream) override;
   webrtc::PacketReceiver* Receiver() override;
+  DeliveryStatus DeliverPacket(const uint8_t* packet, size_t length) override;
 
   webrtc::Call::Stats GetStats() const override;