From ba088b1dcedc677305249178164a3a3bc1b868c7 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Wed, 1 Mar 2023 08:54:53 +0000 Subject: [PATCH] Revert "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 This reverts commit a087f6f1c842f1d70ad207b44c48321ab60d2d95. Reason for revert: Needed to roll back other CL Original change's description: > Add plumbing for video NACK to be coupled between channels. > > 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} Bug: webrtc:13931, webrtc:14920 Change-Id: I19e176e75630313da470542e7ff1e89b6d717fc9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/295664 Commit-Queue: Harald Alvestrand Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#39432} --- 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, 8 insertions(+), 106 deletions(-) diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index 7a117d5f70..d983300c1b 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -513,18 +513,6 @@ 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 571d661d45..657257947c 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -959,11 +959,6 @@ 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 { @@ -993,10 +988,6 @@ 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 96a9bca121..e37116624c 100644 --- a/media/base/media_channel_impl.h +++ b/media/base/media_channel_impl.h @@ -652,15 +652,6 @@ 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_; } @@ -784,13 +775,6 @@ 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 b029c6c46c..ea11ae03f9 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -982,13 +982,14 @@ bool WebRtcVideoChannel::ApplyChangedParams( for (auto& kv : send_streams_) { kv.second->SetSendParameters(changed_params); } - 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( + 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( HasLntf(send_codec_->codec), HasNack(send_codec_->codec), send_params_.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize : webrtc::RtcpMode::kCompound, @@ -998,23 +999,6 @@ 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 5db1e0ed66..b059c39e02 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -231,39 +231,6 @@ 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 dd94b7e46b..6c457fc2b4 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -1064,12 +1064,6 @@ 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; } @@ -1137,12 +1131,6 @@ 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) {