Reland: Update local_ssrc without needing to recreate video streams.
This is comparable to this change done previously for for audio streams:
https://webrtc-review.googlesource.com/c/src/+/222042
This is a reland of commit 16a8b25d809e4d4982f9fc4b4e973acd506b8bca
with an additional fix in Patchset 2. Another problem turned out to be
in RTCPReceiver, which is fixed in:
https://webrtc-review.googlesource.com/c/src/+/262663
Bug: webrtc:11993
Change-Id: I63c7cf62a6dd50f88b491fea3ba866697552ef5f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262665
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36907}
diff --git a/call/call.cc b/call/call.cc
index 3579ad7..1582bd2 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -266,6 +266,10 @@
void OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream,
uint32_t local_ssrc) override;
+ void OnLocalSsrcUpdated(VideoReceiveStream& stream,
+ uint32_t local_ssrc) override;
+ void OnLocalSsrcUpdated(FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) override;
void OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
const std::string& sync_group) override;
@@ -415,6 +419,8 @@
RTC_GUARDED_BY(&receive_11993_checker_);
// Audio and Video send streams are owned by the client that creates them.
+ // TODO(bugs.webrtc.org/11993): `audio_send_ssrcs_` and `video_send_ssrcs_`
+ // should be accessed on the network thread.
std::map<uint32_t, AudioSendStream*> audio_send_ssrcs_
RTC_GUARDED_BY(worker_thread_);
std::map<uint32_t, VideoSendStream*> video_send_ssrcs_
@@ -1385,6 +1391,17 @@
: nullptr);
}
+void Call::OnLocalSsrcUpdated(VideoReceiveStream& stream, uint32_t local_ssrc) {
+ RTC_DCHECK_RUN_ON(worker_thread_);
+ static_cast<VideoReceiveStream2&>(stream).SetLocalSsrc(local_ssrc);
+}
+
+void Call::OnLocalSsrcUpdated(FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) {
+ RTC_DCHECK_RUN_ON(worker_thread_);
+ static_cast<FlexfecReceiveStreamImpl&>(stream).SetLocalSsrc(local_ssrc);
+}
+
void Call::OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
const std::string& sync_group) {
RTC_DCHECK_RUN_ON(worker_thread_);
diff --git a/call/call.h b/call/call.h
index 9d6d4ee..65bd5f2 100644
--- a/call/call.h
+++ b/call/call.h
@@ -165,6 +165,10 @@
// send streams needs to be updated.
virtual void OnLocalSsrcUpdated(AudioReceiveStream& stream,
uint32_t local_ssrc) = 0;
+ virtual void OnLocalSsrcUpdated(VideoReceiveStream& stream,
+ uint32_t local_ssrc) = 0;
+ virtual void OnLocalSsrcUpdated(FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) = 0;
virtual void OnUpdateSyncGroup(AudioReceiveStream& stream,
const std::string& sync_group) = 0;
diff --git a/call/degraded_call.cc b/call/degraded_call.cc
index 3790c78..e6dd361 100644
--- a/call/degraded_call.cc
+++ b/call/degraded_call.cc
@@ -304,6 +304,16 @@
call_->OnLocalSsrcUpdated(stream, local_ssrc);
}
+void DegradedCall::OnLocalSsrcUpdated(VideoReceiveStream& stream,
+ uint32_t local_ssrc) {
+ call_->OnLocalSsrcUpdated(stream, local_ssrc);
+}
+
+void DegradedCall::OnLocalSsrcUpdated(FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) {
+ call_->OnLocalSsrcUpdated(stream, local_ssrc);
+}
+
void DegradedCall::OnUpdateSyncGroup(AudioReceiveStream& stream,
const std::string& sync_group) {
call_->OnUpdateSyncGroup(stream, sync_group);
diff --git a/call/degraded_call.h b/call/degraded_call.h
index 59f5236..586bb91 100644
--- a/call/degraded_call.h
+++ b/call/degraded_call.h
@@ -100,6 +100,10 @@
int transport_overhead_per_packet) override;
void OnLocalSsrcUpdated(AudioReceiveStream& stream,
uint32_t local_ssrc) override;
+ void OnLocalSsrcUpdated(VideoReceiveStream& stream,
+ uint32_t local_ssrc) override;
+ void OnLocalSsrcUpdated(FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) override;
void OnUpdateSyncGroup(AudioReceiveStream& stream,
const std::string& sync_group) override;
void OnSentPacket(const rtc::SentPacket& sent_packet) override;
diff --git a/call/flexfec_receive_stream_impl.cc b/call/flexfec_receive_stream_impl.cc
index 6f2b5dc..59add82 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -213,4 +213,14 @@
return extension_map_;
}
+void FlexfecReceiveStreamImpl::SetLocalSsrc(uint32_t local_ssrc) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ if (local_ssrc == config_.rtp.local_ssrc)
+ return;
+
+ auto& c = const_cast<Config&>(config_);
+ c.rtp.local_ssrc = local_ssrc;
+ rtp_rtcp_->SetLocalSsrc(local_ssrc);
+}
+
} // namespace webrtc
diff --git a/call/flexfec_receive_stream_impl.h b/call/flexfec_receive_stream_impl.h
index e25b7f0..857715a 100644
--- a/call/flexfec_receive_stream_impl.h
+++ b/call/flexfec_receive_stream_impl.h
@@ -62,6 +62,10 @@
void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
RtpHeaderExtensionMap GetRtpExtensionMap() const override;
+ // Updates the `rtp_video_stream_receiver_`'s `local_ssrc` when the default
+ // sender has been created, changed or removed.
+ void SetLocalSsrc(uint32_t local_ssrc);
+
uint32_t remote_ssrc() const { return config_.rtp.remote_ssrc; }
bool transport_cc() const override {
RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
@@ -73,8 +77,8 @@
RtpHeaderExtensionMap extension_map_;
- // Config. Mostly const, header extensions may change, which is an exception
- // case that's specifically handled in `SetRtpExtensions`, which must be
+ // Config. Mostly const, local_ssrc may change, which is an exception
+ // case that's specifically handled in `SetLocalSsrc`, which must be
// called on the `packet_sequence_checker` thread.
const Config config_;
diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc
index 19a4ad2..0cbbf7c 100644
--- a/media/engine/fake_webrtc_call.cc
+++ b/media/engine/fake_webrtc_call.cc
@@ -715,6 +715,18 @@
fake_stream.SetLocalSsrc(local_ssrc);
}
+void FakeCall::OnLocalSsrcUpdated(webrtc::VideoReceiveStream& stream,
+ uint32_t local_ssrc) {
+ auto& fake_stream = static_cast<FakeVideoReceiveStream&>(stream);
+ fake_stream.SetLocalSsrc(local_ssrc);
+}
+
+void FakeCall::OnLocalSsrcUpdated(webrtc::FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) {
+ auto& fake_stream = static_cast<FakeFlexfecReceiveStream&>(stream);
+ fake_stream.SetLocalSsrc(local_ssrc);
+}
+
void FakeCall::OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
const std::string& sync_group) {
auto& fake_stream = static_cast<FakeAudioReceiveStream&>(stream);
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index c26f7b1..8a3e226 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -250,6 +250,10 @@
return base_mininum_playout_delay_ms_;
}
+ void SetLocalSsrc(uint32_t local_ssrc) {
+ config_.rtp.local_ssrc = local_ssrc;
+ }
+
void SetFrameDecryptor(rtc::scoped_refptr<webrtc::FrameDecryptorInterface>
frame_decryptor) override {}
@@ -295,6 +299,10 @@
explicit FakeFlexfecReceiveStream(
const webrtc::FlexfecReceiveStream::Config config);
+ void SetLocalSsrc(uint32_t local_ssrc) {
+ config_.rtp.local_ssrc = local_ssrc;
+ }
+
void SetRtpExtensions(std::vector<webrtc::RtpExtension> extensions) override;
webrtc::RtpHeaderExtensionMap GetRtpExtensionMap() const override;
bool transport_cc() const override { return config_.rtp.transport_cc; }
@@ -411,6 +419,10 @@
int transport_overhead_per_packet) override;
void OnLocalSsrcUpdated(webrtc::AudioReceiveStream& stream,
uint32_t local_ssrc) override;
+ void OnLocalSsrcUpdated(webrtc::VideoReceiveStream& stream,
+ uint32_t local_ssrc) override;
+ void OnLocalSsrcUpdated(webrtc::FlexfecReceiveStream& stream,
+ uint32_t local_ssrc) override;
void OnUpdateSyncGroup(webrtc::AudioReceiveStream& stream,
const std::string& sync_group) override;
void OnSentPacket(const rtc::SentPacket& sent_packet) override;
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 435ad6c..a8f33a9 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1262,6 +1262,16 @@
return out.Release();
}
+// RTC_RUN_ON(&thread_checker_)
+void WebRtcVideoChannel::SetReceiverReportSsrc(uint32_t ssrc) {
+ if (ssrc == rtcp_receiver_report_ssrc_)
+ return;
+
+ rtcp_receiver_report_ssrc_ = ssrc;
+ for (auto& [unused, receive_stream] : receive_streams_)
+ receive_stream->SetLocalSsrc(ssrc);
+}
+
bool WebRtcVideoChannel::GetSendCodec(VideoCodec* codec) {
RTC_DCHECK_RUN_ON(&thread_checker_);
if (!send_codec_) {
@@ -1374,13 +1384,9 @@
send_streams_[ssrc] = stream;
if (rtcp_receiver_report_ssrc_ == kDefaultRtcpReceiverReportSsrc) {
- rtcp_receiver_report_ssrc_ = ssrc;
- RTC_LOG(LS_INFO)
- << "SetLocalSsrc on all the receive streams because we added "
- "a send stream.";
- for (auto& kv : receive_streams_)
- kv.second->SetLocalSsrc(ssrc);
+ SetReceiverReportSsrc(ssrc);
}
+
if (sending_) {
stream->SetSend(true);
}
@@ -1407,15 +1413,8 @@
// Switch receiver report SSRCs, the one in use is no longer valid.
if (rtcp_receiver_report_ssrc_ == ssrc) {
- rtcp_receiver_report_ssrc_ = send_streams_.empty()
- ? kDefaultRtcpReceiverReportSsrc
- : send_streams_.begin()->first;
- RTC_LOG(LS_INFO) << "SetLocalSsrc on all the receive streams because the "
- "previous local SSRC was removed.";
-
- for (auto& kv : receive_streams_) {
- kv.second->SetLocalSsrc(rtcp_receiver_report_ssrc_);
- }
+ SetReceiverReportSsrc(send_streams_.empty() ? kDefaultRtcpReceiverReportSsrc
+ : send_streams_.begin()->first);
}
delete removed_stream;
@@ -2827,7 +2826,7 @@
config_.renderer = this;
ConfigureCodecs(recv_codecs);
flexfec_config_.payload_type = flexfec_config.payload_type;
- RecreateWebRtcVideoStream();
+ RecreateReceiveStream();
}
WebRtcVideoChannel::WebRtcVideoReceiveStream::~WebRtcVideoReceiveStream() {
@@ -2836,6 +2835,17 @@
call_->DestroyFlexfecReceiveStream(flexfec_stream_);
}
+webrtc::VideoReceiveStream&
+WebRtcVideoChannel::WebRtcVideoReceiveStream::stream() {
+ RTC_DCHECK(stream_);
+ return *stream_;
+}
+
+webrtc::FlexfecReceiveStream*
+WebRtcVideoChannel::WebRtcVideoReceiveStream::flexfec_stream() {
+ return flexfec_stream_;
+}
+
const std::vector<uint32_t>&
WebRtcVideoChannel::WebRtcVideoReceiveStream::GetSsrcs() const {
return stream_params_.ssrcs;
@@ -2945,27 +2955,6 @@
return recreate_needed;
}
-void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetLocalSsrc(
- uint32_t local_ssrc) {
- // TODO(pbos): Consider turning this sanity check into a RTC_DCHECK. You
- // should not be able to create a sender with the same SSRC as a receiver, but
- // right now this can't be done due to unittests depending on receiving what
- // they are sending from the same MediaChannel.
- if (local_ssrc == config_.rtp.local_ssrc) {
- RTC_DLOG(LS_INFO) << "Ignoring call to SetLocalSsrc because parameters are "
- "unchanged; local_ssrc="
- << local_ssrc;
- return;
- }
-
- config_.rtp.local_ssrc = local_ssrc;
- flexfec_config_.rtp.local_ssrc = local_ssrc;
- RTC_LOG(LS_INFO)
- << "RecreateWebRtcVideoStream (recv) because of SetLocalSsrc; local_ssrc="
- << local_ssrc;
- RecreateWebRtcVideoStream();
-}
-
void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetFeedbackParameters(
bool lntf_enabled,
bool nack_enabled,
@@ -2994,10 +2983,10 @@
// based on the rtcp-fb for the FlexFEC codec, not the media codec.
flexfec_config_.rtp.transport_cc = config_.rtp.transport_cc;
flexfec_config_.rtcp_mode = config_.rtp.rtcp_mode;
- RTC_LOG(LS_INFO) << "RecreateWebRtcVideoStream (recv) because of "
+ RTC_LOG(LS_INFO) << "RecreateReceiveStream (recv) because of "
"SetFeedbackParameters; nack="
<< nack_enabled << ", transport_cc=" << transport_cc_enabled;
- RecreateWebRtcVideoStream();
+ RecreateReceiveStream();
}
void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters(
@@ -3036,11 +3025,11 @@
video_needs_recreation = true;
}
if (video_needs_recreation) {
- RecreateWebRtcVideoStream();
+ RecreateReceiveStream();
}
}
-void WebRtcVideoChannel::WebRtcVideoReceiveStream::RecreateWebRtcVideoStream() {
+void WebRtcVideoChannel::WebRtcVideoReceiveStream::RecreateReceiveStream() {
absl::optional<int> base_minimum_playout_delay_ms;
absl::optional<webrtc::VideoReceiveStream::RecordingState> recording_state;
if (stream_) {
@@ -3267,6 +3256,13 @@
stream_->SetDepacketizerToDecoderFrameTransformer(frame_transformer);
}
+void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetLocalSsrc(uint32_t ssrc) {
+ config_.rtp.local_ssrc = ssrc;
+ call_->OnLocalSsrcUpdated(stream(), ssrc);
+ if (flexfec_stream_)
+ call_->OnLocalSsrcUpdated(*flexfec_stream_, ssrc);
+}
+
WebRtcVideoChannel::VideoCodecSettings::VideoCodecSettings()
: flexfec_payload_type(-1), rtx_payload_type(-1), rtx_time(-1) {}
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h
index f70ebca..9e4bc4d 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -316,6 +316,10 @@
static std::string CodecSettingsVectorToString(
const std::vector<VideoCodecSettings>& codecs);
+ // Called when the local ssrc changes. Sets `rtcp_receiver_report_ssrc_` and
+ // updates the receive streams.
+ void SetReceiverReportSsrc(uint32_t ssrc) RTC_RUN_ON(&thread_checker_);
+
// Wrapper for the sender part.
class WebRtcVideoSendStream {
public:
@@ -438,6 +442,10 @@
const webrtc::FlexfecReceiveStream::Config& flexfec_config);
~WebRtcVideoReceiveStream();
+ webrtc::VideoReceiveStream& stream();
+ // Return value may be nullptr.
+ webrtc::FlexfecReceiveStream* flexfec_stream();
+
const std::vector<uint32_t>& GetSsrcs() const;
std::vector<webrtc::RtpSource> GetSources();
@@ -445,7 +453,6 @@
// Does not return codecs, they are filled by the owning WebRtcVideoChannel.
webrtc::RtpParameters GetRtpParameters() const;
- void SetLocalSsrc(uint32_t local_ssrc);
// TODO(deadbeef): Move these feedback parameters into the recv parameters.
void SetFeedbackParameters(bool lntf_enabled,
bool nack_enabled,
@@ -477,8 +484,10 @@
rtc::scoped_refptr<webrtc::FrameTransformerInterface>
frame_transformer);
+ void SetLocalSsrc(uint32_t local_ssrc);
+
private:
- void RecreateWebRtcVideoStream();
+ void RecreateReceiveStream();
// Applies a new receive codecs configration to `config_`. Returns true
// if the internal stream needs to be reconstructed, or false if no changes
diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc
index 886d1bd..c4dd7c0 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -923,6 +923,11 @@
nack_module_->UpdateRtt(max_rtt_ms);
}
+void RtpVideoStreamReceiver2::OnLocalSsrcChange(uint32_t local_ssrc) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ rtp_rtcp_->SetLocalSsrc(local_ssrc);
+}
+
absl::optional<int64_t> RtpVideoStreamReceiver2::LastReceivedPacketMs() const {
RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
if (last_received_rtp_system_time_) {
diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h
index 4658401..b0c7eab 100644
--- a/video/rtp_video_stream_receiver2.h
+++ b/video/rtp_video_stream_receiver2.h
@@ -184,6 +184,9 @@
// Called by VideoReceiveStream when stats are updated.
void UpdateRtt(int64_t max_rtt_ms);
+ // Called when the local_ssrc is changed to match with a sender.
+ void OnLocalSsrcChange(uint32_t local_ssrc);
+
absl::optional<int64_t> LastReceivedPacketMs() const;
absl::optional<int64_t> LastReceivedKeyframePacketMs() const;
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 96c13a9..47c1634 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -341,6 +341,16 @@
rtp_stream_sync_.ConfigureSync(audio_syncable);
}
+void VideoReceiveStream2::SetLocalSsrc(uint32_t local_ssrc) {
+ RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
+ if (config_.rtp.local_ssrc == local_ssrc)
+ return;
+
+ // TODO(tommi): Make sure we don't rely on local_ssrc via the config struct.
+ const_cast<uint32_t&>(config_.rtp.local_ssrc) = local_ssrc;
+ rtp_video_stream_receiver_.OnLocalSsrcChange(local_ssrc);
+}
+
void VideoReceiveStream2::Start() {
RTC_DCHECK_RUN_ON(&worker_sequence_checker_);
@@ -476,9 +486,8 @@
// and guarded by `packet_sequence_checker_`. However the scope of that state
// is huge (the whole Config struct), and would require all methods that touch
// the struct to abide the needs of the `extensions` member.
- VideoReceiveStream::Config& c =
- const_cast<VideoReceiveStream::Config&>(config_);
- c.rtp.extensions = std::move(extensions);
+ const_cast<std::vector<RtpExtension>&>(config_.rtp.extensions) =
+ std::move(extensions);
}
RtpHeaderExtensionMap VideoReceiveStream2::GetRtpExtensionMap() const {
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index d5b2b18..184fd20 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -131,6 +131,10 @@
void SetSync(Syncable* audio_syncable);
+ // Updates the `rtp_video_stream_receiver_`'s `local_ssrc` when the default
+ // sender has been created, changed or removed.
+ void SetLocalSsrc(uint32_t local_ssrc);
+
// Implements webrtc::VideoReceiveStream.
void Start() override;
void Stop() override;
