Add/remove receive streams with SSRC 0 from media channels
This enables creation and removal of receive streams with SSRC 0.
Several related methods, for example SetOutputVolume, still use 0 as a
special value.
Bug: webrtc:8694
Change-Id: I341e6bd6c981c9838997510d8d712ad2948f6460
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/152780
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Saurav Das <dinosaurav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#29398}
diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h
index 28af02a..ac303e6 100644
--- a/media/base/fake_media_engine.h
+++ b/media/base/fake_media_engine.h
@@ -117,6 +117,8 @@
}
return RemoveStreamBySsrc(&send_streams_, ssrc);
}
+ virtual void ResetUnsignaledRecvStream() {}
+
virtual bool AddRecvStream(const StreamParams& sp) {
if (absl::c_linear_search(receive_streams_, sp)) {
return false;
diff --git a/media/base/media_channel.h b/media/base/media_channel.h
index d6dfe70..8f6b04b 100644
--- a/media/base/media_channel.h
+++ b/media/base/media_channel.h
@@ -224,6 +224,8 @@
// ssrc must be the first SSRC of the media stream if the stream uses
// multiple SSRCs.
virtual bool RemoveRecvStream(uint32_t ssrc) = 0;
+ // Resets any cached StreamParams for an unsignaled RecvStream.
+ virtual void ResetUnsignaledRecvStream() = 0;
// Returns the absoulte sendtime extension id value from media channel.
virtual int GetRtpSendTimeExtnId() const;
// Set the frame encryptor to use on all outgoing frames. This is optional.
diff --git a/media/base/rtp_data_engine.cc b/media/base/rtp_data_engine.cc
index 33c87cb..6161085 100644
--- a/media/base/rtp_data_engine.cc
+++ b/media/base/rtp_data_engine.cc
@@ -194,6 +194,9 @@
return true;
}
+// Not implemented.
+void RtpDataMediaChannel::ResetUnsignaledRecvStream() {}
+
void RtpDataMediaChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet,
int64_t /* packet_time_us */) {
RtpHeader header;
diff --git a/media/base/rtp_data_engine.h b/media/base/rtp_data_engine.h
index 60d5f55..e5f071d 100644
--- a/media/base/rtp_data_engine.h
+++ b/media/base/rtp_data_engine.h
@@ -72,6 +72,7 @@
virtual bool RemoveSendStream(uint32_t ssrc);
virtual bool AddRecvStream(const StreamParams& sp);
virtual bool RemoveRecvStream(uint32_t ssrc);
+ virtual void ResetUnsignaledRecvStream();
virtual bool SetSend(bool send) {
sending_ = send;
return true;
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 10154d1..96a426d 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1325,7 +1325,6 @@
return false;
uint32_t ssrc = sp.first_ssrc();
- RTC_DCHECK(ssrc != 0); // TODO(pbos): Is this ever valid?
// Remove running stream if this was a default stream.
const auto& prev_stream = receive_streams_.find(ssrc);
@@ -1417,12 +1416,6 @@
bool WebRtcVideoChannel::RemoveRecvStream(uint32_t ssrc) {
RTC_DCHECK_RUN_ON(&thread_checker_);
RTC_LOG(LS_INFO) << "RemoveRecvStream: " << ssrc;
- if (ssrc == 0) {
- // This indicates that we need to remove the unsignaled stream parameters
- // that are cached.
- unsignaled_stream_params_ = StreamParams();
- return true;
- }
std::map<uint32_t, WebRtcVideoReceiveStream*>::iterator stream =
receive_streams_.find(ssrc);
@@ -1436,6 +1429,12 @@
return true;
}
+void WebRtcVideoChannel::ResetUnsignaledRecvStream() {
+ RTC_DCHECK_RUN_ON(&thread_checker_);
+ RTC_LOG(LS_INFO) << "ResetUnsignaledRecvStream.";
+ unsignaled_stream_params_ = StreamParams();
+}
+
bool WebRtcVideoChannel::SetSink(
uint32_t ssrc,
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink) {
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h
index 88956e9..b989e22 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -156,6 +156,7 @@
bool AddRecvStream(const StreamParams& sp) override;
bool AddRecvStream(const StreamParams& sp, bool default_stream);
bool RemoveRecvStream(uint32_t ssrc) override;
+ void ResetUnsignaledRecvStream() override;
bool SetSink(uint32_t ssrc,
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink) override;
void FillBitrateInfo(BandwidthEstimationInfo* bwe_info) override;
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index ca2d1a2..1ed3dc3 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -5533,9 +5533,9 @@
EXPECT_EQ(kSyncLabel,
fake_call_->GetVideoReceiveStreams()[0]->GetConfig().sync_group);
- // Removing the unsignaled stream should clear the cache. This time when
+ // Reset the unsignaled stream to clear the cache. This time when
// a default video receive stream is created it won't have a sync_group.
- ASSERT_TRUE(channel_->RemoveRecvStream(0));
+ channel_->ResetUnsignaledRecvStream();
ASSERT_TRUE(channel_->RemoveRecvStream(kIncomingUnsignalledSsrc));
EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index 195ed5b..bef9d23 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -1841,10 +1841,6 @@
}
const uint32_t ssrc = sp.first_ssrc();
- if (ssrc == 0) {
- RTC_DLOG(LS_WARNING) << "AddRecvStream with ssrc==0 is not supported.";
- return false;
- }
// If this stream was previously received unsignaled, we promote it, possibly
// recreating the AudioReceiveStream, if stream ids have changed.
@@ -1880,13 +1876,6 @@
RTC_DCHECK(worker_thread_checker_.IsCurrent());
RTC_LOG(LS_INFO) << "RemoveRecvStream: " << ssrc;
- if (ssrc == 0) {
- // This indicates that we need to remove the unsignaled stream parameters
- // that are cached.
- unsignaled_stream_params_ = StreamParams();
- return true;
- }
-
const auto it = recv_streams_.find(ssrc);
if (it == recv_streams_.end()) {
RTC_LOG(LS_WARNING) << "Try to remove stream with ssrc " << ssrc
@@ -1902,6 +1891,12 @@
return true;
}
+void WebRtcVoiceMediaChannel::ResetUnsignaledRecvStream() {
+ RTC_DCHECK(worker_thread_checker_.IsCurrent());
+ RTC_LOG(LS_INFO) << "ResetUnsignaledRecvStream.";
+ unsignaled_stream_params_ = StreamParams();
+}
+
bool WebRtcVoiceMediaChannel::SetLocalSource(uint32_t ssrc,
AudioSource* source) {
auto it = send_streams_.find(ssrc);
diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h
index 8990b21..8067ef0 100644
--- a/media/engine/webrtc_voice_engine.h
+++ b/media/engine/webrtc_voice_engine.h
@@ -166,6 +166,7 @@
bool RemoveSendStream(uint32_t ssrc) override;
bool AddRecvStream(const StreamParams& sp) override;
bool RemoveRecvStream(uint32_t ssrc) override;
+ void ResetUnsignaledRecvStream() override;
// E2EE Frame API
// Set a frame decryptor to a particular ssrc that will intercept all
diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc
index c0f0f2e..8fac2a1 100644
--- a/media/engine/webrtc_voice_engine_unittest.cc
+++ b/media/engine/webrtc_voice_engine_unittest.cc
@@ -2590,9 +2590,9 @@
GetRecvStream(kSsrc1).VerifyLastPacket(kPcmuFrame, sizeof(kPcmuFrame)));
EXPECT_EQ(kSyncLabel, GetRecvStream(kSsrc1).GetConfig().sync_group);
- // Removing the unsignaled stream clears the cached parameters. If a new
+ // Remset the unsignaled stream to clear the cached parameters. If a new
// default unsignaled receive stream is created it will not have a sync group.
- channel_->RemoveRecvStream(0);
+ channel_->ResetUnsignaledRecvStream();
channel_->RemoveRecvStream(kSsrc1);
DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame));
@@ -2763,9 +2763,9 @@
EXPECT_EQ(0u, call_.GetAudioReceiveStreams().size());
}
-TEST_F(WebRtcVoiceEngineTestFake, TestAddRecvStreamFailWithZeroSsrc) {
+TEST_F(WebRtcVoiceEngineTestFake, TestAddRecvStreamSuccessWithZeroSsrc) {
EXPECT_TRUE(SetupSendStream());
- EXPECT_FALSE(AddRecvStream(0));
+ EXPECT_TRUE(AddRecvStream(0));
}
TEST_F(WebRtcVoiceEngineTestFake, TestAddRecvStreamFailWithSameSsrc) {
diff --git a/pc/channel.cc b/pc/channel.cc
index bcc3d16..8392775 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -580,6 +580,11 @@
return media_channel()->RemoveRecvStream(ssrc);
}
+void BaseChannel::ResetUnsignaledRecvStream_w() {
+ RTC_DCHECK(worker_thread() == rtc::Thread::Current());
+ media_channel()->ResetUnsignaledRecvStream();
+}
+
bool BaseChannel::UpdateLocalStreams_w(const std::vector<StreamParams>& streams,
SdpType type,
std::string* error_desc) {
@@ -666,8 +671,11 @@
for (const StreamParams& old_stream : remote_streams_) {
// If we no longer have an unsignaled stream, we would like to remove
// the unsignaled stream params that are cached.
- if ((!old_stream.has_ssrcs() && !HasStreamWithNoSsrcs(streams)) ||
- !GetStreamBySsrc(streams, old_stream.first_ssrc())) {
+ if (!old_stream.has_ssrcs() && !HasStreamWithNoSsrcs(streams)) {
+ ResetUnsignaledRecvStream_w();
+ RTC_LOG(LS_INFO) << "Reset unsignaled remote stream.";
+ } else if (old_stream.has_ssrcs() &&
+ !GetStreamBySsrc(streams, old_stream.first_ssrc())) {
if (RemoveRecvStream_w(old_stream.first_ssrc())) {
RTC_LOG(LS_INFO) << "Remove remote ssrc: " << old_stream.first_ssrc();
} else {
@@ -688,10 +696,16 @@
if ((!new_stream.has_ssrcs() && !HasStreamWithNoSsrcs(remote_streams_)) ||
!GetStreamBySsrc(remote_streams_, new_stream.first_ssrc())) {
if (AddRecvStream_w(new_stream)) {
- RTC_LOG(LS_INFO) << "Add remote ssrc: " << new_stream.first_ssrc();
+ RTC_LOG(LS_INFO) << "Add remote ssrc: "
+ << (new_stream.has_ssrcs()
+ ? std::to_string(new_stream.first_ssrc())
+ : "unsignaled");
} else {
rtc::StringBuilder desc;
- desc << "Failed to add remote stream ssrc: " << new_stream.first_ssrc();
+ desc << "Failed to add remote stream ssrc: "
+ << (new_stream.has_ssrcs()
+ ? std::to_string(new_stream.first_ssrc())
+ : "unsignaled");
SafeSetError(desc.str(), error_desc);
ret = false;
}
diff --git a/pc/channel.h b/pc/channel.h
index 3b76776..62fcaa2 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -233,6 +233,7 @@
bool AddRecvStream_w(const StreamParams& sp);
bool RemoveRecvStream_w(uint32_t ssrc);
+ void ResetUnsignaledRecvStream_w();
bool AddSendStream_w(const StreamParams& sp);
bool RemoveSendStream_w(uint32_t ssrc);
