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
diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc
index e6f59d1..23c8a9d 100644
--- a/talk/media/webrtc/webrtcvideoengine2.cc
+++ b/talk/media/webrtc/webrtcvideoengine2.cc
@@ -276,7 +276,7 @@
: 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 @@
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 @@
// 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::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 @@
// 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.";
- return false;
+ {
+ auto it = receive_streams_.find(ssrc);
+ 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 @@
static_cast<WebRtcVoiceMediaChannel*>(voice_channel_)->voe_channel();
}
- receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
- call_.get(), external_decoder_factory_, config, recv_codecs_);
+ receive_streams_[ssrc] =
+ new WebRtcVideoReceiveStream(call_.get(), external_decoder_factory_,
+ default_stream, config, recv_codecs_);
return true;
}
@@ -1098,8 +1112,9 @@
return;
}
- // TODO(pbos): Make sure that the unsignalled SSRC uses the video payload.
- // Also figure out whether RTX needs to be handled.
+ // TODO(pbos): Ignore unsignalled packets that don't use the video payload
+ // (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 @@
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 @@
return true;
}
+bool WebRtcVideoChannel2::WebRtcVideoReceiveStream::IsDefaultStream() const {
+ return default_stream_;
+}
+
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRenderer(
cricket::VideoRenderer* renderer) {
rtc::CritScope crit(&renderer_lock_);
diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h
index b5d69c6..5dc1b04 100644
--- a/talk/media/webrtc/webrtcvideoengine2.h
+++ b/talk/media/webrtc/webrtcvideoengine2.h
@@ -81,7 +81,7 @@
kDropPacket,
kDeliverPacket,
};
- virtual Action OnUnsignalledSsrc(VideoMediaChannel* engine,
+ virtual Action OnUnsignalledSsrc(WebRtcVideoChannel2* channel,
uint32_t ssrc) = 0;
};
@@ -89,7 +89,8 @@
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 @@
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 @@
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 @@
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 @@
webrtc::Call* const call_;
webrtc::VideoReceiveStream* stream_;
+ const bool default_stream_;
webrtc::VideoReceiveStream::Config config_;
WebRtcVideoDecoderFactory* const external_decoder_factory_;
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
index b29fc51..4cf0462 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc
@@ -333,8 +333,21 @@
}
webrtc::PacketReceiver* FakeCall::Receiver() {
- // TODO(pbos): Fix this.
- return NULL;
+ return this;
+}
+
+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 @@
<< "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()
diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.h b/talk/media/webrtc/webrtcvideoengine2_unittest.h
index 7703535..7032dbe 100644
--- a/talk/media/webrtc/webrtcvideoengine2_unittest.h
+++ b/talk/media/webrtc/webrtcvideoengine2_unittest.h
@@ -99,7 +99,7 @@
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 @@
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;
