From 13b9f81b233ed3afaad9088b2f971c7996f58b79 Mon Sep 17 00:00:00 2001 From: Tommi Date: Tue, 16 Aug 2022 10:23:47 +0200 Subject: [PATCH] Updated associated payload types without recreating receive streams. Bug: webrtc:11993 Change-Id: I49c61653b296b1b3ca6a12fa75ac699ee58f096c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271543 Reviewed-by: Rasmus Brandt Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#37799} --- call/BUILD.gn | 1 + call/rtx_receive_stream.cc | 14 +++++++++++--- call/rtx_receive_stream.h | 10 +++++++++- call/video_receive_stream.h | 3 +++ media/BUILD.gn | 1 + media/engine/fake_webrtc_call.h | 5 +++++ media/engine/webrtc_video_engine.cc | 6 +++--- media/engine/webrtc_video_engine.h | 7 ++++--- video/video_receive_stream2.cc | 22 ++++++++++++++++++++-- video/video_receive_stream2.h | 2 ++ 10 files changed, 59 insertions(+), 12 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index e03f15df54..b772f4fc96 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -153,6 +153,7 @@ rtc_library("rtp_receiver") { "../rtc_base:stringutils", "../rtc_base/containers:flat_map", "../rtc_base/containers:flat_set", + "../rtc_base/system:no_unique_address", ] absl_deps = [ "//third_party/abseil-cpp/absl/strings:strings", diff --git a/call/rtx_receive_stream.cc b/call/rtx_receive_stream.cc index c0b138b416..6c5fa3f859 100644 --- a/call/rtx_receive_stream.cc +++ b/call/rtx_receive_stream.cc @@ -32,6 +32,7 @@ RtxReceiveStream::RtxReceiveStream( associated_payload_types_(std::move(associated_payload_types)), media_ssrc_(media_ssrc), rtp_receive_statistics_(rtp_receive_statistics) { + packet_checker_.Detach(); if (associated_payload_types_.empty()) { RTC_LOG(LS_WARNING) << "RtxReceiveStream created with empty payload type mapping."; @@ -40,7 +41,14 @@ RtxReceiveStream::RtxReceiveStream( RtxReceiveStream::~RtxReceiveStream() = default; +void RtxReceiveStream::SetAssociatedPayloadTypes( + std::map associated_payload_types) { + RTC_DCHECK_RUN_ON(&packet_checker_); + associated_payload_types_ = std::move(associated_payload_types); +} + void RtxReceiveStream::OnRtpPacket(const RtpPacketReceived& rtx_packet) { + RTC_DCHECK_RUN_ON(&packet_checker_); if (rtp_receive_statistics_) { rtp_receive_statistics_->OnRtpPacket(rtx_packet); } @@ -52,9 +60,9 @@ void RtxReceiveStream::OnRtpPacket(const RtpPacketReceived& rtx_packet) { auto it = associated_payload_types_.find(rtx_packet.PayloadType()); if (it == associated_payload_types_.end()) { - RTC_LOG(LS_VERBOSE) << "Unknown payload type " - << static_cast(rtx_packet.PayloadType()) - << " on rtx ssrc " << rtx_packet.Ssrc(); + RTC_DLOG(LS_VERBOSE) << "Unknown payload type " + << static_cast(rtx_packet.PayloadType()) + << " on rtx ssrc " << rtx_packet.Ssrc(); return; } RtpPacketReceived media_packet; diff --git a/call/rtx_receive_stream.h b/call/rtx_receive_stream.h index a389fc2a57..79b03d306b 100644 --- a/call/rtx_receive_stream.h +++ b/call/rtx_receive_stream.h @@ -14,7 +14,9 @@ #include #include +#include "api/sequence_checker.h" #include "call/rtp_packet_sink_interface.h" +#include "rtc_base/system/no_unique_address.h" namespace webrtc { @@ -33,13 +35,19 @@ class RtxReceiveStream : public RtpPacketSinkInterface { // RtpStreamReceiverController. ReceiveStatistics* rtp_receive_statistics = nullptr); ~RtxReceiveStream() override; + + // Update payload types post construction. Must be called from the same + // calling context as `OnRtpPacket` is called on. + void SetAssociatedPayloadTypes(std::map associated_payload_types); + // RtpPacketSinkInterface. void OnRtpPacket(const RtpPacketReceived& packet) override; private: + RTC_NO_UNIQUE_ADDRESS SequenceChecker packet_checker_; RtpPacketSinkInterface* const media_sink_; // Map from rtx payload type -> media payload type. - const std::map associated_payload_types_; + std::map associated_payload_types_ RTC_GUARDED_BY(&packet_checker_); // TODO(nisse): Ultimately, the media receive stream shouldn't care about the // ssrc, and we should delete this. const uint32_t media_ssrc_; diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index c98c24b495..3dbf66e1b1 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -308,6 +308,9 @@ class VideoReceiveStreamInterface : public MediaReceiveStreamInterface { virtual void SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) = 0; + virtual void SetAssociatedPayloadTypes( + std::map associated_payload_types) = 0; + protected: virtual ~VideoReceiveStreamInterface() {} }; diff --git a/media/BUILD.gn b/media/BUILD.gn index 8e810b7bcd..c12b08ff5b 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -326,6 +326,7 @@ rtc_library("rtc_audio_video") { "../rtc_base/experiments:normalize_simulcast_size_experiment", "../rtc_base/experiments:rate_control_settings", "../rtc_base/synchronization:mutex", + "../rtc_base/system:no_unique_address", "../rtc_base/system:rtc_export", "../rtc_base/third_party/base64", "../system_wrappers", diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index ad4338bfa1..311e35a7a9 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -308,6 +308,11 @@ class FakeVideoReceiveStream final config_.rtp.rtcp_xr = rtcp_xr; } + void SetAssociatedPayloadTypes(std::map associated_payload_types) { + config_.rtp.rtx_associated_payload_types = + std::move(associated_payload_types); + } + void Start() override; void Stop() override; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 24d6bd7a5e..4005de896b 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -3017,14 +3017,14 @@ bool WebRtcVideoChannel::WebRtcVideoReceiveStream::ReconfigureCodecs( codec.ulpfec.red_payload_type; } - bool recreate_needed = false; - if (config_.rtp.rtx_associated_payload_types != rtx_associated_payload_types) { + stream_->SetAssociatedPayloadTypes(rtx_associated_payload_types); rtx_associated_payload_types.swap(config_.rtp.rtx_associated_payload_types); - recreate_needed = true; } + bool recreate_needed = false; + if (raw_payload_types != config_.rtp.raw_payload_types) { raw_payload_types.swap(config_.rtp.raw_payload_types); recreate_needed = true; diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 29060c2001..3993c6635b 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -35,6 +35,7 @@ #include "media/engine/unhandled_packets_buffer.h" #include "rtc_base/network_route.h" #include "rtc_base/synchronization/mutex.h" +#include "rtc_base/system/no_unique_address.h" #include "rtc_base/thread_annotations.h" namespace webrtc { @@ -424,7 +425,7 @@ class WebRtcVideoChannel : public VideoMediaChannel, webrtc::DegradationPreference GetDegradationPreference() const RTC_EXCLUSIVE_LOCKS_REQUIRED(&thread_checker_); - webrtc::SequenceChecker thread_checker_; + RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker thread_checker_; webrtc::TaskQueueBase* const worker_thread_; const std::vector ssrcs_ RTC_GUARDED_BY(&thread_checker_); const std::vector ssrc_groups_ RTC_GUARDED_BY(&thread_checker_); @@ -588,8 +589,8 @@ class WebRtcVideoChannel : public VideoMediaChannel, webrtc::TaskQueueBase* const worker_thread_; webrtc::ScopedTaskSafety task_safety_; - webrtc::SequenceChecker network_thread_checker_; - webrtc::SequenceChecker thread_checker_; + RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker network_thread_checker_; + RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker thread_checker_; uint32_t rtcp_receiver_report_ssrc_ RTC_GUARDED_BY(thread_checker_); bool sending_ RTC_GUARDED_BY(thread_checker_); diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index f4a53154b7..23302fc599 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -268,8 +268,9 @@ VideoReceiveStream2::VideoReceiveStream2( if (rtx_ssrc()) { rtx_receive_stream_ = std::make_unique( - &rtp_video_stream_receiver_, config_.rtp.rtx_associated_payload_types, - remote_ssrc(), rtp_receive_statistics_.get()); + &rtp_video_stream_receiver_, + std::move(config_.rtp.rtx_associated_payload_types), remote_ssrc(), + rtp_receive_statistics_.get()); } else { rtp_receive_statistics_->EnableRetransmitDetection(remote_ssrc(), true); } @@ -564,6 +565,23 @@ void VideoReceiveStream2::SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) { rtcp_xr.receiver_reference_time_report); } +void VideoReceiveStream2::SetAssociatedPayloadTypes( + std::map associated_payload_types) { + RTC_DCHECK_RUN_ON(&packet_sequence_checker_); + + // For setting the associated payload types after construction, we currently + // assume that the rtx_ssrc cannot change. In such a case we can know that + // if the ssrc is non-0, a `rtx_receive_stream_` instance has previously been + // created and configured (and is referenced by `rtx_receiver_`) and we can + // simply reconfigure it. + // If rtx_ssrc is 0 however, we ignore this call. + if (!rtx_ssrc()) + return; + + rtx_receive_stream_->SetAssociatedPayloadTypes( + std::move(associated_payload_types)); +} + void VideoReceiveStream2::CreateAndRegisterExternalDecoder( const Decoder& decoder) { TRACE_EVENT0("webrtc", diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h index 4420798218..edc514d532 100644 --- a/video/video_receive_stream2.h +++ b/video/video_receive_stream2.h @@ -154,6 +154,8 @@ class VideoReceiveStream2 void SetProtectionPayloadTypes(int red_payload_type, int ulpfec_payload_type) override; void SetRtcpXr(Config::Rtp::RtcpXr rtcp_xr) override; + void SetAssociatedPayloadTypes( + std::map associated_payload_types) override; webrtc::VideoReceiveStreamInterface::Stats GetStats() const override;