From 73894369791cb5eedc8788baf918ec07d11d351d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Tue, 26 Apr 2016 23:54:27 +0200 Subject: [PATCH] Remove VCMQmRobustness. Class contained a lot of not-really-wired-up functionality that ended up being complicated ways of saying return 1; or return false;. This removes this dependency that complicates code readability significantly. BUG=webrtc:5066 R=marpan@google.com, marpan@webrtc.org TBR=stefan@webrtc.org Review URL: https://codereview.webrtc.org/1917083003 . Cr-Commit-Position: refs/heads/master@{#12516} --- webrtc/modules/include/module_common_types.h | 1 - .../modules/rtp_rtcp/source/producer_fec.cc | 11 ++-- .../rtp_rtcp/source/producer_fec_unittest.cc | 6 +-- .../rtp_rtcp/source/rtp_sender_unittest.cc | 1 - webrtc/modules/video_coding/media_opt_util.cc | 29 +---------- webrtc/modules/video_coding/media_opt_util.h | 4 -- .../video_coding/media_optimization.cc | 10 ---- webrtc/modules/video_coding/qm_select.cc | 52 ------------------- webrtc/modules/video_coding/qm_select.h | 30 ----------- 9 files changed, 9 insertions(+), 135 deletions(-) diff --git a/webrtc/modules/include/module_common_types.h b/webrtc/modules/include/module_common_types.h index 82d87d5c5c..a2ab766ce5 100644 --- a/webrtc/modules/include/module_common_types.h +++ b/webrtc/modules/include/module_common_types.h @@ -432,7 +432,6 @@ enum FecMaskType { // Struct containing forward error correction settings. struct FecProtectionParams { int fec_rate; - bool use_uep_protection; int max_fec_frames; FecMaskType fec_mask_type; }; diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc index 69a28ed4db..c7ea19db58 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc @@ -157,12 +157,11 @@ int ProducerFec::AddRtpPacketAndGenerateFec(const uint8_t* data_buffer, (ExcessOverheadBelowMax() && MinimumMediaPacketsReached()))) { assert(num_first_partition_ <= static_cast(ForwardErrorCorrection::kMaxMediaPackets)); - int ret = fec_->GenerateFEC(media_packets_fec_, - params_.fec_rate, - num_first_partition_, - params_.use_uep_protection, - params_.fec_mask_type, - &fec_packets_); + // TODO(pbos): Consider whether unequal protection should be enabled or not, + // it is currently always disabled. + int ret = fec_->GenerateFEC(media_packets_fec_, params_.fec_rate, + num_first_partition_, false, + params_.fec_mask_type, &fec_packets_); if (fec_packets_.empty()) { num_frames_ = 0; DeletePackets(); diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc index fad0f502f5..96c564a714 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc @@ -78,7 +78,7 @@ TEST_F(ProducerFecTest, NoEmptyFecWithSeqNumGaps) { protected_packets.push_back({12, 3, 54, 0}); protected_packets.push_back({21, 0, 55, 0}); protected_packets.push_back({13, 3, 57, 1}); - FecProtectionParams params = {117, 0, 3, kFecMaskBursty}; + FecProtectionParams params = {117, 3, kFecMaskBursty}; producer_->SetFecParameters(¶ms, 0); uint8_t packet[28] = {0}; for (Packet p : protected_packets) { @@ -111,7 +111,7 @@ TEST_F(ProducerFecTest, OneFrameFec) { // of packets is within |kMaxExcessOverhead|, and (2) the total number of // media packets for 1 frame is at least |minimum_media_packets_fec_|. const int kNumPackets = 4; - FecProtectionParams params = {15, false, 3}; + FecProtectionParams params = {15, 3, kFecMaskRandom}; std::list rtp_packets; generator_->NewFrame(kNumPackets); producer_->SetFecParameters(¶ms, 0); // Expecting one FEC packet. @@ -152,7 +152,7 @@ TEST_F(ProducerFecTest, TwoFrameFec) { const int kNumPackets = 2; const int kNumFrames = 2; - FecProtectionParams params = {15, 0, 3}; + FecProtectionParams params = {15, 3, kFecMaskRandom}; std::list rtp_packets; producer_->SetFecParameters(¶ms, 0); // Expecting one FEC packet. uint32_t last_timestamp = 0; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index a7d5657539..f9dc8f1bb2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -1293,7 +1293,6 @@ TEST_F(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { fec_params.fec_mask_type = kFecMaskRandom; fec_params.fec_rate = 1; fec_params.max_fec_frames = 1; - fec_params.use_uep_protection = false; rtp_sender_->SetFecParameters(&fec_params, &fec_params); ASSERT_EQ(0, rtp_sender_->SendOutgoingData(kVideoFrameDelta, payload_type, 1234, 4321, payload, diff --git a/webrtc/modules/video_coding/media_opt_util.cc b/webrtc/modules/video_coding/media_opt_util.cc index 69cf757f2b..42db2facf1 100644 --- a/webrtc/modules/video_coding/media_opt_util.cc +++ b/webrtc/modules/video_coding/media_opt_util.cc @@ -34,19 +34,10 @@ VCMProtectionMethod::VCMProtectionMethod() _protectionFactorD(0), _scaleProtKey(2.0f), _maxPayloadSize(1460), - _qmRobustness(new VCMQmRobustness()), - _useUepProtectionK(false), - _useUepProtectionD(true), _corrFecCost(1.0), _type(kNone) {} -VCMProtectionMethod::~VCMProtectionMethod() { - delete _qmRobustness; -} -void VCMProtectionMethod::UpdateContentMetrics( - const VideoContentMetrics* contentMetrics) { - _qmRobustness->UpdateContent(contentMetrics); -} +VCMProtectionMethod::~VCMProtectionMethod() {} VCMNackFecMethod::VCMNackFecMethod(int64_t lowRttNackThresholdMs, int64_t highRttNackThresholdMs) @@ -333,17 +324,6 @@ bool VCMFecMethod::ProtectionFactor(const VCMProtectionParameters* parameters) { codeRateDelta = kPacketLossMax - 1; } - float adjustFec = 1.0f; - // Avoid additional adjustments when layers are active. - // TODO(mikhal/marco): Update adjusmtent based on layer info. - if (parameters->numLayers == 1) { - adjustFec = _qmRobustness->AdjustFecFactor( - codeRateDelta, parameters->bitRate, parameters->frameRate, - parameters->rtt, packetLoss); - } - - codeRateDelta = static_cast(codeRateDelta * adjustFec); - // For Key frame: // Effectively at a higher rate, so we scale/boost the rate // The boost factor may depend on several factors: ratio of packet @@ -411,13 +391,6 @@ bool VCMFecMethod::ProtectionFactor(const VCMProtectionParameters* parameters) { _corrFecCost = 0.0f; } - // TODO(marpan): Set the UEP protection on/off for Key and Delta frames - _useUepProtectionK = _qmRobustness->SetUepProtection( - codeRateKey, parameters->bitRate, packetLoss, 0); - - _useUepProtectionD = _qmRobustness->SetUepProtection( - codeRateDelta, parameters->bitRate, packetLoss, 1); - // DONE WITH FEC PROTECTION SETTINGS return true; } diff --git a/webrtc/modules/video_coding/media_opt_util.h b/webrtc/modules/video_coding/media_opt_util.h index 6b47e3b2d9..1501f72ef6 100644 --- a/webrtc/modules/video_coding/media_opt_util.h +++ b/webrtc/modules/video_coding/media_opt_util.h @@ -138,9 +138,6 @@ class VCMProtectionMethod { virtual int MaxFramesFec() const { return 1; } - // Updates content metrics - void UpdateContentMetrics(const VideoContentMetrics* contentMetrics); - protected: uint8_t _effectivePacketLoss; uint8_t _protectionFactorK; @@ -149,7 +146,6 @@ class VCMProtectionMethod { float _scaleProtKey; int32_t _maxPayloadSize; - VCMQmRobustness* _qmRobustness; bool _useUepProtectionK; bool _useUepProtectionD; float _corrFecCost; diff --git a/webrtc/modules/video_coding/media_optimization.cc b/webrtc/modules/video_coding/media_optimization.cc index a234a06f9b..f24637e0b2 100644 --- a/webrtc/modules/video_coding/media_optimization.cc +++ b/webrtc/modules/video_coding/media_optimization.cc @@ -33,13 +33,6 @@ void UpdateProtectionCallback( // Get the FEC code rate for Delta frames (set to 0 when NA). delta_fec_params.fec_rate = selected_method->RequiredProtectionFactorD(); - // Get the FEC-UEP protection status for Key frames: UEP on/off. - key_fec_params.use_uep_protection = selected_method->RequiredUepProtectionK(); - - // Get the FEC-UEP protection status for Delta frames: UEP on/off. - delta_fec_params.use_uep_protection = - selected_method->RequiredUepProtectionD(); - // The RTP module currently requires the same |max_fec_frames| for both // key and delta frames. delta_fec_params.max_fec_frames = selected_method->MaxFramesFec(); @@ -229,9 +222,6 @@ uint32_t MediaOptimization::SetTargetRates( // Update protection settings, when applicable. float sent_video_rate_kbps = 0.0f; if (loss_prot_logic_->SelectedType() != kNone) { - // Update protection method with content metrics. - selected_method->UpdateContentMetrics(content_->ShortTermAvgData()); - // Update method will compute the robustness settings for the given // protection method and the overhead cost // the protection method is set by the user via SetVideoProtection. diff --git a/webrtc/modules/video_coding/qm_select.cc b/webrtc/modules/video_coding/qm_select.cc index 9da42bb33c..a090ba1e33 100644 --- a/webrtc/modules/video_coding/qm_select.cc +++ b/webrtc/modules/video_coding/qm_select.cc @@ -898,56 +898,4 @@ void VCMQmResolution::SelectSpatialDirectionMode(float transition_rate) { } } -// ROBUSTNESS CLASS - -VCMQmRobustness::VCMQmRobustness() { - Reset(); -} - -VCMQmRobustness::~VCMQmRobustness() {} - -void VCMQmRobustness::Reset() { - prev_total_rate_ = 0.0f; - prev_rtt_time_ = 0; - prev_packet_loss_ = 0; - prev_code_rate_delta_ = 0; - ResetQM(); -} - -// Adjust the FEC rate based on the content and the network state -// (packet loss rate, total rate/bandwidth, round trip time). -// Note that packetLoss here is the filtered loss value. -float VCMQmRobustness::AdjustFecFactor(uint8_t code_rate_delta, - float total_rate, - float framerate, - int64_t rtt_time, - uint8_t packet_loss) { - // Default: no adjustment - float adjust_fec = 1.0f; - if (content_metrics_ == NULL) { - return adjust_fec; - } - // Compute class state of the content. - ComputeMotionNFD(); - ComputeSpatial(); - - // TODO(marpan): Set FEC adjustment factor. - - // Keep track of previous values of network state: - // adjustment may be also based on pattern of changes in network state. - prev_total_rate_ = total_rate; - prev_rtt_time_ = rtt_time; - prev_packet_loss_ = packet_loss; - prev_code_rate_delta_ = code_rate_delta; - return adjust_fec; -} - -// Set the UEP (unequal-protection across packets) on/off for the FEC. -bool VCMQmRobustness::SetUepProtection(uint8_t code_rate_delta, - float total_rate, - uint8_t packet_loss, - bool frame_type) { - // Default. - return false; -} } // namespace webrtc diff --git a/webrtc/modules/video_coding/qm_select.h b/webrtc/modules/video_coding/qm_select.h index 764b5ed8e3..ae0463f499 100644 --- a/webrtc/modules/video_coding/qm_select.h +++ b/webrtc/modules/video_coding/qm_select.h @@ -322,35 +322,5 @@ class VCMQmResolution : public VCMQmMethod { int num_layers_; }; -// Robustness settings class. - -class VCMQmRobustness : public VCMQmMethod { - public: - VCMQmRobustness(); - ~VCMQmRobustness(); - - virtual void Reset(); - - // Adjust FEC rate based on content: every ~1 sec from SetTargetRates. - // Returns an adjustment factor. - float AdjustFecFactor(uint8_t code_rate_delta, - float total_rate, - float framerate, - int64_t rtt_time, - uint8_t packet_loss); - - // Set the UEP protection on/off. - bool SetUepProtection(uint8_t code_rate_delta, - float total_rate, - uint8_t packet_loss, - bool frame_type); - - private: - // Previous state of network parameters. - float prev_total_rate_; - int64_t prev_rtt_time_; - uint8_t prev_packet_loss_; - uint8_t prev_code_rate_delta_; -}; } // namespace webrtc #endif // WEBRTC_MODULES_VIDEO_CODING_QM_SELECT_H_