From: "Byron Campen [:bwc]" Date: Fri, 19 Feb 2021 15:56:00 -0600 Subject: Bug 1654112 - Get RTCP BYE and RTP timeout handling working again (from Bug 1595479) r=mjf,dminor Differential Revision: https://phabricator.services.mozilla.com/D106145 Mercurial Revision: https://hg.mozilla.org/mozilla-central/rev/d0b311007c033e83824f5f6996a70ab9e870f31f --- audio/audio_receive_stream.cc | 5 ++++- audio/channel_receive.cc | 13 +++++++++---- audio/channel_receive.h | 4 +++- call/audio_receive_stream.h | 3 +++ call/video_receive_stream.cc | 2 ++ call/video_receive_stream.h | 3 +++ modules/rtp_rtcp/include/rtp_rtcp_defines.h | 8 ++++++++ modules/rtp_rtcp/source/rtcp_receiver.cc | 18 ++++++++++++++++-- modules/rtp_rtcp/source/rtcp_receiver.h | 1 + modules/rtp_rtcp/source/rtp_rtcp_interface.h | 3 +++ video/rtp_video_stream_receiver2.cc | 7 +++++-- 11 files changed, 57 insertions(+), 10 deletions(-) diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 978bbb25b2..655b2761ac 100644 --- a/audio/audio_receive_stream.cc +++ b/audio/audio_receive_stream.cc @@ -39,6 +39,8 @@ std::string AudioReceiveStreamInterface::Config::Rtp::ToString() const { ss << "{remote_ssrc: " << remote_ssrc; ss << ", local_ssrc: " << local_ssrc; ss << ", nack: " << nack.ToString(); + ss << ", rtcp_event_observer: " + << (rtcp_event_observer ? "(rtcp_event_observer)" : "nullptr"); ss << '}'; return ss.str(); } @@ -73,7 +75,8 @@ std::unique_ptr CreateChannelReceive( config.jitter_buffer_fast_accelerate, config.jitter_buffer_min_delay_ms, config.enable_non_sender_rtt, config.decoder_factory, config.codec_pair_id, std::move(config.frame_decryptor), - config.crypto_options, std::move(config.frame_transformer)); + config.crypto_options, std::move(config.frame_transformer), + config.rtp.rtcp_event_observer); } } // namespace diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index f5d214f672..c714b1dd4d 100644 --- a/audio/channel_receive.cc +++ b/audio/channel_receive.cc @@ -104,7 +104,8 @@ class ChannelReceive : public ChannelReceiveInterface, absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, const webrtc::CryptoOptions& crypto_options, - rtc::scoped_refptr frame_transformer); + rtc::scoped_refptr frame_transformer, + RtcpEventObserver* rtcp_event_observer); ~ChannelReceive() override; void SetSink(AudioSinkInterface* sink) override; @@ -538,7 +539,8 @@ ChannelReceive::ChannelReceive( absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, const webrtc::CryptoOptions& crypto_options, - rtc::scoped_refptr frame_transformer) + rtc::scoped_refptr frame_transformer, + RtcpEventObserver* rtcp_event_observer) : worker_thread_(TaskQueueBase::Current()), event_log_(rtc_event_log), rtp_receive_statistics_(ReceiveStatistics::Create(clock)), @@ -584,6 +586,7 @@ ChannelReceive::ChannelReceive( configuration.local_media_ssrc = local_ssrc; configuration.rtcp_packet_type_counter_observer = this; configuration.non_sender_rtt_measurement = enable_non_sender_rtt; + configuration.rtcp_event_observer = rtcp_event_observer; if (frame_transformer) InitFrameTransformerDelegate(std::move(frame_transformer)); @@ -1111,13 +1114,15 @@ std::unique_ptr CreateChannelReceive( absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, const webrtc::CryptoOptions& crypto_options, - rtc::scoped_refptr frame_transformer) { + rtc::scoped_refptr frame_transformer, + RtcpEventObserver* rtcp_event_observer) { return std::make_unique( clock, neteq_factory, audio_device_module, rtcp_send_transport, rtc_event_log, local_ssrc, remote_ssrc, jitter_buffer_max_packets, jitter_buffer_fast_playout, jitter_buffer_min_delay_ms, enable_non_sender_rtt, decoder_factory, codec_pair_id, - std::move(frame_decryptor), crypto_options, std::move(frame_transformer)); + std::move(frame_decryptor), crypto_options, std::move(frame_transformer), + rtcp_event_observer); } } // namespace voe diff --git a/audio/channel_receive.h b/audio/channel_receive.h index ab69103269..5713d97aaa 100644 --- a/audio/channel_receive.h +++ b/audio/channel_receive.h @@ -28,6 +28,7 @@ #include "call/rtp_packet_sink_interface.h" #include "call/syncable.h" #include "modules/audio_coding/include/audio_coding_module_typedefs.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/source_tracker.h" #include "system_wrappers/include/clock.h" @@ -186,7 +187,8 @@ std::unique_ptr CreateChannelReceive( absl::optional codec_pair_id, rtc::scoped_refptr frame_decryptor, const webrtc::CryptoOptions& crypto_options, - rtc::scoped_refptr frame_transformer); + rtc::scoped_refptr frame_transformer, + RtcpEventObserver* rtcp_event_observer); } // namespace voe } // namespace webrtc diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h index 4879311fdb..88b74b44ac 100644 --- a/call/audio_receive_stream.h +++ b/call/audio_receive_stream.h @@ -19,6 +19,7 @@ #include "absl/types/optional.h" #include "api/audio_codecs/audio_decoder_factory.h" #include "api/call/transport.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "api/crypto/crypto_options.h" #include "api/rtp_parameters.h" #include "call/receive_stream.h" @@ -117,6 +118,8 @@ class AudioReceiveStreamInterface : public MediaReceiveStreamInterface { // See NackConfig for description. NackConfig nack; + + RtcpEventObserver* rtcp_event_observer = nullptr; } rtp; // Receive-side RTT. diff --git a/call/video_receive_stream.cc b/call/video_receive_stream.cc index 8d88ce23c6..9ee9ed3e76 100644 --- a/call/video_receive_stream.cc +++ b/call/video_receive_stream.cc @@ -161,6 +161,8 @@ std::string VideoReceiveStreamInterface::Config::Rtp::ToString() const { ss << pt << ", "; } ss << '}'; + ss << ", rtcp_event_observer: " + << (rtcp_event_observer ? "(rtcp_event_observer)" : "nullptr"); ss << '}'; return ss.str(); } diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index a1fc204e7c..01ac7e0ba4 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -20,6 +20,7 @@ #include #include "api/call/transport.h" +#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "api/crypto/crypto_options.h" #include "api/rtp_headers.h" #include "api/rtp_parameters.h" @@ -241,6 +242,8 @@ class VideoReceiveStreamInterface : public MediaReceiveStreamInterface { // meta data is expected to be present in generic frame descriptor // RTP header extension). std::set raw_payload_types; + + RtcpEventObserver* rtcp_event_observer = nullptr; } rtp; // Transport for outgoing packets (RTCP). diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 249cf835ba..de85abd4ae 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -173,6 +173,14 @@ class NetworkLinkRtcpObserver { virtual void OnRttUpdate(Timestamp receive_time, TimeDelta rtt) {} }; +class RtcpEventObserver { + public: + virtual void OnRtcpBye() = 0; + virtual void OnRtcpTimeout() = 0; + + virtual ~RtcpEventObserver() {} +}; + // NOTE! `kNumMediaTypes` must be kept in sync with RtpPacketMediaType! static constexpr size_t kNumMediaTypes = 5; enum class RtpPacketMediaType : size_t { diff --git a/modules/rtp_rtcp/source/rtcp_receiver.cc b/modules/rtp_rtcp/source/rtcp_receiver.cc index a98b200c05..e2ad674012 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -144,6 +144,7 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, rtp_rtcp_(owner), registered_ssrcs_(false, config), network_link_rtcp_observer_(config.network_link_rtcp_observer), + rtcp_event_observer_(config.rtcp_event_observer), rtcp_intra_frame_observer_(config.intra_frame_callback), rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), network_state_estimate_observer_(config.network_state_estimate_observer), @@ -171,6 +172,7 @@ RTCPReceiver::RTCPReceiver(const RtpRtcpInterface::Configuration& config, rtp_rtcp_(owner), registered_ssrcs_(true, config), network_link_rtcp_observer_(config.network_link_rtcp_observer), + rtcp_event_observer_(config.rtcp_event_observer), rtcp_intra_frame_observer_(config.intra_frame_callback), rtcp_loss_notification_observer_(config.rtcp_loss_notification_observer), network_state_estimate_observer_(config.network_state_estimate_observer), @@ -778,6 +780,10 @@ bool RTCPReceiver::HandleBye(const CommonHeader& rtcp_block) { return false; } + if (rtcp_event_observer_) { + rtcp_event_observer_->OnRtcpBye(); + } + // Clear our lists. rtts_.erase(bye.sender_ssrc()); EraseIf(received_report_blocks_, [&](const auto& elem) { @@ -1199,12 +1205,20 @@ std::vector RTCPReceiver::TmmbrReceived() { } bool RTCPReceiver::RtcpRrTimeoutLocked(Timestamp now) { - return ResetTimestampIfExpired(now, last_received_rb_, report_interval_); + bool result = ResetTimestampIfExpired(now, last_received_rb_, report_interval_); + if (result && rtcp_event_observer_) { + rtcp_event_observer_->OnRtcpTimeout(); + } + return result; } bool RTCPReceiver::RtcpRrSequenceNumberTimeoutLocked(Timestamp now) { - return ResetTimestampIfExpired(now, last_increased_sequence_number_, + bool result = ResetTimestampIfExpired(now, last_increased_sequence_number_, report_interval_); + if (result && rtcp_event_observer_) { + rtcp_event_observer_->OnRtcpTimeout(); + } + return result; } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtcp_receiver.h b/modules/rtp_rtcp/source/rtcp_receiver.h index e748b257e8..36e117af55 100644 --- a/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/modules/rtp_rtcp/source/rtcp_receiver.h @@ -362,6 +362,7 @@ class RTCPReceiver final { RegisteredSsrcs registered_ssrcs_; NetworkLinkRtcpObserver* const network_link_rtcp_observer_; + RtcpEventObserver* const rtcp_event_observer_; RtcpIntraFrameObserver* const rtcp_intra_frame_observer_; RtcpLossNotificationObserver* const rtcp_loss_notification_observer_; NetworkStateEstimateObserver* const network_state_estimate_observer_; diff --git a/modules/rtp_rtcp/source/rtp_rtcp_interface.h b/modules/rtp_rtcp/source/rtp_rtcp_interface.h index 0bdd389795..2c56dccd2a 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_interface.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_interface.h @@ -74,6 +74,9 @@ class RtpRtcpInterface : public RtcpFeedbackSenderInterface { // bandwidth estimation related message. NetworkLinkRtcpObserver* network_link_rtcp_observer = nullptr; + // Called when we receive a RTCP bye or timeout + RtcpEventObserver* rtcp_event_observer = nullptr; + NetworkStateEstimateObserver* network_state_estimate_observer = nullptr; TransportFeedbackObserver* transport_feedback_callback = nullptr; VideoBitrateAllocationObserver* bitrate_allocation_observer = nullptr; diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index d12e833cab..2ea8ce8c62 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -83,7 +83,8 @@ std::unique_ptr CreateRtpRtcpModule( RtcpCnameCallback* rtcp_cname_callback, bool non_sender_rtt_measurement, uint32_t local_ssrc, - RtcEventLog* rtc_event_log) { + RtcEventLog* rtc_event_log, + RtcpEventObserver* rtcp_event_observer) { RtpRtcpInterface::Configuration configuration; configuration.clock = clock; configuration.audio = false; @@ -95,6 +96,7 @@ std::unique_ptr CreateRtpRtcpModule( rtcp_packet_type_counter_observer; configuration.rtcp_cname_callback = rtcp_cname_callback; configuration.local_media_ssrc = local_ssrc; + configuration.rtcp_event_observer = rtcp_event_observer; configuration.non_sender_rtt_measurement = non_sender_rtt_measurement; configuration.event_log = rtc_event_log; @@ -275,7 +277,8 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( rtcp_cname_callback, config_.rtp.rtcp_xr.receiver_reference_time_report, config_.rtp.local_ssrc, - event_log)), + event_log, + config_.rtp.rtcp_event_observer)), nack_periodic_processor_(nack_periodic_processor), complete_frame_callback_(complete_frame_callback), keyframe_request_method_(config_.rtp.keyframe_method),