From ba8c15b857c0f341d9c1e02d41b6ccd56f9f1030 Mon Sep 17 00:00:00 2001 From: pbos Date: Tue, 14 Jul 2015 09:36:34 -0700 Subject: [PATCH] Merge methods for configuring NACK/FEC/hybrid. BUG=webrtc:1695 R=stefan@webrtc.org Review URL: https://codereview.webrtc.org/1226143013 Cr-Commit-Position: refs/heads/master@{#9580} --- webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h | 12 +- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 10 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 15 +-- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 12 +- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 18 +-- webrtc/modules/rtp_rtcp/source/rtp_sender.h | 11 +- .../main/interface/video_coding_defines.h | 2 - .../main/source/media_opt_util.cc | 34 ++---- .../video_coding/main/source/media_opt_util.h | 3 +- .../main/source/media_optimization.cc | 7 +- .../main/source/media_optimization.h | 2 +- .../main/source/video_coding_impl.cc | 4 +- .../main/source/video_coding_impl.h | 2 +- .../main/source/video_receiver.cc | 35 ++---- .../video_coding/main/source/video_sender.cc | 13 +-- webrtc/video/video_receive_stream.cc | 3 +- webrtc/video/video_send_stream.cc | 28 ++--- webrtc/video_engine/vie_channel.cc | 109 ++++++------------ webrtc/video_engine/vie_channel.h | 21 +--- webrtc/video_engine/vie_encoder.cc | 11 +- 20 files changed, 126 insertions(+), 226 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index 7a960881c9..3dccee8a46 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -608,19 +608,15 @@ class RtpRtcp : public Module { /* * Turn on/off generic FEC - * - * return -1 on failure else 0 */ - virtual int32_t SetGenericFECStatus(bool enable, - uint8_t payloadTypeRED, - uint8_t payloadTypeFEC) = 0; + virtual void SetGenericFECStatus(bool enable, + uint8_t payload_type_red, + uint8_t payload_type_fec) = 0; /* * Get generic FEC setting - * - * return -1 on failure else 0 */ - virtual int32_t GenericFECStatus(bool& enable, + virtual void GenericFECStatus(bool& enable, uint8_t& payloadTypeRED, uint8_t& payloadTypeFEC) = 0; diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 127a795e0b..56291ef188 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -227,11 +227,13 @@ class MockRtpRtcp : public RtpRtcp { MOCK_METHOD1(SetTargetSendBitrate, void(uint32_t bitrate_bps)); MOCK_METHOD3(SetGenericFECStatus, - int32_t(const bool enable, - const uint8_t payloadTypeRED, - const uint8_t payloadTypeFEC)); + void(const bool enable, + const uint8_t payload_type_red, + const uint8_t payload_type_fec)); MOCK_METHOD3(GenericFECStatus, - int32_t(bool& enable, uint8_t& payloadTypeRED, uint8_t& payloadTypeFEC)); + void(bool& enable, + uint8_t& payloadTypeRED, + uint8_t& payloadTypeFEC)); MOCK_METHOD2(SetFecParameters, int32_t(const FecProtectionParams* delta_params, const FecProtectionParams* key_params)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 9e05b767c4..b92ac5920f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -834,20 +834,17 @@ int32_t ModuleRtpRtcpImpl::SendRTCPSliceLossIndication( GetFeedbackState(), kRtcpSli, 0, 0, false, picture_id); } -int32_t ModuleRtpRtcpImpl::SetGenericFECStatus( +void ModuleRtpRtcpImpl::SetGenericFECStatus( const bool enable, const uint8_t payload_type_red, const uint8_t payload_type_fec) { - return rtp_sender_.SetGenericFECStatus(enable, - payload_type_red, - payload_type_fec); + rtp_sender_.SetGenericFECStatus(enable, payload_type_red, payload_type_fec); } -int32_t ModuleRtpRtcpImpl::GenericFECStatus( - bool& enable, - uint8_t& payload_type_red, - uint8_t& payload_type_fec) { - return rtp_sender_.GenericFECStatus(&enable, &payload_type_red, +void ModuleRtpRtcpImpl::GenericFECStatus(bool& enable, + uint8_t& payload_type_red, + uint8_t& payload_type_fec) { + rtp_sender_.GenericFECStatus(&enable, &payload_type_red, &payload_type_fec); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 791495a8ba..f4bed46e68 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -280,13 +280,13 @@ class ModuleRtpRtcpImpl : public RtpRtcp { void SetTargetSendBitrate(uint32_t bitrate_bps) override; - int32_t SetGenericFECStatus(bool enable, - uint8_t payload_type_red, - uint8_t payload_type_fec) override; + void SetGenericFECStatus(bool enable, + uint8_t payload_type_red, + uint8_t payload_type_fec) override; - int32_t GenericFECStatus(bool& enable, - uint8_t& payload_type_red, - uint8_t& payload_type_fec) override; + void GenericFECStatus(bool& enable, + uint8_t& payload_type_red, + uint8_t& payload_type_fec) override; int32_t SetFecParameters(const FecProtectionParams* delta_params, const FecProtectionParams* key_params) override; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index f7f8bb05ac..e7306a11bd 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -1736,24 +1736,18 @@ int32_t RTPSender::SendRTPIntraRequest() { return video_->SendRTPIntraRequest(); } -int32_t RTPSender::SetGenericFECStatus(bool enable, - uint8_t payload_type_red, - uint8_t payload_type_fec) { - if (audio_configured_) { - return -1; - } +void RTPSender::SetGenericFECStatus(bool enable, + uint8_t payload_type_red, + uint8_t payload_type_fec) { + DCHECK(!audio_configured_); video_->SetGenericFECStatus(enable, payload_type_red, payload_type_fec); - return 0; } -int32_t RTPSender::GenericFECStatus(bool* enable, +void RTPSender::GenericFECStatus(bool* enable, uint8_t* payload_type_red, uint8_t* payload_type_fec) const { - if (audio_configured_) { - return -1; - } + DCHECK(!audio_configured_); video_->GenericFECStatus(*enable, *payload_type_red, *payload_type_fec); - return 0; } int32_t RTPSender::SetFecParameters( diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h index 61a1fb5ec7..a4703989d6 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -263,12 +263,13 @@ class RTPSender : public RTPSenderInterface { int32_t SendRTPIntraRequest(); // FEC. - int32_t SetGenericFECStatus(bool enable, - uint8_t payload_type_red, - uint8_t payload_type_fec); + void SetGenericFECStatus(bool enable, + uint8_t payload_type_red, + uint8_t payload_type_fec); - int32_t GenericFECStatus(bool *enable, uint8_t *payload_type_red, - uint8_t *payload_type_fec) const; + void GenericFECStatus(bool* enable, + uint8_t* payload_type_red, + uint8_t* payload_type_fec) const; int32_t SetFecParameters(const FecProtectionParams *delta_params, const FecProtectionParams *key_params); diff --git a/webrtc/modules/video_coding/main/interface/video_coding_defines.h b/webrtc/modules/video_coding/main/interface/video_coding_defines.h index d40023c2b1..138a3ec721 100644 --- a/webrtc/modules/video_coding/main/interface/video_coding_defines.h +++ b/webrtc/modules/video_coding/main/interface/video_coding_defines.h @@ -41,8 +41,6 @@ enum { kDefaultStartBitrateKbps = 300 }; enum VCMVideoProtection { kProtectionNone, kProtectionNack, // Both send-side and receive-side - kProtectionNackSender, // Send-side only - kProtectionNackReceiver, // Receive-side only kProtectionFEC, kProtectionNackFEC, kProtectionKeyOnLoss, diff --git a/webrtc/modules/video_coding/main/source/media_opt_util.cc b/webrtc/modules/video_coding/main/source/media_opt_util.cc index d929cbc35a..51decbed97 100644 --- a/webrtc/modules/video_coding/main/source/media_opt_util.cc +++ b/webrtc/modules/video_coding/main/source/media_opt_util.cc @@ -519,7 +519,6 @@ VCMFecMethod::UpdateParameters(const VCMProtectionParameters* parameters) return true; } VCMLossProtectionLogic::VCMLossProtectionLogic(int64_t nowMs): -_selectedMethod(NULL), _currentParameters(), _rtt(0), _lossPr(0.0f), @@ -548,25 +547,21 @@ VCMLossProtectionLogic::~VCMLossProtectionLogic() void VCMLossProtectionLogic::SetMethod( enum VCMProtectionMethodEnum newMethodType) { - if (_selectedMethod != nullptr) { - if (_selectedMethod->Type() == newMethodType) - return; - // Remove old method. - delete _selectedMethod; - } + if (_selectedMethod && _selectedMethod->Type() == newMethodType) + return; switch(newMethodType) { case kNack: - _selectedMethod = new VCMNackMethod(); + _selectedMethod.reset(new VCMNackMethod()); break; case kFec: - _selectedMethod = new VCMFecMethod(); + _selectedMethod.reset(new VCMFecMethod()); break; case kNackFec: - _selectedMethod = new VCMNackFecMethod(kLowRttNackMs, -1); + _selectedMethod.reset(new VCMNackFecMethod(kLowRttNackMs, -1)); break; case kNone: - _selectedMethod = nullptr; + _selectedMethod.reset(); break; } UpdateMethod(); @@ -726,10 +721,8 @@ void VCMLossProtectionLogic::UpdateNumLayers(int numLayers) { bool VCMLossProtectionLogic::UpdateMethod() { - if (_selectedMethod == NULL) - { - return false; - } + if (!_selectedMethod) + return false; _currentParameters.rtt = _rtt; _currentParameters.lossPr = _lossPr; _currentParameters.bitRate = _bitRate; @@ -748,11 +741,11 @@ VCMLossProtectionLogic::UpdateMethod() VCMProtectionMethod* VCMLossProtectionLogic::SelectedMethod() const { - return _selectedMethod; + return _selectedMethod.get(); } VCMProtectionMethodEnum VCMLossProtectionLogic::SelectedType() const { - return _selectedMethod == nullptr ? kNone : _selectedMethod->Type(); + return _selectedMethod ? _selectedMethod->Type() : kNone; } void @@ -773,11 +766,8 @@ VCMLossProtectionLogic::Reset(int64_t nowMs) Release(); } -void -VCMLossProtectionLogic::Release() -{ - delete _selectedMethod; - _selectedMethod = NULL; +void VCMLossProtectionLogic::Release() { + _selectedMethod.reset(); } } // namespace media_optimization diff --git a/webrtc/modules/video_coding/main/source/media_opt_util.h b/webrtc/modules/video_coding/main/source/media_opt_util.h index 498238768f..62d067ab3a 100644 --- a/webrtc/modules/video_coding/main/source/media_opt_util.h +++ b/webrtc/modules/video_coding/main/source/media_opt_util.h @@ -15,6 +15,7 @@ #include #include "webrtc/base/exp_filter.h" +#include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/video_coding/main/source/internal_defines.h" #include "webrtc/modules/video_coding/main/source/qm_select.h" #include "webrtc/system_wrappers/interface/trace.h" @@ -335,7 +336,7 @@ private: // Sets the available loss protection methods. void UpdateMaxLossHistory(uint8_t lossPr255, int64_t now); uint8_t MaxFilteredLossPr(int64_t nowMs) const; - VCMProtectionMethod* _selectedMethod; + rtc::scoped_ptr _selectedMethod; VCMProtectionParameters _currentParameters; int64_t _rtt; float _lossPr; diff --git a/webrtc/modules/video_coding/main/source/media_optimization.cc b/webrtc/modules/video_coding/main/source/media_optimization.cc index 4dbdf44d58..7f60c6c999 100644 --- a/webrtc/modules/video_coding/main/source/media_optimization.cc +++ b/webrtc/modules/video_coding/main/source/media_optimization.cc @@ -317,13 +317,8 @@ uint32_t MediaOptimization::SetTargetRates( return target_bit_rate_; } -void MediaOptimization::EnableProtectionMethod(bool enable, - VCMProtectionMethodEnum method) { +void MediaOptimization::SetProtectionMethod(VCMProtectionMethodEnum method) { CriticalSectionScoped lock(crit_sect_.get()); - if (!enable && loss_prot_logic_->SelectedType() != method) - return; - if (!enable) - method = kNone; loss_prot_logic_->SetMethod(method); } diff --git a/webrtc/modules/video_coding/main/source/media_optimization.h b/webrtc/modules/video_coding/main/source/media_optimization.h index e0010db4e2..c3bb3a8599 100644 --- a/webrtc/modules/video_coding/main/source/media_optimization.h +++ b/webrtc/modules/video_coding/main/source/media_optimization.h @@ -62,7 +62,7 @@ class MediaOptimization { VCMProtectionCallback* protection_callback, VCMQMSettingsCallback* qmsettings_callback); - void EnableProtectionMethod(bool enable, VCMProtectionMethodEnum method); + void SetProtectionMethod(VCMProtectionMethodEnum method); void EnableQM(bool enable); void EnableFrameDropper(bool enable); diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.cc b/webrtc/modules/video_coding/main/source/video_coding_impl.cc index c207f00f0e..e0cf479623 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl.cc +++ b/webrtc/modules/video_coding/main/source/video_coding_impl.cc @@ -168,7 +168,9 @@ class VideoCodingModuleImpl : public VideoCodingModule { int32_t SetVideoProtection(VCMVideoProtection videoProtection, bool enable) override { - sender_->SetVideoProtection(enable, videoProtection); + // TODO(pbos): Remove enable from receive-side protection modes as well. + if (enable) + sender_->SetVideoProtection(videoProtection); return receiver_->SetVideoProtection(videoProtection, enable); } diff --git a/webrtc/modules/video_coding/main/source/video_coding_impl.h b/webrtc/modules/video_coding/main/source/video_coding_impl.h index d738a7cef2..575d6551ab 100644 --- a/webrtc/modules/video_coding/main/source/video_coding_impl.h +++ b/webrtc/modules/video_coding/main/source/video_coding_impl.h @@ -98,7 +98,7 @@ class VideoSender { int32_t RegisterTransportCallback(VCMPacketizationCallback* transport); int32_t RegisterSendStatisticsCallback(VCMSendStatisticsCallback* sendStats); int32_t RegisterProtectionCallback(VCMProtectionCallback* protection); - void SetVideoProtection(bool enable, VCMVideoProtection videoProtection); + void SetVideoProtection(VCMVideoProtection videoProtection); int32_t AddVideoFrame(const VideoFrame& videoFrame, const VideoContentMetrics* _contentMetrics, diff --git a/webrtc/modules/video_coding/main/source/video_receiver.cc b/webrtc/modules/video_coding/main/source/video_receiver.cc index 08e6208c36..b728e9b380 100644 --- a/webrtc/modules/video_coding/main/source/video_receiver.cc +++ b/webrtc/modules/video_coding/main/source/video_receiver.cc @@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "webrtc/base/checks.h" #include "webrtc/common_types.h" #include "webrtc/common_video/libyuv/include/webrtc_libyuv.h" #include "webrtc/modules/video_coding/codecs/interface/video_codec_interface.h" @@ -187,15 +188,9 @@ int32_t VideoReceiver::SetVideoProtection(VCMVideoProtection videoProtection, // By default, do not decode with errors. _receiver.SetDecodeErrorMode(kNoErrors); switch (videoProtection) { - case kProtectionNack: - case kProtectionNackReceiver: { - CriticalSectionScoped cs(_receiveCritSect); - if (enable) { - // Enable NACK and always wait for retransmits. - _receiver.SetNackMode(kNack, -1, -1); - } else { - _receiver.SetNackMode(kNoNack, -1, -1); - } + case kProtectionNack: { + DCHECK(enable); + _receiver.SetNackMode(kNack, -1, -1); break; } @@ -226,25 +221,17 @@ int32_t VideoReceiver::SetVideoProtection(VCMVideoProtection videoProtection, case kProtectionNackFEC: { CriticalSectionScoped cs(_receiveCritSect); - if (enable) { - // Enable hybrid NACK/FEC. Always wait for retransmissions - // and don't add extra delay when RTT is above - // kLowRttNackMs. - _receiver.SetNackMode(kNack, media_optimization::kLowRttNackMs, -1); - _receiver.SetDecodeErrorMode(kNoErrors); - _receiver.SetDecodeErrorMode(kNoErrors); - } else { - _receiver.SetNackMode(kNoNack, -1, -1); - } + DCHECK(enable); + _receiver.SetNackMode(kNack, media_optimization::kLowRttNackMs, -1); + _receiver.SetDecodeErrorMode(kNoErrors); break; } - case kProtectionNackSender: case kProtectionFEC: - // Ignore encoder modes. - return VCM_OK; case kProtectionNone: - // TODO(pbos): Implement like sender and remove enable parameter. Ignored - // for now. + // No receiver-side protection. + DCHECK(enable); + _receiver.SetNackMode(kNoNack, -1, -1); + _receiver.SetDecodeErrorMode(kWithErrors); break; } return VCM_OK; diff --git a/webrtc/modules/video_coding/main/source/video_sender.cc b/webrtc/modules/video_coding/main/source/video_sender.cc index 93d82084f3..1be2f06803 100644 --- a/webrtc/modules/video_coding/main/source/video_sender.cc +++ b/webrtc/modules/video_coding/main/source/video_sender.cc @@ -296,24 +296,21 @@ int32_t VideoSender::RegisterProtectionCallback( } // Enable or disable a video protection method. -void VideoSender::SetVideoProtection(bool enable, - VCMVideoProtection videoProtection) { +void VideoSender::SetVideoProtection(VCMVideoProtection videoProtection) { CriticalSectionScoped cs(_sendCritSect); switch (videoProtection) { case kProtectionNone: - _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNone); + _mediaOpt.SetProtectionMethod(media_optimization::kNone); break; case kProtectionNack: - case kProtectionNackSender: - _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNack); + _mediaOpt.SetProtectionMethod(media_optimization::kNack); break; case kProtectionNackFEC: - _mediaOpt.EnableProtectionMethod(enable, media_optimization::kNackFec); + _mediaOpt.SetProtectionMethod(media_optimization::kNackFec); break; case kProtectionFEC: - _mediaOpt.EnableProtectionMethod(enable, media_optimization::kFec); + _mediaOpt.SetProtectionMethod(media_optimization::kFec); break; - case kProtectionNackReceiver: case kProtectionKeyOnLoss: case kProtectionKeyOnKeyLoss: // Ignore receiver modes. diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 91a37076f3..d0c17d649e 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -144,7 +144,8 @@ VideoReceiveStream::VideoReceiveStream(int num_cpu_cores, vie_channel_ = channel_group_->GetChannel(channel_id_); // TODO(pbos): This is not fine grained enough... - vie_channel_->SetNACKStatus(config_.rtp.nack.rtp_history_ms > 0); + vie_channel_->SetProtectionMode(config_.rtp.nack.rtp_history_ms > 0, false, + -1, -1); vie_channel_->SetKeyFrameRequestMethod(kKeyFrameReqPliRtcp); SetRtcpMode(config_.rtp.rtcp_mode); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 9001897323..0b59c8bba2 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -147,28 +147,14 @@ VideoSendStream::VideoSendStream( channel_group_->SetChannelRembStatus(true, false, vie_channel_); // Enable NACK, FEC or both. - bool enable_protection_nack = false; - bool enable_protection_fec = false; - if (config_.rtp.fec.red_payload_type != -1) { - enable_protection_fec = true; - DCHECK(config_.rtp.fec.ulpfec_payload_type != -1); - if (config_.rtp.nack.rtp_history_ms > 0) { - enable_protection_nack = true; - vie_channel_->SetHybridNACKFECStatus( - true, static_cast(config_.rtp.fec.red_payload_type), - static_cast(config_.rtp.fec.ulpfec_payload_type)); - } else { - vie_channel_->SetFECStatus( - true, static_cast(config_.rtp.fec.red_payload_type), - static_cast(config_.rtp.fec.ulpfec_payload_type)); - } - // TODO(changbin): Should set RTX for RED mapping in RTP sender in future. - } else { - enable_protection_nack = config_.rtp.nack.rtp_history_ms > 0; - vie_channel_->SetNACKStatus(config_.rtp.nack.rtp_history_ms > 0); - } + const bool enable_protection_nack = config_.rtp.nack.rtp_history_ms > 0; + const bool enable_protection_fec = config_.rtp.fec.red_payload_type != -1; + // TODO(changbin): Should set RTX for RED mapping in RTP sender in future. + vie_channel_->SetProtectionMode(enable_protection_nack, enable_protection_fec, + config_.rtp.fec.red_payload_type, + config_.rtp.fec.ulpfec_payload_type); vie_encoder_->UpdateProtectionMethod(enable_protection_nack, - enable_protection_fec); + enable_protection_fec); ConfigureSsrcs(); diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index 0581226220..d0465b209d 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -496,54 +496,54 @@ int ViEChannel::ReceiveDelay() const { return vcm_->Delay(); } -int32_t ViEChannel::SetSignalPacketLossStatus(bool enable, - bool only_key_frames) { - if (enable) { - if (only_key_frames) { - vcm_->SetVideoProtection(kProtectionKeyOnLoss, false); - if (vcm_->SetVideoProtection(kProtectionKeyOnKeyLoss, true) != VCM_OK) { - return -1; - } - } else { - vcm_->SetVideoProtection(kProtectionKeyOnKeyLoss, false); - if (vcm_->SetVideoProtection(kProtectionKeyOnLoss, true) != VCM_OK) { - return -1; - } - } - } else { - vcm_->SetVideoProtection(kProtectionKeyOnLoss, false); - vcm_->SetVideoProtection(kProtectionKeyOnKeyLoss, false); - } - return 0; -} - void ViEChannel::SetRTCPMode(const RTCPMethod rtcp_mode) { for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) rtp_rtcp->SetRTCPStatus(rtcp_mode); } -int32_t ViEChannel::SetNACKStatus(const bool enable) { - // Update the decoding VCM. - if (vcm_->SetVideoProtection(kProtectionNack, enable) != VCM_OK) { - return -1; +void ViEChannel::SetProtectionMode(bool enable_nack, + bool enable_fec, + int payload_type_red, + int payload_type_fec) { + // Validate payload types. + if (enable_fec) { + DCHECK_GE(payload_type_red, 0); + DCHECK_GE(payload_type_fec, 0); + DCHECK_LE(payload_type_red, 127); + DCHECK_LE(payload_type_fec, 127); + } else { + DCHECK_EQ(payload_type_red, -1); + DCHECK_EQ(payload_type_fec, -1); + // Set to valid uint8_ts to be castable later without signed overflows. + payload_type_red = 0; + payload_type_fec = 0; } - if (enable) { - // Disable possible FEC. - SetFECStatus(false, 0, 0); + + VCMVideoProtection protection_method; + if (enable_nack) { + protection_method = enable_fec ? kProtectionNackFEC : kProtectionNack; + } else { + protection_method = kProtectionNone; } - // Update the decoding VCM. - if (vcm_->SetVideoProtection(kProtectionNack, enable) != VCM_OK) { - return -1; + + vcm_->SetVideoProtection(protection_method, true); + + // Set NACK. + ProcessNACKRequest(enable_nack); + + // Set FEC. + for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) { + rtp_rtcp->SetGenericFECStatus(enable_fec, + static_cast(payload_type_red), + static_cast(payload_type_fec)); } - return ProcessNACKRequest(enable); } -int32_t ViEChannel::ProcessNACKRequest(const bool enable) { +void ViEChannel::ProcessNACKRequest(const bool enable) { if (enable) { // Turn on NACK. - if (rtp_rtcp_modules_[0]->RTCP() == kRtcpOff) { - return -1; - } + if (rtp_rtcp_modules_[0]->RTCP() == kRtcpOff) + return; vie_receiver_.SetNackStatus(true, max_nack_reordering_threshold_); for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) @@ -563,18 +563,6 @@ int32_t ViEChannel::ProcessNACKRequest(const bool enable) { // will freeze, and will only recover with a complete key frame. vcm_->SetDecodeErrorMode(kWithErrors); } - return 0; -} - -int32_t ViEChannel::SetFECStatus(const bool enable, - const unsigned char payload_typeRED, - const unsigned char payload_typeFEC) { - // Disable possible NACK. - if (enable) { - SetNACKStatus(false); - } - - return ProcessFECRequest(enable, payload_typeRED, payload_typeFEC); } bool ViEChannel::IsSendingFecEnabled() { @@ -590,31 +578,6 @@ bool ViEChannel::IsSendingFecEnabled() { return false; } -int32_t ViEChannel::ProcessFECRequest( - const bool enable, - const unsigned char payload_typeRED, - const unsigned char payload_typeFEC) { - for (RtpRtcp* rtp_rtcp : rtp_rtcp_modules_) - rtp_rtcp->SetGenericFECStatus(enable, payload_typeRED, payload_typeFEC); - return 0; -} - -int32_t ViEChannel::SetHybridNACKFECStatus( - const bool enable, - const unsigned char payload_typeRED, - const unsigned char payload_typeFEC) { - if (vcm_->SetVideoProtection(kProtectionNackFEC, enable) != VCM_OK) { - return -1; - } - - int32_t ret_val = 0; - ret_val = ProcessNACKRequest(enable); - if (ret_val < 0) { - return ret_val; - } - return ProcessFECRequest(enable, payload_typeRED, payload_typeFEC); -} - int ViEChannel::SetSenderBufferingMode(int target_delay_ms) { if ((target_delay_ms < 0) || (target_delay_ms > kMaxTargetDelayMs)) { LOG(LS_ERROR) << "Invalid send buffer value."; diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index 0f8477d4ef..188fe3c3a4 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -136,19 +136,11 @@ class ViEChannel : public VCMFrameTypeCallback, // Returns the estimated delay in milliseconds. int ReceiveDelay() const; - // If enabled, a key frame request will be sent as soon as there are lost - // packets. If |only_key_frames| are set, requests are only sent for loss in - // key frames. - int32_t SetSignalPacketLossStatus(bool enable, bool only_key_frames); - void SetRTCPMode(const RTCPMethod rtcp_mode); - int32_t SetNACKStatus(const bool enable); - int32_t SetFECStatus(const bool enable, - const unsigned char payload_typeRED, - const unsigned char payload_typeFEC); - int32_t SetHybridNACKFECStatus(const bool enable, - const unsigned char payload_typeRED, - const unsigned char payload_typeFEC); + void SetProtectionMode(bool enable_nack, + bool enable_fec, + int payload_type_red, + int payload_type_fec); bool IsSendingFecEnabled(); int SetSenderBufferingMode(int target_delay_ms); int SetReceiverBufferingMode(int target_delay_ms); @@ -354,10 +346,7 @@ class ViEChannel : public VCMFrameTypeCallback, void StartDecodeThread(); void StopDecodeThread(); - int32_t ProcessNACKRequest(const bool enable); - int32_t ProcessFECRequest(const bool enable, - const unsigned char payload_typeRED, - const unsigned char payload_typeFEC); + void ProcessNACKRequest(const bool enable); // Compute NACK list parameters for the buffering mode. int GetRequiredNackListSize(int target_delay_ms); void SetRtxSendStatus(bool enable); diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc index 0eea5f5506..26e76e7de8 100644 --- a/webrtc/video_engine/vie_encoder.cc +++ b/webrtc/video_engine/vie_encoder.cc @@ -612,13 +612,14 @@ int32_t ViEEncoder::UpdateProtectionMethod(bool nack, bool fec) { nack_enabled_ = nack; // Set Video Protection for VCM. - if (fec_enabled_ && nack_enabled_) { - vcm_->SetVideoProtection(webrtc::kProtectionNackFEC, true); + VCMVideoProtection protection_mode; + if (fec_enabled_) { + protection_mode = + nack_enabled_ ? webrtc::kProtectionNackFEC : kProtectionFEC; } else { - vcm_->SetVideoProtection(webrtc::kProtectionFEC, fec_enabled_); - vcm_->SetVideoProtection(webrtc::kProtectionNackSender, nack_enabled_); - vcm_->SetVideoProtection(webrtc::kProtectionNackFEC, false); + protection_mode = nack_enabled_ ? kProtectionNack : kProtectionNone; } + vcm_->SetVideoProtection(protection_mode, true); if (fec_enabled_ || nack_enabled_) { // The send codec must be registered to set correct MTU.