diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index f92a732632..ed565ee5b9 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -837,6 +837,8 @@ struct RtpParameters { RtcpParameters rtcp; }; +// TODO(deadbeef): Rename to RtpSenderParameters, since they're intended to +// encapsulate all the parameters needed for an RtpSender. template struct RtpSendParameters : RtpParameters { std::string ToString() const override { @@ -934,6 +936,8 @@ class VoiceMediaChannel : public MediaChannel { std::unique_ptr sink) = 0; }; +// TODO(deadbeef): Rename to VideoSenderParameters, since they're intended to +// encapsulate all the parameters needed for a video RtpSender. struct VideoSendParameters : RtpSendParameters { // Use conference mode? This flag comes from the remote // description's SDP line 'a=x-google-flag:conference', copied over @@ -944,6 +948,8 @@ struct VideoSendParameters : RtpSendParameters { bool conference_mode = false; }; +// TODO(deadbeef): Rename to VideoReceiverParameters, since they're intended to +// encapsulate all the parameters needed for a video RtpReceiver. struct VideoRecvParameters : RtpParameters { }; diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index e34056d905..83f81f0f42 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -802,16 +802,18 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) { for (auto& kv : send_streams_) { kv.second->SetSendParameters(changed_params); } - if (changed_params.codec) { - // Update receive feedback parameters from new codec. + if (changed_params.codec || changed_params.rtcp_mode) { + // Update receive feedback parameters from new codec or RTCP mode. LOG(LS_INFO) << "SetFeedbackOptions on all the receive streams because the send " - "codec has changed."; + "codec or RTCP mode has changed."; for (auto& kv : receive_streams_) { RTC_DCHECK(kv.second != nullptr); - kv.second->SetFeedbackParameters(HasNack(send_codec_->codec), - HasRemb(send_codec_->codec), - HasTransportCc(send_codec_->codec)); + kv.second->SetFeedbackParameters( + HasNack(send_codec_->codec), HasRemb(send_codec_->codec), + HasTransportCc(send_codec_->codec), + params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize + : webrtc::RtcpMode::kCompound); } } } @@ -882,13 +884,6 @@ bool WebRtcVideoChannel2::GetChangedRecvParameters( rtc::Optional>(filtered_extensions); } - // Handle RTCP mode. - if (params.rtcp.reduced_size != recv_params_.rtcp.reduced_size) { - changed_params->rtcp_mode = rtc::Optional( - params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize - : webrtc::RtcpMode::kCompound); - } - return true; } @@ -1159,7 +1154,12 @@ void WebRtcVideoChannel2::ConfigureReceiverRtp( config->rtp.local_ssrc = rtcp_receiver_report_ssrc_; config->rtp.extensions = recv_rtp_extensions_; - config->rtp.rtcp_mode = recv_params_.rtcp.reduced_size + // 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; @@ -2294,11 +2294,13 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetLocalSsrc( void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters( bool nack_enabled, bool remb_enabled, - bool transport_cc_enabled) { + bool transport_cc_enabled, + webrtc::RtcpMode rtcp_mode) { int nack_history_ms = nack_enabled ? kNackHistoryMs : 0; if (config_.rtp.nack.rtp_history_ms == nack_history_ms && config_.rtp.remb == remb_enabled && - config_.rtp.transport_cc == transport_cc_enabled) { + config_.rtp.transport_cc == transport_cc_enabled && + config_.rtp.rtcp_mode == rtcp_mode) { LOG(LS_INFO) << "Ignoring call to SetFeedbackParameters because parameters are " "unchanged; nack=" @@ -2309,6 +2311,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetFeedbackParameters( config_.rtp.remb = remb_enabled; config_.rtp.nack.rtp_history_ms = nack_history_ms; config_.rtp.transport_cc = transport_cc_enabled; + config_.rtp.rtcp_mode = rtcp_mode; LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetFeedbackParameters; nack=" << nack_enabled << ", remb=" << remb_enabled @@ -2328,10 +2331,6 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvParameters( config_.rtp.extensions = *params.rtp_header_extensions; needs_recreation = true; } - if (params.rtcp_mode) { - config_.rtp.rtcp_mode = *params.rtcp_mode; - needs_recreation = true; - } if (needs_recreation) { LOG(LS_INFO) << "RecreateWebRtcStream (recv) because of SetRecvParameters"; RecreateWebRtcStream(); diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 850bcaef63..7ffd85343b 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -200,7 +200,6 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { // These optionals are unset if not changed. rtc::Optional> codec_settings; rtc::Optional> rtp_header_extensions; - rtc::Optional rtcp_mode; }; bool GetChangedSendParameters(const VideoSendParameters& params, @@ -411,9 +410,11 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { const std::vector& GetSsrcs() const; void SetLocalSsrc(uint32_t local_ssrc); + // TODO(deadbeef): Move these feedback parameters into the recv parameters. void SetFeedbackParameters(bool nack_enabled, bool remb_enabled, - bool transport_cc_enabled); + bool transport_cc_enabled, + webrtc::RtcpMode rtcp_mode); void SetRecvParameters(const ChangedRecvParameters& recv_params); void RenderFrame(const webrtc::VideoFrame& frame, diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index b44eb9907d..64c691ff8d 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -2575,8 +2575,10 @@ TEST_F(WebRtcVideoChannel2Test, TestSetRecvRtcpReducedSize) { EXPECT_EQ(webrtc::RtcpMode::kCompound, stream1->GetConfig().rtp.rtcp_mode); // Now enable reduced size mode. - recv_parameters_.rtcp.reduced_size = true; - EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_)); + // TODO(deadbeef): Once "recv_parameters" becomes "receiver_parameters", + // the reduced_size flag should come from that. + send_parameters_.rtcp.reduced_size = true; + EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); stream1 = fake_call_->GetVideoReceiveStreams()[0]; EXPECT_EQ(webrtc::RtcpMode::kReducedSize, stream1->GetConfig().rtp.rtcp_mode); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 0873254f63..1bfa7cdebc 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -47,7 +47,6 @@ RTCPReceiver::RTCPReceiver( : TMMBRHelp(), _clock(clock), receiver_only_(receiver_only), - _method(RtcpMode::kOff), _lastReceived(0), _rtpRtcp(*owner), _criticalSectionFeedbacks( @@ -103,16 +102,6 @@ RTCPReceiver::~RTCPReceiver() { } } -RtcpMode RTCPReceiver::Status() const { - CriticalSectionScoped lock(_criticalSectionRTCPReceiver); - return _method; -} - -void RTCPReceiver::SetRTCPStatus(RtcpMode method) { - CriticalSectionScoped lock(_criticalSectionRTCPReceiver); - _method = method; -} - int64_t RTCPReceiver::LastReceived() { CriticalSectionScoped lock(_criticalSectionRTCPReceiver); return _lastReceived; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 1930670064..475ab1e26f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -38,9 +38,6 @@ public: ModuleRtpRtcpImpl* owner); virtual ~RTCPReceiver(); - RtcpMode Status() const; - void SetRTCPStatus(RtcpMode method); - int64_t LastReceived(); int64_t LastReceivedReceiverReport() const; @@ -267,7 +264,6 @@ protected: Clock* const _clock; const bool receiver_only_; - RtcpMode _method; int64_t _lastReceived; ModuleRtpRtcpImpl& _rtpRtcp; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index ef720bef52..581aa5111a 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -497,16 +497,12 @@ int32_t ModuleRtpRtcpImpl::SetMaxTransferUnit(const uint16_t mtu) { } RtcpMode ModuleRtpRtcpImpl::RTCP() const { - if (rtcp_sender_.Status() != RtcpMode::kOff) { - return rtcp_receiver_.Status(); - } - return RtcpMode::kOff; + return rtcp_sender_.Status(); } // Configure RTCP status i.e on/off. void ModuleRtpRtcpImpl::SetRTCPStatus(const RtcpMode method) { rtcp_sender_.SetRTCPStatus(method); - rtcp_receiver_.SetRTCPStatus(method); } int32_t ModuleRtpRtcpImpl::SetCNAME(const char* c_name) { diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 6a98f7d076..a9d1b95c22 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -755,11 +755,9 @@ static bool CreateMediaContentOffer( offer->set_crypto_required(CT_SDES); } offer->set_rtcp_mux(options.rtcp_mux_enabled); - // TODO(deadbeef): Once we're sure this works correctly, enable it in - // CreateOffer. - // if (offer->type() == cricket::MEDIA_TYPE_VIDEO) { - // offer->set_rtcp_reduced_size(true); - // } + if (offer->type() == cricket::MEDIA_TYPE_VIDEO) { + offer->set_rtcp_reduced_size(true); + } offer->set_multistream(options.is_muc); offer->set_rtp_header_extensions(rtp_extensions); @@ -1053,11 +1051,9 @@ static bool CreateMediaContentAnswer( answer->set_rtp_header_extensions(negotiated_rtp_extensions); answer->set_rtcp_mux(options.rtcp_mux_enabled && offer->rtcp_mux()); - // TODO(deadbeef): Once we're sure this works correctly, enable it in - // CreateAnswer. - // if (answer->type() == cricket::MEDIA_TYPE_VIDEO) { - // answer->set_rtcp_reduced_size(offer->rtcp_reduced_size()); - // } + if (answer->type() == cricket::MEDIA_TYPE_VIDEO) { + answer->set_rtcp_reduced_size(offer->rtcp_reduced_size()); + } if (sdes_policy != SEC_DISABLED) { CryptoParams crypto; diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 21a22dc914..6682396fcd 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -69,6 +69,9 @@ std::string VideoSendStream::Config::Rtp::ToString() const { ss << ", "; } ss << ']'; + ss << ", rtcp_mode: " + << (rtcp_mode == RtcpMode::kCompound ? "RtcpMode::kCompound" + : "RtcpMode::kReducedSize"); ss << ", max_packet_size: " << max_packet_size; ss << ", extensions: ["; for (size_t i = 0; i < extensions.size(); ++i) {