Revert "Update local_ssrc without needing to recreate video streams."
This reverts commit 16a8b25d809e4d4982f9fc4b4e973acd506b8bca.
Reason for revert: Checking if this is blocking the Chromium autoroller.
Original change's description:
> 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
>
> Bug: webrtc:11993
> Change-Id: Ic953f816a8f7c56d1c3dc9a16d85bef3696a663d
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261960
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#36876}
Bug: webrtc:11993
Change-Id: I3a8d2f6a7e89b6784754d8e891a4e01479807c2d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262422
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#36892}
diff --git a/call/call.cc b/call/call.cc
index 1582bd2..3579ad7 100644
--- a/call/call.cc
+++ b/call/call.cc
@@ -266,10 +266,6 @@
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;
@@ -419,8 +415,6 @@
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_
@@ -1391,17 +1385,6 @@
: 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 65bd5f2..9d6d4ee 100644
--- a/call/call.h
+++ b/call/call.h
@@ -165,10 +165,6 @@
// 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 e6dd361..3790c78 100644
--- a/call/degraded_call.cc
+++ b/call/degraded_call.cc
@@ -304,16 +304,6 @@
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 586bb91..59f5236 100644
--- a/call/degraded_call.h
+++ b/call/degraded_call.h
@@ -100,10 +100,6 @@
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 59add82..6f2b5dc 100644
--- a/call/flexfec_receive_stream_impl.cc
+++ b/call/flexfec_receive_stream_impl.cc
@@ -213,14 +213,4 @@
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 857715a..e25b7f0 100644
--- a/call/flexfec_receive_stream_impl.h
+++ b/call/flexfec_receive_stream_impl.h
@@ -62,10 +62,6 @@
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_);
@@ -77,8 +73,8 @@
RtpHeaderExtensionMap extension_map_;
- // Config. Mostly const, local_ssrc may change, which is an exception
- // case that's specifically handled in `SetLocalSsrc`, which must be
+ // Config. Mostly const, header extensions may change, which is an exception
+ // case that's specifically handled in `SetRtpExtensions`, 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 0cbbf7c..19a4ad2 100644
--- a/media/engine/fake_webrtc_call.cc
+++ b/media/engine/fake_webrtc_call.cc
@@ -715,18 +715,6 @@
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 8a3e226..c26f7b1 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -250,10 +250,6 @@
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 {}
@@ -299,10 +295,6 @@
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; }
@@ -419,10 +411,6 @@
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 7ca8f67..0c3a3de 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1240,20 +1240,6 @@
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_) {
- call_->OnLocalSsrcUpdated(receive_stream->stream(), ssrc);
- webrtc::FlexfecReceiveStream* flex_fec = receive_stream->flexfec_stream();
- if (flex_fec)
- call_->OnLocalSsrcUpdated(*flex_fec, ssrc);
- }
-}
-
bool WebRtcVideoChannel::GetSendCodec(VideoCodec* codec) {
RTC_DCHECK_RUN_ON(&thread_checker_);
if (!send_codec_) {
@@ -1366,9 +1352,13 @@
send_streams_[ssrc] = stream;
if (rtcp_receiver_report_ssrc_ == kDefaultRtcpReceiverReportSsrc) {
- SetReceiverReportSsrc(ssrc);
+ 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);
}
-
if (sending_) {
stream->SetSend(true);
}
@@ -1395,8 +1385,15 @@
// Switch receiver report SSRCs, the one in use is no longer valid.
if (rtcp_receiver_report_ssrc_ == ssrc) {
- SetReceiverReportSsrc(send_streams_.empty() ? kDefaultRtcpReceiverReportSsrc
- : send_streams_.begin()->first);
+ 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_);
+ }
}
delete removed_stream;
@@ -2808,7 +2805,7 @@
config_.renderer = this;
ConfigureCodecs(recv_codecs);
flexfec_config_.payload_type = flexfec_config.payload_type;
- RecreateReceiveStream();
+ RecreateWebRtcVideoStream();
}
WebRtcVideoChannel::WebRtcVideoReceiveStream::~WebRtcVideoReceiveStream() {
@@ -2817,17 +2814,6 @@
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;
@@ -2937,6 +2923,27 @@
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,
@@ -2965,10 +2972,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) << "RecreateReceiveStream (recv) because of "
+ RTC_LOG(LS_INFO) << "RecreateWebRtcVideoStream (recv) because of "
"SetFeedbackParameters; nack="
<< nack_enabled << ", transport_cc=" << transport_cc_enabled;
- RecreateReceiveStream();
+ RecreateWebRtcVideoStream();
}
void WebRtcVideoChannel::WebRtcVideoReceiveStream::SetRecvParameters(
@@ -3007,11 +3014,11 @@
video_needs_recreation = true;
}
if (video_needs_recreation) {
- RecreateReceiveStream();
+ RecreateWebRtcVideoStream();
}
}
-void WebRtcVideoChannel::WebRtcVideoReceiveStream::RecreateReceiveStream() {
+void WebRtcVideoChannel::WebRtcVideoReceiveStream::RecreateWebRtcVideoStream() {
absl::optional<int> base_minimum_playout_delay_ms;
absl::optional<webrtc::VideoReceiveStream::RecordingState> recording_state;
if (stream_) {
diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h
index 62a2654..f70ebca 100644
--- a/media/engine/webrtc_video_engine.h
+++ b/media/engine/webrtc_video_engine.h
@@ -316,10 +316,6 @@
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:
@@ -442,10 +438,6 @@
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();
@@ -453,6 +445,7 @@
// 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,
@@ -485,7 +478,7 @@
frame_transformer);
private:
- void RecreateReceiveStream();
+ void RecreateWebRtcVideoStream();
// 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 c4dd7c0..886d1bd 100644
--- a/video/rtp_video_stream_receiver2.cc
+++ b/video/rtp_video_stream_receiver2.cc
@@ -923,11 +923,6 @@
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 b0c7eab..4658401 100644
--- a/video/rtp_video_stream_receiver2.h
+++ b/video/rtp_video_stream_receiver2.h
@@ -184,9 +184,6 @@
// 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 47c1634..96c13a9 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -341,16 +341,6 @@
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_);
@@ -486,8 +476,9 @@
// 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.
- const_cast<std::vector<RtpExtension>&>(config_.rtp.extensions) =
- std::move(extensions);
+ VideoReceiveStream::Config& c =
+ const_cast<VideoReceiveStream::Config&>(config_);
+ c.rtp.extensions = std::move(extensions);
}
RtpHeaderExtensionMap VideoReceiveStream2::GetRtpExtensionMap() const {
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 184fd20..d5b2b18 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -131,10 +131,6 @@
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;
