From 63551c6f0cd0f8e60bbe8e08e24668b0a13dadf1 Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Fri, 12 May 2023 08:24:02 +0000 Subject: [PATCH] Initialize RTP modes from callback Before the channel split, the RTP modes were set by reading the configuration of the send codec. After the split, this is done via the SetReceiverFeedbackParams function. This CL adds caching those parameters so that they are applied to receive streams created after the SetReceiverFeedbackParams call. Bug: webrtc:13931 Change-Id: I92eb651e5dd1ec68aca7f6a162e3521eb835a11d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305021 Commit-Queue: Harald Alvestrand Reviewed-by: Per Kjellander Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#40056} --- media/base/media_channel_impl.h | 2 +- media/engine/webrtc_video_engine.cc | 33 +++++++++++++++++++---------- media/engine/webrtc_video_engine.h | 5 +++++ 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/media/base/media_channel_impl.h b/media/base/media_channel_impl.h index 1887181a62..35ee7d527f 100644 --- a/media/base/media_channel_impl.h +++ b/media/base/media_channel_impl.h @@ -86,7 +86,7 @@ class MediaChannel : public MediaSendChannelInterface, bool enable_dscp = false); virtual ~MediaChannel(); - Role role() { return role_; } + Role role() const { return role_; } // Downcasting to the subclasses. virtual VideoMediaChannel* AsVideoChannel() { diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 255664821f..1a5ad4bd96 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -985,9 +985,6 @@ bool WebRtcVideoChannel::ApplyChangedParams( if (changed_params.send_codec || changed_params.rtcp_mode) { // Update receive feedback parameters from new codec or RTCP mode. if (send_codec_) { - 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 @@ -1018,6 +1015,15 @@ void WebRtcVideoChannel::SetReceiverFeedbackParameters( kv.second->SetFeedbackParameters(lntf_enabled, nack_enabled, rtcp_mode, rtx_time); } + // Store for future creation of receive streams + rtp_config_.lntf.enabled = lntf_enabled; + if (nack_enabled) { + rtp_config_.nack.rtp_history_ms = kNackHistoryMs; + } else { + rtp_config_.nack.rtp_history_ms = 0; + } + rtp_config_.rtcp_mode = rtcp_mode; + // Note: There is no place in config to store rtx_time. } webrtc::RtpParameters WebRtcVideoChannel::GetRtpSendParameters( @@ -1525,14 +1531,19 @@ void WebRtcVideoChannel::ConfigureReceiverRtp( } } - // Whether or not the receive stream sends reduced size RTCP is determined - // by the send params. - // TODO(deadbeef): Once we change "send_params" to "sender_params" and - // "recv_params" to "receiver_params", we should get this out of - // receiver_params_. - config->rtp.rtcp_mode = send_params_.rtcp.reduced_size - ? webrtc::RtcpMode::kReducedSize - : webrtc::RtcpMode::kCompound; + if (role() == MediaChannel::Role::kBoth) { + // Whether or not the receive stream sends reduced size RTCP is determined + // by the send params. + // TODO(deadbeef): Once we change "send_params" to "sender_params" and + // "recv_params" to "receiver_params", we should get this out of + // receiver_params_. + config->rtp.rtcp_mode = send_params_.rtcp.reduced_size + ? webrtc::RtcpMode::kReducedSize + : webrtc::RtcpMode::kCompound; + } else { + // The mode is determined by a call to the configuration function. + config->rtp.rtcp_mode = rtp_config_.rtcp_mode; + } // rtx-time (RFC 4588) is a declarative attribute similar to rtcp-rsize and // determined by the sender / send codec. diff --git a/media/engine/webrtc_video_engine.h b/media/engine/webrtc_video_engine.h index 2cdd05ffd6..50eecd7d1f 100644 --- a/media/engine/webrtc_video_engine.h +++ b/media/engine/webrtc_video_engine.h @@ -704,6 +704,11 @@ class WebRtcVideoChannel : public VideoMediaChannel, // Presence of EncoderSelector allows switching to specific encoders. bool allow_codec_switching_ = false; + // RTP parameters that need to be set when creating a video receive stream. + // Only used in Receiver mode - in Both mode, it reads those things from the + // codec. + webrtc::VideoReceiveStreamInterface::Config::Rtp rtp_config_; + // Callback invoked whenever the send codec changes. // TODO(bugs.webrtc.org/13931): Remove again when coupling isn't needed. absl::AnyInvocable send_codec_changed_callback_;