From a087f6f1c842f1d70ad207b44c48321ab60d2d95 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 21 Feb 2023 15:08:22 +0000 Subject: [PATCH] Add plumbing for video NACK to be coupled between channels. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:13931, webrtc:14920 Change-Id: I451869e295e099a1d08c0c80e481decd53149f1b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294382 Reviewed-by: Henrik Boström Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#39373} --- media/base/fake_media_engine.h | 12 +++++++++++ media/base/media_channel.h | 9 ++++++++ media/base/media_channel_impl.h | 16 ++++++++++++++ media/engine/webrtc_video_engine.cc | 32 +++++++++++++++++++++------- media/engine/webrtc_video_engine.h | 33 +++++++++++++++++++++++++++++ pc/channel.cc | 12 +++++++++++ 6 files changed, 106 insertions(+), 8 deletions(-) diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index 1d8f38337b..ac55fd1b37 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -513,6 +513,18 @@ class FakeVideoMediaChannel : public RtpHelper { void RequestRecvKeyFrame(uint32_t ssrc) override; void GenerateSendKeyFrame(uint32_t ssrc, const std::vector& rids) override; + webrtc::RtcpMode SendCodecRtcpMode() const override { + return webrtc::RtcpMode::kCompound; + } + bool SendCodecHasLntf() const override { return false; } + bool SendCodecHasNack() const override { return false; } + absl::optional SendCodecRtxTime() const override { + return absl::nullopt; + } + void SetReceiverFeedbackParameters(bool lntf_enabled, + bool nack_enabled, + webrtc::RtcpMode rtcp_mode, + absl::optional rtx_time) override {} private: bool SetRecvCodecs(const std::vector& codecs); diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 657257947c..571d661d45 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -959,6 +959,11 @@ class VideoMediaSendChannelInterface : public MediaSendChannelInterface { virtual void SetVideoCodecSwitchingEnabled(bool enabled) = 0; virtual bool GetStats(VideoMediaSendInfo* stats) = 0; virtual void FillBitrateInfo(BandwidthEstimationInfo* bwe_info) = 0; + // Information queries to support SetReceiverFeedbackParameters + virtual webrtc::RtcpMode SendCodecRtcpMode() const = 0; + virtual bool SendCodecHasLntf() const = 0; + virtual bool SendCodecHasNack() const = 0; + virtual absl::optional SendCodecRtxTime() const = 0; }; class VideoMediaReceiveChannelInterface : public MediaReceiveChannelInterface { @@ -988,6 +993,10 @@ class VideoMediaReceiveChannelInterface : public MediaReceiveChannelInterface { // Clear recordable encoded frame callback for `ssrc` virtual void ClearRecordableEncodedFrameCallback(uint32_t ssrc) = 0; virtual bool GetStats(VideoMediaReceiveInfo* stats) = 0; + virtual void SetReceiverFeedbackParameters(bool lntf_enabled, + bool nack_enabled, + webrtc::RtcpMode rtcp_mode, + absl::optional rtx_time) = 0; }; // Info about data received in DataMediaChannel. For use in diff --git a/media/base/media_channel_impl.h b/media/base/media_channel_impl.h index e37116624c..96a9bca121 100644 --- a/media/base/media_channel_impl.h +++ b/media/base/media_channel_impl.h @@ -652,6 +652,15 @@ class VideoMediaSendChannel : public VideoMediaSendChannelInterface { void FillBitrateInfo(BandwidthEstimationInfo* bwe_info) override { return impl_->FillBitrateInfo(bwe_info); } + // Information queries to support SetReceiverFeedbackParameters + webrtc::RtcpMode SendCodecRtcpMode() const override { + return impl()->SendCodecRtcpMode(); + } + bool SendCodecHasLntf() const override { return impl()->SendCodecHasLntf(); } + bool SendCodecHasNack() const override { return impl()->SendCodecHasNack(); } + absl::optional SendCodecRtxTime() const override { + return impl()->SendCodecRtxTime(); + } MediaChannel* ImplForTesting() override { return impl_; } @@ -775,6 +784,13 @@ class VideoMediaReceiveChannel : public VideoMediaReceiveChannelInterface { bool GetStats(VideoMediaReceiveInfo* info) override { return impl_->GetReceiveStats(info); } + void SetReceiverFeedbackParameters(bool lntf_enabled, + bool nack_enabled, + webrtc::RtcpMode rtcp_mode, + absl::optional rtx_time) override { + impl()->SetReceiverFeedbackParameters(lntf_enabled, nack_enabled, rtcp_mode, + rtx_time); + } MediaChannel* ImplForTesting() override { return impl_; } private: diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index edd6c3854c..976dba3af2 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -981,14 +981,13 @@ bool WebRtcVideoChannel::ApplyChangedParams( for (auto& kv : send_streams_) { kv.second->SetSendParameters(changed_params); } - if (changed_params.send_codec || changed_params.rtcp_mode) { - // Update receive feedback parameters from new codec or RTCP mode. - RTC_LOG(LS_INFO) - << "SetFeedbackParameters on all the receive streams because the send " - "codec or RTCP mode has changed."; - for (auto& kv : receive_streams_) { - RTC_DCHECK(kv.second != nullptr); - kv.second->SetFeedbackParameters( + if (role() == MediaChannel::Role::kBoth) { + if (changed_params.send_codec || changed_params.rtcp_mode) { + // Update receive feedback parameters from new codec or RTCP mode. + RTC_LOG(LS_INFO) << "SetFeedbackParameters on all the receive streams " + "because the send " + "codec or RTCP mode has changed."; + SetReceiverFeedbackParameters( HasLntf(send_codec_->codec), HasNack(send_codec_->codec), send_params_.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize : webrtc::RtcpMode::kCompound, @@ -998,6 +997,23 @@ bool WebRtcVideoChannel::ApplyChangedParams( return true; } +void WebRtcVideoChannel::SetReceiverFeedbackParameters( + bool lntf_enabled, + bool nack_enabled, + webrtc::RtcpMode rtcp_mode, + absl::optional rtx_time) { + RTC_DCHECK_RUN_ON(&thread_checker_); + + RTC_DCHECK(role() == MediaChannel::Role::kReceive || + role() == MediaChannel::Role::kBoth); + // Update receive feedback parameters from new codec or RTCP mode. + for (auto& kv : receive_streams_) { + RTC_DCHECK(kv.second != nullptr); + kv.second->SetFeedbackParameters(lntf_enabled, nack_enabled, rtcp_mode, + rtx_time); + } +} + webrtc::RtpParameters WebRtcVideoChannel::GetRtpSendParameters( uint32_t ssrc) const { RTC_DCHECK_RUN_ON(&thread_checker_); diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 8a14ea42ec..1b9ca3a428 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -231,6 +231,39 @@ class WebRtcVideoChannel : public VideoMediaChannel, rtc::scoped_refptr frame_transformer) override; + // Information queries to support SetReceiverFeedbackParameters + webrtc::RtcpMode SendCodecRtcpMode() const override { + RTC_DCHECK_RUN_ON(&thread_checker_); + return send_params_.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize + : webrtc::RtcpMode::kCompound; + } + + bool SendCodecHasLntf() const override { + RTC_DCHECK_RUN_ON(&thread_checker_); + if (!send_codec_) { + return false; + } + return HasLntf(send_codec_->codec); + } + bool SendCodecHasNack() const override { + RTC_DCHECK_RUN_ON(&thread_checker_); + if (!send_codec_) { + return false; + } + return HasNack(send_codec_->codec); + } + absl::optional SendCodecRtxTime() const override { + RTC_DCHECK_RUN_ON(&thread_checker_); + if (!send_codec_) { + return absl::nullopt; + } + return send_codec_->rtx_time; + } + void SetReceiverFeedbackParameters(bool lntf_enabled, + bool nack_enabled, + webrtc::RtcpMode rtcp_mode, + absl::optional rtx_time) override; + private: class WebRtcVideoReceiveStream; diff --git a/pc/channel.cc b/pc/channel.cc index 6c457fc2b4..dd94b7e46b 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -1064,6 +1064,12 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content, mid().c_str()); return false; } + // adjust receive streams based on send codec + media_receive_channel()->SetReceiverFeedbackParameters( + media_send_channel()->SendCodecHasLntf(), + media_send_channel()->SendCodecHasNack(), + media_send_channel()->SendCodecRtcpMode(), + media_send_channel()->SendCodecRtxTime()); last_send_params_ = send_params; } @@ -1131,6 +1137,12 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content, mid().c_str()); return false; } + // adjust receive streams based on send codec + media_receive_channel()->SetReceiverFeedbackParameters( + media_send_channel()->SendCodecHasLntf(), + media_send_channel()->SendCodecHasNack(), + media_send_channel()->SendCodecRtcpMode(), + media_send_channel()->SendCodecRtxTime()); last_send_params_ = send_params; if (needs_recv_params_update) {