diff --git a/src/modules/video_coding/main/source/jitter_buffer.cc b/src/modules/video_coding/main/source/jitter_buffer.cc index 191857342..ddd098a5b 100644 --- a/src/modules/video_coding/main/source/jitter_buffer.cc +++ b/src/modules/video_coding/main/source/jitter_buffer.cc @@ -16,7 +16,6 @@ #include "jitter_buffer.h" #include "jitter_buffer_common.h" #include "jitter_estimator.h" -#include "media_optimization.h" // hybrid NACK/FEC thresholds. #include "packet.h" #include "event.h" @@ -86,6 +85,8 @@ VCMJitterBuffer::VCMJitterBuffer(WebRtc_Word32 vcmId, WebRtc_Word32 receiverId, _jitterEstimate(vcmId, receiverId), _rttMs(0), _nackMode(kNoNack), + _lowRttNackThresholdMs(-1), + _highRttNackThresholdMs(-1), _NACKSeqNum(), _NACKSeqNumLength(0), _waitingForKeyFrame(false), @@ -870,8 +871,10 @@ VCMJitterBuffer::GetEstimatedJitterMsInternal() WebRtc_UWord32 estimate = VCMJitterEstimator::OPERATING_SYSTEM_JITTER; // Compute RTT multiplier for estimation + // _lowRttNackThresholdMs == -1 means no FEC. double rttMult = 1.0f; - if (_nackMode == kNackHybrid && _rttMs > kLowRttNackMs) + if (_nackMode == kNackHybrid && (_lowRttNackThresholdMs >= 0 && + static_cast(_rttMs) > _lowRttNackThresholdMs)) { // from here we count on FEC rttMult = 0.0f; @@ -1775,10 +1778,18 @@ VCMJitterBuffer::GetNackMode() const // Set NACK mode void -VCMJitterBuffer::SetNackMode(VCMNackMode mode) +VCMJitterBuffer::SetNackMode(VCMNackMode mode, + int lowRttNackThresholdMs, + int highRttNackThresholdMs) { CriticalSectionScoped cs(_critSect); _nackMode = mode; + assert(lowRttNackThresholdMs >= -1 && highRttNackThresholdMs >= -1); + assert(highRttNackThresholdMs == -1 || + lowRttNackThresholdMs <= highRttNackThresholdMs); + assert(lowRttNackThresholdMs > -1 || highRttNackThresholdMs == -1); + _lowRttNackThresholdMs = lowRttNackThresholdMs; + _highRttNackThresholdMs = highRttNackThresholdMs; if (_nackMode == kNoNack) { _jitterEstimate.ResetNackCount(); @@ -2037,7 +2048,8 @@ VCMJitterBuffer::WaitForNack() } // else: hybrid mode, evaluate // RTT high, don't wait - if (_rttMs >= kHighRttNackMs) + if (_highRttNackThresholdMs >= 0 && + _rttMs >= static_cast(_highRttNackThresholdMs)) { return false; } @@ -2045,5 +2057,4 @@ VCMJitterBuffer::WaitForNack() return true; } - -} +} // namespace webrtc diff --git a/src/modules/video_coding/main/source/jitter_buffer.h b/src/modules/video_coding/main/source/jitter_buffer.h index 467552866..3a126c500 100644 --- a/src/modules/video_coding/main/source/jitter_buffer.h +++ b/src/modules/video_coding/main/source/jitter_buffer.h @@ -121,7 +121,15 @@ public: void UpdateRtt(WebRtc_UWord32 rttMs); // NACK - void SetNackMode(VCMNackMode mode); // Enable/disable nack + // Set the NACK mode. "highRttNackThreshold" is an RTT threshold in ms above + // which NACK will be disabled if the NACK mode is "kNackHybrid", + // -1 meaning that NACK is always enabled in the Hybrid mode. + // "lowRttNackThreshold" is an RTT threshold in ms below which we expect to + // rely on NACK only, and therefore are using larger buffers to have time to + // wait for retransmissions. + void SetNackMode(VCMNackMode mode, + int lowRttNackThresholdMs, + int highRttNackThresholdMs); VCMNackMode GetNackMode() const; // Get nack mode // Get list of missing sequence numbers (size in number of elements) WebRtc_UWord16* GetNackList(WebRtc_UWord16& nackSize, @@ -236,6 +244,8 @@ private: // NACK VCMNackMode _nackMode; + int _lowRttNackThresholdMs; + int _highRttNackThresholdMs; // Holds the internal nack list (the missing sequence numbers) WebRtc_Word32 _NACKSeqNumInternal[kNackHistoryLength]; WebRtc_UWord16 _NACKSeqNum[kNackHistoryLength]; diff --git a/src/modules/video_coding/main/source/media_opt_util.cc b/src/modules/video_coding/main/source/media_opt_util.cc index 80b12129d..22d2f9c6e 100644 --- a/src/modules/video_coding/main/source/media_opt_util.cc +++ b/src/modules/video_coding/main/source/media_opt_util.cc @@ -49,10 +49,16 @@ VCMProtectionMethod::UpdateContentMetrics(const _qmRobustness->UpdateContent(contentMetrics); } -VCMNackFecMethod::VCMNackFecMethod(): -VCMFecMethod() -{ - _type = kNackFec; +VCMNackFecMethod::VCMNackFecMethod(int lowRttNackThresholdMs, + int highRttNackThresholdMs) + : VCMFecMethod(), + _lowRttNackMs(lowRttNackThresholdMs), + _highRttNackMs(highRttNackThresholdMs) { + assert(lowRttNackThresholdMs >= -1 && highRttNackThresholdMs >= -1); + assert(highRttNackThresholdMs == -1 || + lowRttNackThresholdMs <= highRttNackThresholdMs); + assert(lowRttNackThresholdMs > -1 || highRttNackThresholdMs == -1); + _type = kNackFec; } VCMNackFecMethod::~VCMNackFecMethod() @@ -60,13 +66,13 @@ VCMNackFecMethod::~VCMNackFecMethod() // } bool -VCMNackFecMethod::ProtectionFactor(const - VCMProtectionParameters* parameters) +VCMNackFecMethod::ProtectionFactor(const VCMProtectionParameters* parameters) { // Hybrid Nack FEC has three operational modes: // 1. Low RTT (below kLowRttNackMs) - Nack only: Set FEC rate - // (_protectionFactorD) to zero. - // 2. High RTT (above kHighRttNackMs) - FEC Only: Keep FEC factors. + // (_protectionFactorD) to zero. -1 means no FEC. + // 2. High RTT (above _highRttNackMs) - FEC Only: Keep FEC factors. + // -1 means always allow NACK. // 3. Medium RTT values - Hybrid mode: We will only nack the // residual following the decoding of the FEC (refer to JB logic). FEC // delta protection factor will be adjusted based on the RTT. @@ -76,7 +82,7 @@ VCMNackFecMethod::ProtectionFactor(const // Compute the protection factors VCMFecMethod::ProtectionFactor(parameters); - if (parameters->rtt < kLowRttNackMs) + if (_lowRttNackMs == -1 || parameters->rtt < _lowRttNackMs) { _protectionFactorD = 0; VCMFecMethod::UpdateProtectionFactorD(_protectionFactorD); @@ -84,7 +90,7 @@ VCMNackFecMethod::ProtectionFactor(const // When in Hybrid mode (RTT range), adjust FEC rates based on the // RTT (NACK effectiveness) - adjustment factor is in the range [0,1]. - else if (parameters->rtt < kHighRttNackMs) + else if (_highRttNackMs == -1 || parameters->rtt < _highRttNackMs) { // TODO(mikhal): Disabling adjustment temporarily. // WebRtc_UWord16 rttIndex = (WebRtc_UWord16) parameters->rtt; @@ -103,8 +109,7 @@ VCMNackFecMethod::ProtectionFactor(const } bool -VCMNackFecMethod::EffectivePacketLoss(const - VCMProtectionParameters* parameters) +VCMNackFecMethod::EffectivePacketLoss(const VCMProtectionParameters* parameters) { // Set the effective packet loss for encoder (based on FEC code). // Compute the effective packet loss and residual packet loss due to FEC. @@ -125,7 +130,7 @@ VCMNackFecMethod::UpdateParameters(const VCMProtectionParameters* parameters) _efficiency = parameters->bitRate * fecRate * _corrFecCost; // Add NACK cost, when applicable - if (parameters->rtt < kHighRttNackMs) + if (_highRttNackMs == -1 || parameters->rtt < _highRttNackMs) { // nackCost = (bitRate - nackCost) * (lossPr) _efficiency += parameters->bitRate * _residualPacketLossFec / @@ -603,7 +608,8 @@ VCMLossProtectionLogic::SetMethod(enum VCMProtectionMethodEnum newMethodType) } case kNackFec: { - newMethod = new VCMNackFecMethod(); + // Default to always having NACK enabled for the hybrid mode. + newMethod = new VCMNackFecMethod(kLowRttNackMs, -1); break; } default: diff --git a/src/modules/video_coding/main/source/media_opt_util.h b/src/modules/video_coding/main/source/media_opt_util.h index b235fc727..c6566fb6a 100644 --- a/src/modules/video_coding/main/source/media_opt_util.h +++ b/src/modules/video_coding/main/source/media_opt_util.h @@ -49,7 +49,7 @@ struct VCMProtectionParameters residualPacketLossFec(0.0f), codecWidth(0), codecHeight(0) {} - WebRtc_UWord32 rtt; + int rtt; float lossPr; float bitRate; float packetsPerFrame; @@ -195,13 +195,18 @@ public: class VCMNackFecMethod : public VCMFecMethod { public: - VCMNackFecMethod(); + VCMNackFecMethod(int lowRttNackThresholdMs, + int highRttNackThresholdMs); virtual ~VCMNackFecMethod(); virtual bool UpdateParameters(const VCMProtectionParameters* parameters); // Get the effective packet loss for ER bool EffectivePacketLoss(const VCMProtectionParameters* parameters); // Get the protection factors bool ProtectionFactor(const VCMProtectionParameters* parameters); + +private: + int _lowRttNackMs; + int _highRttNackMs; }; class VCMLossProtectionLogic diff --git a/src/modules/video_coding/main/source/receiver.cc b/src/modules/video_coding/main/source/receiver.cc index 92fdb13f5..aa3662437 100644 --- a/src/modules/video_coding/main/source/receiver.cc +++ b/src/modules/video_coding/main/source/receiver.cc @@ -8,12 +8,14 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "video_coding.h" -#include "trace.h" +#include "receiver.h" + #include "encoded_frame.h" #include "internal_defines.h" -#include "receiver.h" +#include "media_opt_util.h" #include "tick_time.h" +#include "trace.h" +#include "video_coding.h" #include @@ -373,7 +375,8 @@ void VCMReceiver::SetNackMode(VCMNackMode nackMode) { CriticalSectionScoped cs(_critSect); - _jitterBuffer.SetNackMode(nackMode); + // Default to always having NACK enabled in hybrid mode. + _jitterBuffer.SetNackMode(nackMode, kLowRttNackMs, -1); if (!_master) { _state = kPassive; // The dual decoder defaults to passive diff --git a/src/modules/video_coding/main/test/jitter_buffer_test.cc b/src/modules/video_coding/main/test/jitter_buffer_test.cc index d651c4b0a..a25bf9c9e 100644 --- a/src/modules/video_coding/main/test/jitter_buffer_test.cc +++ b/src/modules/video_coding/main/test/jitter_buffer_test.cc @@ -8,20 +8,22 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "common_types.h" -#include "jitter_buffer.h" -#include "jitter_estimator.h" -#include "inter_frame_delay.h" -#include "packet.h" -#include "tick_time.h" -#include "../source/event.h" -#include "frame_buffer.h" -#include "jitter_estimate_test.h" -#include "test_util.h" -#include "test_macros.h" #include #include +#include "common_types.h" +#include "../source/event.h" +#include "frame_buffer.h" +#include "inter_frame_delay.h" +#include "jitter_buffer.h" +#include "jitter_estimate_test.h" +#include "jitter_estimator.h" +#include "media_opt_util.h" +#include "packet.h" +#include "test_util.h" +#include "test_macros.h" +#include "tick_time.h" + using namespace webrtc; int CheckOutFrame(VCMEncodedFrame* frameOut, unsigned int size, bool startCode) @@ -1568,7 +1570,7 @@ int JitterBufferTest(CmdArgs& args) // --------------------------------------------------------------------------------------------- // | 3 | 4 | 5 | 6 | 7 | 9 | x | 11 | 12 | 13 | 14 | 15 | 16 | 17 | 18 | 19 | x | 21 |.....| 102 | // --------------------------------------------------------------------------------------------- - jb.SetNackMode(kNackInfinite); + jb.SetNackMode(kNackInfinite, -1, -1); TEST(jb.GetNackMode() == kNackInfinite); @@ -2039,7 +2041,7 @@ int JitterBufferTest(CmdArgs& args) // testing that empty packets do not clog the jitter buffer // Set hybrid mode - jb.SetNackMode(kNackHybrid); + jb.SetNackMode(kNackHybrid, kLowRttNackMs, -1); TEST(jb.GetNackMode() == kNackHybrid); int maxSize = 100; @@ -2067,7 +2069,7 @@ int JitterBufferTest(CmdArgs& args) testFrame = jb.GetFrame(packet); TEST(frameIn != 0); - jb.SetNackMode(kNoNack); + jb.SetNackMode(kNoNack, -1, -1); jb.Flush(); // Testing that 1 empty packet inserted last will not be set for decoding @@ -2103,7 +2105,7 @@ int JitterBufferTest(CmdArgs& args) // Test incomplete NALU frames jb.Flush(); - jb.SetNackMode(kNoNack); + jb.SetNackMode(kNoNack, -1, -1); seqNum ++; timeStamp += 33*90; int insertedLength=0;