Revert of Fix BitrateProber to match the requested bitrate more precisely (patchset #4 id:60001 of https://codereview.webrtc.org/2613543003/ )
Reason for revert:
Speculative revert.
Linux memcheck bot started failing a lot at the time of this cl. Doesn't look related at first glance, but we don't have another lead yet.
Original issue's description:
> Fix BitrateProber to match the requested bitrate more precisely
>
> Previously BirateProber was calculating delay between probes based on
> the size of the previous probe. Because of that the actual sent bitrate
> can deviate greatly from the target value. With this change it uses
> total number of bytes in the cluster to estimate delay before each
> probe.
>
> BUG=webrtc:6952
>
> Review-Url: https://codereview.webrtc.org/2613543003
> Cr-Commit-Position: refs/heads/master@{#15971}
> Committed: 599c5011e7
TBR=philipel@webrtc.org,stefan@webrtc.org,sergeyu@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6952
Review-Url: https://codereview.webrtc.org/2626473004
Cr-Commit-Position: refs/heads/master@{#15979}
This commit is contained in:
parent
9774447b8f
commit
ebfbc8ebfd
@ -20,6 +20,9 @@ namespace webrtc {
|
||||
|
||||
namespace {
|
||||
|
||||
// Inactivity threshold above which probing is restarted.
|
||||
constexpr int kInactivityThresholdMs = 5000;
|
||||
|
||||
// A minimum interval between probes to allow scheduling to be feasible.
|
||||
constexpr int kMinProbeDeltaMs = 1;
|
||||
|
||||
@ -29,11 +32,18 @@ constexpr int kMinProbePacketsSent = 5;
|
||||
// The minimum probing duration in ms.
|
||||
constexpr int kMinProbeDurationMs = 15;
|
||||
|
||||
int ComputeDeltaFromBitrate(size_t probe_size, uint32_t bitrate_bps) {
|
||||
RTC_CHECK_GT(bitrate_bps, 0);
|
||||
// Compute the time delta needed to send probe_size bytes at bitrate_bps
|
||||
// bps. Result is in milliseconds.
|
||||
return static_cast<int>((1000ll * probe_size * 8) / bitrate_bps);
|
||||
}
|
||||
} // namespace
|
||||
|
||||
BitrateProber::BitrateProber()
|
||||
: probing_state_(ProbingState::kDisabled),
|
||||
next_probe_time_ms_(-1),
|
||||
probe_size_last_sent_(0),
|
||||
time_last_probe_sent_ms_(-1),
|
||||
next_cluster_id_(0) {
|
||||
SetEnabled(true);
|
||||
}
|
||||
@ -60,8 +70,6 @@ void BitrateProber::OnIncomingPacket(size_t packet_size) {
|
||||
if (probing_state_ == ProbingState::kInactive &&
|
||||
!clusters_.empty() &&
|
||||
packet_size >= PacedSender::kMinProbePacketSize) {
|
||||
// Send next probe right away.
|
||||
next_probe_time_ms_ = -1;
|
||||
probing_state_ = ProbingState::kActive;
|
||||
}
|
||||
}
|
||||
@ -85,6 +93,9 @@ void BitrateProber::CreateProbeCluster(int bitrate_bps) {
|
||||
}
|
||||
|
||||
void BitrateProber::ResetState() {
|
||||
time_last_probe_sent_ms_ = -1;
|
||||
probe_size_last_sent_ = 0;
|
||||
|
||||
// Recreate all probing clusters.
|
||||
std::queue<ProbeCluster> clusters;
|
||||
clusters.swap(clusters_);
|
||||
@ -101,19 +112,36 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
|
||||
// Probing is not active or probing is already complete.
|
||||
if (probing_state_ != ProbingState::kActive || clusters_.empty())
|
||||
return -1;
|
||||
|
||||
// time_last_probe_sent_ms_ of -1 indicates no probes have yet been sent.
|
||||
int64_t elapsed_time_ms;
|
||||
if (time_last_probe_sent_ms_ == -1) {
|
||||
elapsed_time_ms = 0;
|
||||
} else {
|
||||
elapsed_time_ms = now_ms - time_last_probe_sent_ms_;
|
||||
}
|
||||
// If no probes have been sent for a while, abort current probing and
|
||||
// reset.
|
||||
if (elapsed_time_ms > kInactivityThresholdMs) {
|
||||
ResetState();
|
||||
return -1;
|
||||
}
|
||||
// We will send the first probe packet immediately if no packet has been
|
||||
// sent before.
|
||||
int time_until_probe_ms = 0;
|
||||
if (next_probe_time_ms_ >= 0) {
|
||||
time_until_probe_ms = next_probe_time_ms_ - now_ms;
|
||||
// If we have waited more than 3 ms for a new packet we will restart probing
|
||||
// again later.
|
||||
if (probe_size_last_sent_ != 0 && probing_state_ == ProbingState::kActive) {
|
||||
int next_delta_ms = ComputeDeltaFromBitrate(probe_size_last_sent_,
|
||||
clusters_.front().bitrate_bps);
|
||||
time_until_probe_ms = next_delta_ms - elapsed_time_ms;
|
||||
// If we have waited more than 3 ms for a new packet to probe with we will
|
||||
// consider this probing session over.
|
||||
const int kMaxProbeDelayMs = 3;
|
||||
if (time_until_probe_ms < -kMaxProbeDelayMs) {
|
||||
ResetState();
|
||||
return -1;
|
||||
if (next_delta_ms < kMinProbeDeltaMs ||
|
||||
time_until_probe_ms < -kMaxProbeDelayMs) {
|
||||
probing_state_ = ProbingState::kSuspended;
|
||||
LOG(LS_INFO) << "Delta too small or missed probing accurately, suspend";
|
||||
time_until_probe_ms = 0;
|
||||
}
|
||||
}
|
||||
|
||||
return std::max(time_until_probe_ms, 0);
|
||||
}
|
||||
|
||||
@ -134,16 +162,12 @@ size_t BitrateProber::RecommendedMinProbeSize() const {
|
||||
void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) {
|
||||
RTC_DCHECK(probing_state_ == ProbingState::kActive);
|
||||
RTC_DCHECK_GT(bytes, 0);
|
||||
|
||||
probe_size_last_sent_ = bytes;
|
||||
time_last_probe_sent_ms_ = now_ms;
|
||||
if (!clusters_.empty()) {
|
||||
ProbeCluster* cluster = &clusters_.front();
|
||||
if (cluster->sent_probes == 0) {
|
||||
RTC_DCHECK_EQ(cluster->time_started_ms, -1);
|
||||
cluster->time_started_ms = now_ms;
|
||||
}
|
||||
cluster->sent_bytes += static_cast<int>(bytes);
|
||||
cluster->sent_probes += 1;
|
||||
next_probe_time_ms_ = GetNextProbeTime(clusters_.front());
|
||||
if (cluster->sent_bytes >= cluster->min_bytes &&
|
||||
cluster->sent_probes >= cluster->min_probes) {
|
||||
clusters_.pop();
|
||||
@ -152,17 +176,4 @@ void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) {
|
||||
probing_state_ = ProbingState::kSuspended;
|
||||
}
|
||||
}
|
||||
|
||||
int64_t BitrateProber::GetNextProbeTime(const ProbeCluster& cluster) {
|
||||
RTC_CHECK_GT(cluster.bitrate_bps, 0);
|
||||
RTC_CHECK_GE(cluster.time_started_ms, 0);
|
||||
|
||||
// Compute the time delta from the cluster start to ensure probe bitrate stays
|
||||
// close to the target bitrate. Result is in milliseconds.
|
||||
int64_t delta_ms = (8000ll * cluster.sent_bytes + cluster.bitrate_bps / 2) /
|
||||
cluster.bitrate_bps;
|
||||
return cluster.time_started_ms + delta_ms;
|
||||
}
|
||||
|
||||
|
||||
} // namespace webrtc
|
||||
|
||||
@ -72,36 +72,29 @@ class BitrateProber {
|
||||
};
|
||||
|
||||
// A probe cluster consists of a set of probes. Each probe in turn can be
|
||||
// divided into a number of packets to accommodate the MTU on the network.
|
||||
// divided into a number of packets to accomodate the MTU on the network.
|
||||
struct ProbeCluster {
|
||||
int min_probes = 0;
|
||||
int sent_probes = 0;
|
||||
int min_bytes = 0;
|
||||
int sent_bytes = 0;
|
||||
int bitrate_bps = 0;
|
||||
int id = -1;
|
||||
|
||||
int sent_probes = 0;
|
||||
int sent_bytes = 0;
|
||||
int64_t time_started_ms = -1;
|
||||
};
|
||||
|
||||
// Resets the state of the prober and clears any cluster/timing data tracked.
|
||||
void ResetState();
|
||||
|
||||
int64_t GetNextProbeTime(const ProbeCluster& cluster);
|
||||
|
||||
ProbingState probing_state_;
|
||||
|
||||
// Probe bitrate per packet. These are used to compute the delta relative to
|
||||
// the previous probe packet based on the size and time when that packet was
|
||||
// sent.
|
||||
std::queue<ProbeCluster> clusters_;
|
||||
|
||||
// Time the next probe should be sent when in kActive state.
|
||||
int64_t next_probe_time_ms_;
|
||||
|
||||
// A probe can include one or more packets.
|
||||
size_t probe_size_last_sent_;
|
||||
// The last time a probe was sent.
|
||||
int64_t time_last_probe_sent_ms_;
|
||||
int next_cluster_id_;
|
||||
};
|
||||
|
||||
} // namespace webrtc
|
||||
|
||||
#endif // WEBRTC_MODULES_PACING_BITRATE_PROBER_H_
|
||||
|
||||
@ -21,53 +21,35 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) {
|
||||
int64_t now_ms = 0;
|
||||
EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
|
||||
|
||||
const int kTestBitrate1 = 900000;
|
||||
const int kTestBitrate2 = 1800000;
|
||||
const int kClusterSize = 5;
|
||||
const int kProbeSize = 1000;
|
||||
const int kMinProbeDurationMs = 15;
|
||||
|
||||
prober.CreateProbeCluster(kTestBitrate1);
|
||||
prober.CreateProbeCluster(kTestBitrate2);
|
||||
prober.CreateProbeCluster(900000);
|
||||
prober.CreateProbeCluster(1800000);
|
||||
EXPECT_FALSE(prober.IsProbing());
|
||||
|
||||
prober.OnIncomingPacket(kProbeSize);
|
||||
prober.OnIncomingPacket(1000);
|
||||
EXPECT_TRUE(prober.IsProbing());
|
||||
EXPECT_EQ(0, prober.CurrentClusterId());
|
||||
|
||||
// First packet should probe as soon as possible.
|
||||
EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms));
|
||||
prober.ProbeSent(now_ms, 1000);
|
||||
|
||||
for (int i = 0; i < kClusterSize; ++i) {
|
||||
now_ms += prober.TimeUntilNextProbe(now_ms);
|
||||
for (int i = 0; i < 4; ++i) {
|
||||
EXPECT_EQ(8, prober.TimeUntilNextProbe(now_ms));
|
||||
now_ms += 4;
|
||||
EXPECT_EQ(4, prober.TimeUntilNextProbe(now_ms));
|
||||
now_ms += 4;
|
||||
EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms));
|
||||
EXPECT_EQ(0, prober.CurrentClusterId());
|
||||
prober.ProbeSent(now_ms, kProbeSize);
|
||||
prober.ProbeSent(now_ms, 1000);
|
||||
}
|
||||
|
||||
EXPECT_GE(now_ms, kMinProbeDurationMs);
|
||||
// Verify that the actual bitrate is withing 10% of the target.
|
||||
double bitrate = kProbeSize * (kClusterSize - 1) * 8 * 1000.0 / now_ms;
|
||||
EXPECT_GT(bitrate, kTestBitrate1 * 0.9);
|
||||
EXPECT_LT(bitrate, kTestBitrate1 * 1.1);
|
||||
|
||||
now_ms += prober.TimeUntilNextProbe(now_ms);
|
||||
int64_t probe2_started = now_ms;
|
||||
|
||||
for (int i = 0; i < kClusterSize; ++i) {
|
||||
now_ms += prober.TimeUntilNextProbe(now_ms);
|
||||
for (int i = 0; i < 5; ++i) {
|
||||
EXPECT_EQ(4, prober.TimeUntilNextProbe(now_ms));
|
||||
now_ms += 4;
|
||||
EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms));
|
||||
EXPECT_EQ(1, prober.CurrentClusterId());
|
||||
prober.ProbeSent(now_ms, kProbeSize);
|
||||
prober.ProbeSent(now_ms, 1000);
|
||||
}
|
||||
|
||||
// Verify that the actual bitrate is withing 10% of the target.
|
||||
int duration = now_ms - probe2_started;
|
||||
EXPECT_GE(duration, kMinProbeDurationMs);
|
||||
bitrate = kProbeSize * (kClusterSize - 1) * 8 * 1000.0 / duration;
|
||||
EXPECT_GT(bitrate, kTestBitrate2 * 0.9);
|
||||
EXPECT_LT(bitrate, kTestBitrate2 * 1.1);
|
||||
|
||||
EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
|
||||
EXPECT_FALSE(prober.IsProbing());
|
||||
}
|
||||
|
||||
@ -828,7 +828,6 @@ TEST_F(PacedSenderTest, ProbingWithInsertedPackets) {
|
||||
kFirstClusterBps, kBitrateProbingError);
|
||||
EXPECT_EQ(0, packet_sender.padding_sent());
|
||||
|
||||
clock_.AdvanceTimeMilliseconds(send_bucket_->TimeUntilNextProcess());
|
||||
start = clock_.TimeInMilliseconds();
|
||||
while (packet_sender.packets_sent() < 10) {
|
||||
int time_until_process = send_bucket_->TimeUntilNextProcess();
|
||||
@ -837,9 +836,9 @@ TEST_F(PacedSenderTest, ProbingWithInsertedPackets) {
|
||||
}
|
||||
packets_sent = packet_sender.packets_sent() - packets_sent;
|
||||
// Validate second cluster bitrate.
|
||||
EXPECT_NEAR((packets_sent - 1) * kPacketSize * 8000 /
|
||||
(clock_.TimeInMilliseconds() - start),
|
||||
kSecondClusterBps, kBitrateProbingError);
|
||||
EXPECT_NEAR(
|
||||
packets_sent * kPacketSize * 8000 / (clock_.TimeInMilliseconds() - start),
|
||||
kSecondClusterBps, kBitrateProbingError);
|
||||
}
|
||||
|
||||
TEST_F(PacedSenderTest, ProbingWithPaddingSupport) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user