diff --git a/webrtc/call/bitrate_estimator_tests.cc b/webrtc/call/bitrate_estimator_tests.cc index 3f8ea128cf..a7e04aa695 100644 --- a/webrtc/call/bitrate_estimator_tests.cc +++ b/webrtc/call/bitrate_estimator_tests.cc @@ -270,7 +270,6 @@ TEST_F(BitrateEstimatorTest, InstantiatesTOFPerDefaultForVideo) { video_send_config_.rtp.extensions.push_back( RtpExtension(RtpExtension::kTimestampOffsetUri, kTOFExtensionId)); receiver_log_.PushExpectedLogLine(kSingleStreamLog); - receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); receiver_log_.PushExpectedLogLine(kSingleStreamLog); streams_.push_back(new Stream(this, false)); EXPECT_TRUE(receiver_log_.Wait()); @@ -280,10 +279,9 @@ TEST_F(BitrateEstimatorTest, ImmediatelySwitchToASTForVideo) { video_send_config_.rtp.extensions.push_back( RtpExtension(RtpExtension::kAbsSendTimeUri, kASTExtensionId)); receiver_log_.PushExpectedLogLine(kSingleStreamLog); - receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); receiver_log_.PushExpectedLogLine(kSingleStreamLog); - receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); receiver_log_.PushExpectedLogLine("Switching to absolute send time RBE."); + receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); streams_.push_back(new Stream(this, false)); EXPECT_TRUE(receiver_log_.Wait()); } @@ -292,15 +290,14 @@ TEST_F(BitrateEstimatorTest, SwitchesToASTForVideo) { video_send_config_.rtp.extensions.push_back( RtpExtension(RtpExtension::kTimestampOffsetUri, kTOFExtensionId)); receiver_log_.PushExpectedLogLine(kSingleStreamLog); - receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); receiver_log_.PushExpectedLogLine(kSingleStreamLog); streams_.push_back(new Stream(this, false)); EXPECT_TRUE(receiver_log_.Wait()); video_send_config_.rtp.extensions[0] = RtpExtension(RtpExtension::kAbsSendTimeUri, kASTExtensionId); - receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); receiver_log_.PushExpectedLogLine("Switching to absolute send time RBE."); + receiver_log_.PushExpectedLogLine(kAbsSendTimeLog); streams_.push_back(new Stream(this, false)); EXPECT_TRUE(receiver_log_.Wait()); } diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index bb13aba37a..a7d2788ff8 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -25,50 +25,21 @@ #include "webrtc/typedefs.h" namespace { -enum { - kTimestampGroupLengthMs = 5, - kAbsSendTimeFraction = 18, - kAbsSendTimeInterArrivalUpshift = 8, - kInterArrivalShift = kAbsSendTimeFraction + kAbsSendTimeInterArrivalUpshift, - kInitialProbingIntervalMs = 2000, - kMinClusterSize = 4, - kMaxProbePackets = 15, - kExpectedNumberOfProbes = 3 -}; - -static const double kTimestampToMs = +constexpr int kTimestampGroupLengthMs = 5; +constexpr int kAbsSendTimeFraction = 18; +constexpr int kAbsSendTimeInterArrivalUpshift = 8; +constexpr int kInterArrivalShift = + kAbsSendTimeFraction + kAbsSendTimeInterArrivalUpshift; +constexpr double kTimestampToMs = 1000.0 / static_cast(1 << kInterArrivalShift); -template -std::vector Keys(const std::map& map) { - std::vector keys; - keys.reserve(map.size()); - for (typename std::map::const_iterator it = map.begin(); - it != map.end(); ++it) { - keys.push_back(it->first); - } - return keys; -} - -uint32_t ConvertMsTo24Bits(int64_t time_ms) { - uint32_t time_24_bits = - static_cast( - ((static_cast(time_ms) << kAbsSendTimeFraction) + 500) / - 1000) & - 0x00FFFFFF; - return time_24_bits; -} +// This ssrc is used to fulfill the current API but will be removed +// after the API has been changed. +constexpr uint32_t kFixedSsrc = 0; } // namespace namespace webrtc { -void DelayBasedBwe::AddCluster(std::list* clusters, Cluster* cluster) { - cluster->send_mean_ms /= static_cast(cluster->count); - cluster->recv_mean_ms /= static_cast(cluster->count); - cluster->mean_size /= cluster->count; - clusters->push_back(*cluster); -} - DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock) : clock_(clock), observer_(observer), @@ -76,124 +47,14 @@ DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock) estimator_(), detector_(OverUseDetectorOptions()), incoming_bitrate_(kBitrateWindowMs, 8000), - total_probes_received_(0), first_packet_time_ms_(-1), last_update_ms_(-1), + last_seen_packet_ms_(-1), uma_recorded_(false) { RTC_DCHECK(observer_); - // NOTE! The BitrateEstimatorTest relies on this EXACT log line. - LOG(LS_INFO) << "RemoteBitrateEstimatorAbsSendTime: Instantiating."; network_thread_.DetachFromThread(); } -void DelayBasedBwe::ComputeClusters(std::list* clusters) const { - Cluster current; - int64_t prev_send_time = -1; - int64_t prev_recv_time = -1; - int last_probe_cluster_id = -1; - for (std::list::const_iterator it = probes_.begin(); - it != probes_.end(); ++it) { - if (last_probe_cluster_id == -1) - last_probe_cluster_id = it->cluster_id; - if (prev_send_time >= 0) { - int send_delta_ms = it->send_time_ms - prev_send_time; - int recv_delta_ms = it->recv_time_ms - prev_recv_time; - if (send_delta_ms >= 1 && recv_delta_ms >= 1) { - ++current.num_above_min_delta; - } - if (it->cluster_id != last_probe_cluster_id) { - if (current.count >= kMinClusterSize) - AddCluster(clusters, ¤t); - current = Cluster(); - } - current.send_mean_ms += send_delta_ms; - current.recv_mean_ms += recv_delta_ms; - current.mean_size += it->payload_size; - ++current.count; - last_probe_cluster_id = it->cluster_id; - } - prev_send_time = it->send_time_ms; - prev_recv_time = it->recv_time_ms; - } - if (current.count >= kMinClusterSize) - AddCluster(clusters, ¤t); -} - -std::list::const_iterator DelayBasedBwe::FindBestProbe( - const std::list& clusters) const { - int highest_probe_bitrate_bps = 0; - std::list::const_iterator best_it = clusters.end(); - for (std::list::const_iterator it = clusters.begin(); - it != clusters.end(); ++it) { - if (it->send_mean_ms == 0 || it->recv_mean_ms == 0) - continue; - int send_bitrate_bps = it->mean_size * 8 * 1000 / it->send_mean_ms; - int recv_bitrate_bps = it->mean_size * 8 * 1000 / it->recv_mean_ms; - if (it->num_above_min_delta > it->count / 2 && - (it->recv_mean_ms - it->send_mean_ms <= 2.0f && - it->send_mean_ms - it->recv_mean_ms <= 5.0f)) { - int probe_bitrate_bps = - std::min(it->GetSendBitrateBps(), it->GetRecvBitrateBps()); - if (probe_bitrate_bps > highest_probe_bitrate_bps) { - highest_probe_bitrate_bps = probe_bitrate_bps; - best_it = it; - } - } else { - LOG(LS_INFO) << "Probe failed, sent at " << send_bitrate_bps - << " bps, received at " << recv_bitrate_bps - << " bps. Mean send delta: " << it->send_mean_ms - << " ms, mean recv delta: " << it->recv_mean_ms - << " ms, num probes: " << it->count; - break; - } - } - return best_it; -} - -DelayBasedBwe::ProbeResult DelayBasedBwe::ProcessClusters(int64_t now_ms) { - std::list clusters; - ComputeClusters(&clusters); - if (clusters.empty()) { - // If we reach the max number of probe packets and still have no clusters, - // we will remove the oldest one. - if (probes_.size() >= kMaxProbePackets) - probes_.pop_front(); - return ProbeResult::kNoUpdate; - } - - std::list::const_iterator best_it = FindBestProbe(clusters); - if (best_it != clusters.end()) { - int probe_bitrate_bps = - std::min(best_it->GetSendBitrateBps(), best_it->GetRecvBitrateBps()); - // Make sure that a probe sent on a lower bitrate than our estimate can't - // reduce the estimate. - if (IsBitrateImproving(probe_bitrate_bps)) { - LOG(LS_INFO) << "Probe successful, sent at " - << best_it->GetSendBitrateBps() << " bps, received at " - << best_it->GetRecvBitrateBps() - << " bps. Mean send delta: " << best_it->send_mean_ms - << " ms, mean recv delta: " << best_it->recv_mean_ms - << " ms, num probes: " << best_it->count; - remote_rate_.SetEstimate(probe_bitrate_bps, now_ms); - return ProbeResult::kBitrateUpdated; - } - } - - // Not probing and received non-probe packet, or finished with current set - // of probes. - if (clusters.size() >= kExpectedNumberOfProbes) - probes_.clear(); - return ProbeResult::kNoUpdate; -} - -bool DelayBasedBwe::IsBitrateImproving(int new_bitrate_bps) const { - bool initial_probe = !remote_rate_.ValidEstimate() && new_bitrate_bps > 0; - bool bitrate_above_estimate = - remote_rate_.ValidEstimate() && - new_bitrate_bps > static_cast(remote_rate_.LatestEstimate()); - return initial_probe || bitrate_above_estimate; -} - void DelayBasedBwe::IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector) { RTC_DCHECK(network_thread_.CalledOnValidThread()); @@ -204,82 +65,61 @@ void DelayBasedBwe::IncomingPacketFeedbackVector( uma_recorded_ = true; } for (const auto& packet_info : packet_feedback_vector) { - IncomingPacketInfo(packet_info.arrival_time_ms, - ConvertMsTo24Bits(packet_info.send_time_ms), - packet_info.payload_size, 0, - packet_info.probe_cluster_id); + IncomingPacketInfo(packet_info); } } -void DelayBasedBwe::IncomingPacketInfo(int64_t arrival_time_ms, - uint32_t send_time_24bits, - size_t payload_size, - uint32_t ssrc, - int probe_cluster_id) { - assert(send_time_24bits < (1ul << 24)); - // Shift up send time to use the full 32 bits that inter_arrival works with, - // so wrapping works properly. - uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift; - int64_t send_time_ms = static_cast(timestamp) * kTimestampToMs; - +void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) { int64_t now_ms = clock_->TimeInMilliseconds(); - // TODO(holmer): SSRCs are only needed for REMB, should be broken out from - // here. - incoming_bitrate_.Update(payload_size, arrival_time_ms); if (first_packet_time_ms_ == -1) first_packet_time_ms_ = now_ms; - uint32_t ts_delta = 0; - int64_t t_delta = 0; - int size_delta = 0; - + incoming_bitrate_.Update(info.payload_size, info.arrival_time_ms); bool update_estimate = false; uint32_t target_bitrate_bps = 0; - std::vector ssrcs; { rtc::CritScope lock(&crit_); - TimeoutStreams(now_ms); - RTC_DCHECK(inter_arrival_.get()); - RTC_DCHECK(estimator_.get()); - ssrcs_[ssrc] = now_ms; - - // For now only try to detect probes while we don't have a valid estimate, - // and make sure the packet was paced. We currently assume that only packets - // larger than 200 bytes are paced by the sender. - if (probe_cluster_id != PacketInfo::kNotAProbe && - payload_size > PacedSender::kMinProbePacketSize && - (!remote_rate_.ValidEstimate() || - now_ms - first_packet_time_ms_ < kInitialProbingIntervalMs)) { - // TODO(holmer): Use a map instead to get correct order? - if (total_probes_received_ < kMaxProbePackets) { - int send_delta_ms = -1; - int recv_delta_ms = -1; - if (!probes_.empty()) { - send_delta_ms = send_time_ms - probes_.back().send_time_ms; - recv_delta_ms = arrival_time_ms - probes_.back().recv_time_ms; - } - LOG(LS_INFO) << "Probe packet received: send time=" << send_time_ms - << " ms, recv time=" << arrival_time_ms - << " ms, send delta=" << send_delta_ms - << " ms, recv delta=" << recv_delta_ms << " ms."; - } - probes_.push_back( - Probe(send_time_ms, arrival_time_ms, payload_size, probe_cluster_id)); - ++total_probes_received_; - // Make sure that a probe which updated the bitrate immediately has an - // effect by calling the OnReceiveBitrateChanged callback. - if (ProcessClusters(now_ms) == ProbeResult::kBitrateUpdated) - update_estimate = true; + // Reset if the stream has timed out. + if (last_seen_packet_ms_ == -1 || + now_ms - last_seen_packet_ms_ > kStreamTimeOutMs) { + inter_arrival_.reset(new InterArrival( + (kTimestampGroupLengthMs << kInterArrivalShift) / 1000, + kTimestampToMs, true)); + estimator_.reset(new OveruseEstimator(OverUseDetectorOptions())); } - if (inter_arrival_->ComputeDeltas(timestamp, arrival_time_ms, now_ms, - payload_size, &ts_delta, &t_delta, + last_seen_packet_ms_ = now_ms; + + if (info.probe_cluster_id != PacketInfo::kNotAProbe) { + ProbingResult probe_result = + probe_bitrate_estimator_.PacketFeedback(info); + if (probe_result.valid()) { + remote_rate_.SetEstimate(probe_result.bps, probe_result.timestamp); + update_estimate = true; + } + } + + uint32_t send_time_24bits = + static_cast(((static_cast(info.send_time_ms) + << kAbsSendTimeFraction) + + 500) / + 1000) & + 0x00FFFFFF; + // Shift up send time to use the full 32 bits that inter_arrival works with, + // so wrapping works properly. + uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift; + + uint32_t ts_delta = 0; + int64_t t_delta = 0; + int size_delta = 0; + if (inter_arrival_->ComputeDeltas(timestamp, info.arrival_time_ms, now_ms, + info.payload_size, &ts_delta, &t_delta, &size_delta)) { double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift); estimator_->Update(t_delta, ts_delta_ms, size_delta, detector_.State()); detector_.Detect(estimator_->offset(), ts_delta_ms, - estimator_->num_of_deltas(), arrival_time_ms); + estimator_->num_of_deltas(), info.arrival_time_ms); } if (!update_estimate) { @@ -290,7 +130,7 @@ void DelayBasedBwe::IncomingPacketInfo(int64_t arrival_time_ms, update_estimate = true; } else if (detector_.State() == kBwOverusing) { rtc::Optional incoming_rate = - incoming_bitrate_.Rate(arrival_time_ms); + incoming_bitrate_.Rate(info.arrival_time_ms); if (incoming_rate && remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) { update_estimate = true; @@ -303,17 +143,17 @@ void DelayBasedBwe::IncomingPacketInfo(int64_t arrival_time_ms, // We also have to update the estimate immediately if we are overusing // and the target bitrate is too high compared to what we are receiving. const RateControlInput input(detector_.State(), - incoming_bitrate_.Rate(arrival_time_ms), + incoming_bitrate_.Rate(info.arrival_time_ms), estimator_->var_noise()); remote_rate_.Update(&input, now_ms); target_bitrate_bps = remote_rate_.UpdateBandwidthEstimate(now_ms); update_estimate = remote_rate_.ValidEstimate(); - ssrcs = Keys(ssrcs_); } } + if (update_estimate) { last_update_ms_ = now_ms; - observer_->OnReceiveBitrateChanged(ssrcs, target_bitrate_bps); + observer_->OnReceiveBitrateChanged({kFixedSsrc}, target_bitrate_bps); } } @@ -324,34 +164,12 @@ int64_t DelayBasedBwe::TimeUntilNextProcess() { return kDisabledModuleTime; } -void DelayBasedBwe::TimeoutStreams(int64_t now_ms) { - for (Ssrcs::iterator it = ssrcs_.begin(); it != ssrcs_.end();) { - if ((now_ms - it->second) > kStreamTimeOutMs) { - ssrcs_.erase(it++); - } else { - ++it; - } - } - if (ssrcs_.empty()) { - // We can't update the estimate if we don't have any active streams. - inter_arrival_.reset( - new InterArrival((kTimestampGroupLengthMs << kInterArrivalShift) / 1000, - kTimestampToMs, true)); - estimator_.reset(new OveruseEstimator(OverUseDetectorOptions())); - // We deliberately don't reset the first_packet_time_ms_ here for now since - // we only probe for bandwidth in the beginning of a call right now. - } -} - void DelayBasedBwe::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) { rtc::CritScope lock(&crit_); remote_rate_.SetRtt(avg_rtt_ms); } -void DelayBasedBwe::RemoveStream(uint32_t ssrc) { - rtc::CritScope lock(&crit_); - ssrcs_.erase(ssrc); -} +void DelayBasedBwe::RemoveStream(uint32_t ssrc) {} bool DelayBasedBwe::LatestEstimate(std::vector* ssrcs, uint32_t* bitrate_bps) const { @@ -362,15 +180,11 @@ bool DelayBasedBwe::LatestEstimate(std::vector* ssrcs, RTC_DCHECK(ssrcs); RTC_DCHECK(bitrate_bps); rtc::CritScope lock(&crit_); - if (!remote_rate_.ValidEstimate()) { + if (!remote_rate_.ValidEstimate()) return false; - } - *ssrcs = Keys(ssrcs_); - if (ssrcs_.empty()) { - *bitrate_bps = 0; - } else { - *bitrate_bps = remote_rate_.LatestEstimate(); - } + + *ssrcs = {kFixedSsrc}; + *bitrate_bps = remote_rate_.LatestEstimate(); return true; } diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index 4e34aa13c6..19410c658a 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -21,6 +21,7 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/rate_statistics.h" #include "webrtc/base/thread_checker.h" +#include "webrtc/modules/congestion_controller/probe_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/aimd_rate_control.h" #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h" #include "webrtc/modules/remote_bitrate_estimator/inter_arrival.h" @@ -37,90 +38,25 @@ class DelayBasedBwe : public RemoteBitrateEstimator { void IncomingPacketFeedbackVector( const std::vector& packet_feedback_vector) override; + void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; + bool LatestEstimate(std::vector* ssrcs, + uint32_t* bitrate_bps) const override; + void SetMinBitrate(int min_bitrate_bps) override; + // Required by RemoteBitrateEstimator but does nothing. + void Process() override; + // Required by RemoteBitrateEstimator but does nothing. + int64_t TimeUntilNextProcess() override; + // Required by RemoteBitrateEstimator but does nothing. + void RemoveStream(uint32_t ssrc) override; void IncomingPacket(int64_t arrival_time_ms, size_t payload_size, const RTPHeader& header) override { RTC_NOTREACHED(); } - // This class relies on Process() being called periodically (at least once - // every other second) for streams to be timed out properly. Therefore it - // shouldn't be detached from the ProcessThread except if it's about to be - // deleted. - void Process() override; - int64_t TimeUntilNextProcess() override; - void OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) override; - void RemoveStream(uint32_t ssrc) override; - bool LatestEstimate(std::vector* ssrcs, - uint32_t* bitrate_bps) const override; - void SetMinBitrate(int min_bitrate_bps) override; - private: - struct Probe { - Probe(int64_t send_time_ms, - int64_t recv_time_ms, - size_t payload_size, - int cluster_id) - : send_time_ms(send_time_ms), - recv_time_ms(recv_time_ms), - payload_size(payload_size), - cluster_id(cluster_id) {} - int64_t send_time_ms; - int64_t recv_time_ms; - size_t payload_size; - int cluster_id; - }; - - struct Cluster { - Cluster() - : send_mean_ms(0.0f), - recv_mean_ms(0.0f), - mean_size(0), - count(0), - num_above_min_delta(0) {} - - int GetSendBitrateBps() const { - RTC_CHECK_GT(send_mean_ms, 0.0f); - return mean_size * 8 * 1000 / send_mean_ms; - } - - int GetRecvBitrateBps() const { - RTC_CHECK_GT(recv_mean_ms, 0.0f); - return mean_size * 8 * 1000 / recv_mean_ms; - } - - float send_mean_ms; - float recv_mean_ms; - // TODO(holmer): Add some variance metric as well? - size_t mean_size; - int count; - int num_above_min_delta; - }; - - typedef std::map Ssrcs; - enum class ProbeResult { kBitrateUpdated, kNoUpdate }; - - static void AddCluster(std::list* clusters, Cluster* cluster); - - void IncomingPacketInfo(int64_t arrival_time_ms, - uint32_t send_time_24bits, - size_t payload_size, - uint32_t ssrc, - int probe_cluster_id); - - void ComputeClusters(std::list* clusters) const; - - std::list::const_iterator FindBestProbe( - const std::list& clusters) const; - - // Returns true if a probe which changed the estimate was detected. - ProbeResult ProcessClusters(int64_t now_ms) EXCLUSIVE_LOCKS_REQUIRED(&crit_); - - bool IsBitrateImproving(int probe_bitrate_bps) const - EXCLUSIVE_LOCKS_REQUIRED(&crit_); - - void TimeoutStreams(int64_t now_ms) EXCLUSIVE_LOCKS_REQUIRED(&crit_); + void IncomingPacketInfo(const PacketInfo& info); rtc::ThreadChecker network_thread_; Clock* const clock_; @@ -129,15 +65,14 @@ class DelayBasedBwe : public RemoteBitrateEstimator { std::unique_ptr estimator_; OveruseDetector detector_; RateStatistics incoming_bitrate_; - std::list probes_; - size_t total_probes_received_; int64_t first_packet_time_ms_; int64_t last_update_ms_; + int64_t last_seen_packet_ms_; bool uma_recorded_; rtc::CriticalSection crit_; - Ssrcs ssrcs_ GUARDED_BY(&crit_); AimdRateControl remote_rate_ GUARDED_BY(&crit_); + ProbeBitrateEstimator probe_bitrate_estimator_ GUARDED_BY(&crit_); RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(DelayBasedBwe); }; diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index 3228b6252d..6319e988b7 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -65,50 +65,6 @@ TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) { EXPECT_GT(bitrate_observer_->latest_bitrate(), 800000u); } -// Packets will require 5 ms to be transmitted to the receiver, causing packets -// of the second probe to be dispersed. -TEST_F(DelayBasedBweTest, ProbeDetectionTooHighBitrate) { - int64_t now_ms = clock_.TimeInMilliseconds(); - int64_t send_time_ms = 0; - uint16_t seq_num = 0; - // First burst sent at 8 * 1000 / 10 = 800 kbps. - for (int i = 0; i < kNumProbes; ++i) { - clock_.AdvanceTimeMilliseconds(10); - now_ms = clock_.TimeInMilliseconds(); - send_time_ms += 10; - IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, 0); - } - - // Second burst sent at 8 * 1000 / 5 = 1600 kbps, arriving at 8 * 1000 / 8 = - // 1000 kbps. - for (int i = 0; i < kNumProbes; ++i) { - clock_.AdvanceTimeMilliseconds(8); - now_ms = clock_.TimeInMilliseconds(); - send_time_ms += 5; - IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, 1); - } - - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 800000u, 10000u); -} - -TEST_F(DelayBasedBweTest, ProbeDetectionSlightlyFasterArrival) { - int64_t now_ms = clock_.TimeInMilliseconds(); - uint16_t seq_num = 0; - // First burst sent at 8 * 1000 / 10 = 800 kbps. - // Arriving at 8 * 1000 / 5 = 1600 kbps. - int64_t send_time_ms = 0; - for (int i = 0; i < kNumProbes; ++i) { - clock_.AdvanceTimeMilliseconds(5); - send_time_ms += 10; - now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, send_time_ms, seq_num++, 1000, 23); - } - - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_GT(bitrate_observer_->latest_bitrate(), 800000u); -} - TEST_F(DelayBasedBweTest, ProbeDetectionFasterArrival) { int64_t now_ms = clock_.TimeInMilliseconds(); uint16_t seq_num = 0; @@ -159,35 +115,6 @@ TEST_F(DelayBasedBweTest, ProbeDetectionSlowerArrivalHighBitrate) { EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 4000000u, 10000u); } -TEST_F(DelayBasedBweTest, ProbingIgnoresSmallPackets) { - int64_t now_ms = clock_.TimeInMilliseconds(); - uint16_t seq_num = 0; - // Probing with 200 bytes every 10 ms, should be ignored by the probe - // detection. - for (int i = 0; i < kNumProbes; ++i) { - clock_.AdvanceTimeMilliseconds(10); - now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, now_ms, seq_num++, - PacedSender::kMinProbePacketSize, 1); - } - - EXPECT_FALSE(bitrate_observer_->updated()); - - // Followed by a probe with 1000 bytes packets, should be detected as a - // probe. - for (int i = 0; i < kNumProbes; ++i) { - clock_.AdvanceTimeMilliseconds(10); - now_ms = clock_.TimeInMilliseconds(); - IncomingFeedback(now_ms, now_ms, seq_num++, 1000, 1); - } - - // Wait long enough so that we can call Process again. - clock_.AdvanceTimeMilliseconds(1000); - - EXPECT_TRUE(bitrate_observer_->updated()); - EXPECT_NEAR(bitrate_observer_->latest_bitrate(), 800000u, 10000u); -} - TEST_F(DelayBasedBweTest, InitialBehavior) { InitialBehaviorTestHelper(674840); } diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index c6907f7a0b..0ed7816e50 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -18,11 +18,13 @@ namespace webrtc { -const size_t kMtu = 1200; -const uint32_t kAcceptedBitrateErrorBps = 50000; +constexpr size_t kMtu = 1200; +constexpr uint32_t kAcceptedBitrateErrorBps = 50000; // Number of packets needed before we have a valid estimate. -const int kNumInitialPackets = 2; +constexpr int kNumInitialPackets = 2; + +constexpr int kInitialProbingPackets = 5; namespace test { @@ -168,7 +170,7 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, uint16_t sequence_number, size_t payload_size) { IncomingFeedback(arrival_time_ms, send_time_ms, sequence_number, payload_size, - 0); + PacketInfo::kNotAProbe); } void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, @@ -268,6 +270,10 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( clock_.AdvanceTimeMilliseconds(1000); // Inserting packets for 5 seconds to get a valid estimate. for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) { + // NOTE!!! If the following line is moved under the if case then this test + // wont work on windows realease bots. + int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe; + if (i == kNumInitialPackets) { bitrate_estimator_->Process(); EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); @@ -275,9 +281,8 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( EXPECT_FALSE(bitrate_observer_->updated()); bitrate_observer_->Reset(); } - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, - sequence_number++, kMtu); + sequence_number++, kMtu, cluster_id); clock_.AdvanceTimeMilliseconds(1000 / kFramerate); send_time_ms += kFrameIntervalMs; } @@ -289,10 +294,6 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( EXPECT_TRUE(bitrate_observer_->updated()); bitrate_observer_->Reset(); EXPECT_EQ(bitrate_observer_->latest_bitrate(), bitrate_bps); - bitrate_estimator_->RemoveStream(kDefaultSsrc); - EXPECT_TRUE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); - ASSERT_EQ(0u, ssrcs.size()); - EXPECT_EQ(0u, bitrate_bps); } void DelayBasedBweTest::RateIncreaseReorderingTestHelper( @@ -303,6 +304,10 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( uint16_t sequence_number = 0; // Inserting packets for five seconds to get a valid estimate. for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) { + // NOTE!!! If the following line is moved under the if case then this test + // wont work on windows realease bots. + int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe; + // TODO(sprang): Remove this hack once the single stream estimator is gone, // as it doesn't do anything in Process(). if (i == kNumInitialPackets) { @@ -310,9 +315,8 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( bitrate_estimator_->Process(); EXPECT_FALSE(bitrate_observer_->updated()); // No valid estimate. } - IncomingFeedback(clock_.TimeInMilliseconds(), send_time_ms, - sequence_number++, kMtu); + sequence_number++, kMtu, cluster_id); clock_.AdvanceTimeMilliseconds(kFrameIntervalMs); send_time_ms += kFrameIntervalMs; }