admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move RtcEventProbeClusterCreated to the network controller.

Browse Source 
Originally RtcEventProbeClusterCreated was logged in bitrate prober. This means that anyone who was using GoogCcNetworkControl wasn't logging it, and the NetworkControl wasn't self-contained.
This changes moves the responsibility for logging ProbeClusterCreated to ProbeController (where the probe is created), it also moves the responsibility for assigning probe ids to the probe controller.

Bug: None
Change-Id: If0433cc6d311b5483ea3980749b03ddbcd2bf041
Reviewed-on: https://webrtc-review.googlesource.com/c/122927
Commit-Queue: Peter Slatala <psla@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26713}
This commit is contained in:
Piotr (Peter) Slatala 2019-02-15 07:38:04 -08:00 committed by Commit Bot
parent 6255af99a8
commit c39f462b2d
17 changed files with 83 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
api/transport/network_types.h
View File

@ -176,6 +176,7 @@ struct ProbeClusterConfig {
DataRate target_data_rate = DataRate::Zero();
TimeDelta target_duration = TimeDelta::Zero();
int32_t target_probe_count = 0;
int32_t id = 0;
};
struct TargetTransferRate {

2
call/rtp_transport_controller_send.cc
View File

@ -541,7 +541,7 @@ void RtpTransportControllerSend::PostUpdates(NetworkControlUpdate update) {
}
for (const auto& probe : update.probe_cluster_configs) {
int64_t bitrate_bps = probe.target_data_rate.bps();
pacer_.CreateProbeCluster(bitrate_bps);
pacer_.CreateProbeCluster(bitrate_bps, probe.id);
}
if (update.target_rate) {
control_handler_->SetTargetRate(*update.target_rate);

2
modules/congestion_controller/goog_cc/BUILD.gn
View File

@ -170,12 +170,14 @@ rtc_source_set("probe_controller") {
"../../../api/units:data_rate",
"../../../api/units:time_delta",
"../../../api/units:timestamp",
"../../../logging:rtc_event_bwe",
"../../../logging:rtc_event_log_api",
"../../../logging:rtc_event_pacing",
"../../../rtc_base:checks",
"../../../rtc_base:rtc_base_approved",
"../../../rtc_base/system:unused",
"../../../system_wrappers:metrics",
"//third_party/abseil-cpp/absl/memory:memory",
"//third_party/abseil-cpp/absl/types:optional",
]
}

2
modules/congestion_controller/goog_cc/goog_cc_network_control.cc
View File

@ -104,7 +104,7 @@ GoogCcNetworkController::GoogCcNetworkController(RtcEventLog* event_log,
.find("Enabled") == 0),
rate_control_settings_(
RateControlSettings::ParseFromKeyValueConfig(key_value_config_)),
probe_controller_(new ProbeController(key_value_config_)),
probe_controller_(new ProbeController(key_value_config_, event_log)),
congestion_window_pushback_controller_(
rate_control_settings_.UseCongestionWindowPushback()
? absl::make_unique<CongestionWindowPushbackController>(

25
modules/congestion_controller/goog_cc/probe_controller.cc
View File

@ -13,10 +13,12 @@
#include <algorithm>
#include <initializer_list>
#include <string>
#include "absl/memory/memory.h"
#include "api/units/data_rate.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "logging/rtc_event_log/events/rtc_event_probe_cluster_created.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
@ -80,16 +82,32 @@ constexpr char kBweRapidRecoveryExperiment[] =
// Never probe higher than configured by OnMaxTotalAllocatedBitrate().
constexpr char kCappedProbingFieldTrialName[] = "WebRTC-BweCappedProbing";
void MaybeLogProbeClusterCreated(RtcEventLog* event_log,
const ProbeClusterConfig& probe) {
RTC_DCHECK(event_log);
if (!event_log) {
return;
}
size_t min_bytes = static_cast<int32_t>(probe.target_data_rate.bps() *
probe.target_duration.ms() / 8000);
event_log->Log(absl::make_unique<RtcEventProbeClusterCreated>(
probe.id, probe.target_data_rate.bps(), probe.target_probe_count,
min_bytes));
}
} // namespace
ProbeController::ProbeController(const WebRtcKeyValueConfig* key_value_config)
ProbeController::ProbeController(const WebRtcKeyValueConfig* key_value_config,
RtcEventLog* event_log)
: enable_periodic_alr_probing_(false),
in_rapid_recovery_experiment_(
key_value_config->Lookup(kBweRapidRecoveryExperiment)
.find("Enabled") == 0),
limit_probes_with_allocateable_rate_(
key_value_config->Lookup(kCappedProbingFieldTrialName)
.find("Disabled") != 0) {
.find("Disabled") != 0),
event_log_(event_log) {
Reset(0);
}
@ -364,6 +382,9 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
config.target_data_rate = DataRate::bps(rtc::dchecked_cast<int>(bitrate));
config.target_duration = TimeDelta::ms(kMinProbeDurationMs);
config.target_probe_count = kMinProbePacketsSent;
config.id = next_probe_cluster_id_;
next_probe_cluster_id_++;
MaybeLogProbeClusterCreated(event_log_, config);
pending_probes.push_back(config);
}
time_last_probing_initiated_ms_ = now_ms;

7
modules/congestion_controller/goog_cc/probe_controller.h
View File

@ -19,6 +19,7 @@
#include "absl/types/optional.h"
#include "api/transport/network_control.h"
#include "api/transport/webrtc_key_value_config.h"
#include "logging/rtc_event_log/rtc_event_log.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/system/unused.h"
@ -31,7 +32,8 @@ class Clock;
// bitrate is adjusted by an application.
class ProbeController {
public:
explicit ProbeController(const WebRtcKeyValueConfig* key_value_config);
explicit ProbeController(const WebRtcKeyValueConfig* key_value_config,
RtcEventLog* event_log);
~ProbeController();
RTC_WARN_UNUSED_RESULT std::vector<ProbeClusterConfig> SetBitrates(
@ -112,6 +114,9 @@ class ProbeController {
bool mid_call_probing_waiting_for_result_;
int64_t mid_call_probing_bitrate_bps_;
int64_t mid_call_probing_succcess_threshold_;
RtcEventLog* event_log_;
int32_t next_probe_cluster_id_ = 1;
RTC_DISALLOW_COPY_AND_ASSIGN(ProbeController);
};

8
modules/congestion_controller/goog_cc/probe_controller_unittest.cc
View File

@ -13,6 +13,7 @@
#include "api/transport/network_types.h"
#include "api/units/data_rate.h"
#include "api/units/timestamp.h"
#include "logging/rtc_event_log/mock/mock_rtc_event_log.h"
#include "modules/congestion_controller/goog_cc/probe_controller.h"
#include "system_wrappers/include/clock.h"
#include "test/gmock.h"
@ -44,7 +45,8 @@ constexpr int kBitrateDropTimeoutMs = 5000;
class ProbeControllerTest : public ::testing::Test {
protected:
ProbeControllerTest() : clock_(100000000L) {
probe_controller_.reset(new ProbeController(&field_trial_config_));
probe_controller_.reset(
new ProbeController(&field_trial_config_, &mock_rtc_event_log));
}
~ProbeControllerTest() override {}
@ -59,6 +61,7 @@ class ProbeControllerTest : public ::testing::Test {
FieldTrialBasedConfig field_trial_config_;
SimulatedClock clock_;
MockRtcEventLog mock_rtc_event_log;
std::unique_ptr<ProbeController> probe_controller_;
};
@ -225,7 +228,8 @@ TEST_F(ProbeControllerTest, PeriodicProbing) {
}
TEST_F(ProbeControllerTest, PeriodicProbingAfterReset) {
probe_controller_.reset(new ProbeController(&field_trial_config_));
probe_controller_.reset(
new ProbeController(&field_trial_config_, &mock_rtc_event_log));
int64_t alr_start_time = clock_.TimeInMilliseconds();
probe_controller_->SetAlrStartTimeMs(alr_start_time);

5
modules/congestion_controller/send_side_congestion_controller.cc
View File

@ -99,7 +99,7 @@ DEPRECATED_SendSideCongestionController::
BitrateController::CreateBitrateController(clock_, event_log)),
acknowledged_bitrate_estimator_(
absl::make_unique<AcknowledgedBitrateEstimator>(key_value_config_)),
probe_controller_(new ProbeController(key_value_config_)),
probe_controller_(new ProbeController(key_value_config_, event_log)),
retransmission_rate_limiter_(
new RateLimiter(clock, kRetransmitWindowSizeMs)),
transport_feedback_adapter_(clock_),
@ -341,7 +341,8 @@ int64_t DEPRECATED_SendSideCongestionController::TimeUntilNextProcess() {
void DEPRECATED_SendSideCongestionController::SendProbes(
std::vector<ProbeClusterConfig> probe_configs) {
for (auto probe_config : probe_configs) {
pacer_->CreateProbeCluster(probe_config.target_data_rate.bps());
pacer_->CreateProbeCluster(probe_config.target_data_rate.bps(),
probe_config.id);
}
}

8
modules/congestion_controller/send_side_congestion_controller_unittest.cc
View File

@ -62,8 +62,8 @@ class LegacySendSideCongestionControllerTest : public ::testing::Test {
// to be updated.
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps, _, _, _));
EXPECT_CALL(*pacer_, SetEstimatedBitrate(kInitialBitrateBps));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 3));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 5));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 3, 1));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 5, 2));
controller_->SetBweBitrates(0, kInitialBitrateBps, 5 * kInitialBitrateBps);
}
@ -335,8 +335,8 @@ TEST_F(LegacySendSideCongestionControllerTest, GetProbingInterval) {
TEST_F(LegacySendSideCongestionControllerTest, ProbeOnRouteChange) {
testing::Mock::VerifyAndClearExpectations(pacer_.get());
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 6));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 12));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 6, _));
EXPECT_CALL(*pacer_, CreateProbeCluster(kInitialBitrateBps * 12, _));
EXPECT_CALL(observer_, OnNetworkChanged(kInitialBitrateBps * 2, _, _, _));
rtc::NetworkRoute route;
route.local_network_id = 1;

1
modules/pacing/BUILD.gn
View File

@ -34,6 +34,7 @@ rtc_static_library("pacing") {
"../../logging:rtc_event_log_api",
"../../logging:rtc_event_pacing",
"../../rtc_base:checks",
"../../rtc_base:deprecation",
"../../rtc_base:rtc_base_approved",
"../../rtc_base/experiments:alr_experiment",
"../../rtc_base/experiments:field_trial_parser",

17
modules/pacing/bitrate_prober.cc
View File

@ -49,11 +49,9 @@ BitrateProber::BitrateProber() : BitrateProber(nullptr) {}
BitrateProber::~BitrateProber() = default;
// TODO(psla): Remove this constructor in a follow up change.
BitrateProber::BitrateProber(RtcEventLog* event_log)
: probing_state_(ProbingState::kDisabled),
next_probe_time_ms_(-1),
next_cluster_id_(0),
event_log_(event_log) {
: probing_state_(ProbingState::kDisabled), next_probe_time_ms_(-1) {
SetEnabled(true);
}
@ -85,7 +83,9 @@ void BitrateProber::OnIncomingPacket(size_t packet_size) {
}
}
void BitrateProber::CreateProbeCluster(int bitrate_bps, int64_t now_ms) {
void BitrateProber::CreateProbeCluster(int bitrate_bps,
int64_t now_ms,
int cluster_id) {
RTC_DCHECK(probing_state_ != ProbingState::kDisabled);
RTC_DCHECK_GT(bitrate_bps, 0);
while (!clusters_.empty() &&
@ -100,13 +100,8 @@ void BitrateProber::CreateProbeCluster(int bitrate_bps, int64_t now_ms) {
static_cast<int64_t>(bitrate_bps) * kMinProbeDurationMs / 8000);
RTC_DCHECK_GE(cluster.pace_info.probe_cluster_min_bytes, 0);
cluster.pace_info.send_bitrate_bps = bitrate_bps;
cluster.pace_info.probe_cluster_id = next_cluster_id_++;
cluster.pace_info.probe_cluster_id = cluster_id;
clusters_.push(cluster);
if (event_log_)
event_log_->Log(absl::make_unique<RtcEventProbeClusterCreated>(
cluster.pace_info.probe_cluster_id, cluster.pace_info.send_bitrate_bps,
cluster.pace_info.probe_cluster_min_probes,
cluster.pace_info.probe_cluster_min_bytes));
RTC_LOG(LS_INFO) << "Probe cluster (bitrate:min bytes:min packets): ("
<< cluster.pace_info.send_bitrate_bps << ":"

5
modules/pacing/bitrate_prober.h
View File

@ -42,7 +42,7 @@ class BitrateProber {
// Create a cluster used to probe for |bitrate_bps| with |num_probes| number
// of probes.
void CreateProbeCluster(int bitrate_bps, int64_t now_ms);
void CreateProbeCluster(int bitrate_bps, int64_t now_ms, int cluster_id);
// Returns the number of milliseconds until the next probe should be sent to
// get accurate probing.
@ -98,9 +98,6 @@ class BitrateProber {
// Time the next probe should be sent when in kActive state.
int64_t next_probe_time_ms_;
int next_cluster_id_;
RtcEventLog* const event_log_;
};
} // namespace webrtc

20
modules/pacing/bitrate_prober_unittest.cc
View File

@ -25,8 +25,8 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) {
const int kProbeSize = 1000;
const int kMinProbeDurationMs = 15;
prober.CreateProbeCluster(kTestBitrate1, now_ms);
prober.CreateProbeCluster(kTestBitrate2, now_ms);
prober.CreateProbeCluster(kTestBitrate1, now_ms, 0);
prober.CreateProbeCluster(kTestBitrate2, now_ms, 1);
EXPECT_FALSE(prober.IsProbing());
prober.OnIncomingPacket(kProbeSize);
@ -76,7 +76,7 @@ TEST(BitrateProberTest, DoesntProbeWithoutRecentPackets) {
int64_t now_ms = 0;
EXPECT_EQ(-1, prober.TimeUntilNextProbe(now_ms));
prober.CreateProbeCluster(900000, now_ms);
prober.CreateProbeCluster(900000, now_ms, 0);
EXPECT_FALSE(prober.IsProbing());
prober.OnIncomingPacket(1000);
@ -105,7 +105,7 @@ TEST(BitrateProberTest, VerifyProbeSizeOnHighBitrate) {
BitrateProber prober;
constexpr unsigned kHighBitrateBps = 10000000; // 10 Mbps
prober.CreateProbeCluster(kHighBitrateBps, 0);
prober.CreateProbeCluster(kHighBitrateBps, 0, /*cluster_id=*/0);
// Probe size should ensure a minimum of 1 ms interval.
EXPECT_GT(prober.RecommendedMinProbeSize(), kHighBitrateBps / 8000);
}
@ -117,7 +117,7 @@ TEST(BitrateProberTest, MinumumNumberOfProbingPackets) {
constexpr int kBitrateBps = 100000; // 100 kbps
constexpr int kPacketSizeBytes = 1000;
prober.CreateProbeCluster(kBitrateBps, 0);
prober.CreateProbeCluster(kBitrateBps, 0, 0);
prober.OnIncomingPacket(kPacketSizeBytes);
for (int i = 0; i < 5; ++i) {
EXPECT_TRUE(prober.IsProbing());
@ -133,7 +133,7 @@ TEST(BitrateProberTest, ScaleBytesUsedForProbing) {
constexpr int kPacketSizeBytes = 1000;
constexpr int kExpectedBytesSent = kBitrateBps * 15 / 8000;
prober.CreateProbeCluster(kBitrateBps, 0);
prober.CreateProbeCluster(kBitrateBps, 0, /*cluster_id=*/0);
prober.OnIncomingPacket(kPacketSizeBytes);
int bytes_sent = 0;
while (bytes_sent < kExpectedBytesSent) {
@ -151,7 +151,7 @@ TEST(BitrateProberTest, HighBitrateProbing) {
constexpr int kPacketSizeBytes = 1000;
constexpr int kExpectedBytesSent = (kBitrateBps / 8000) * 15;
prober.CreateProbeCluster(kBitrateBps, 0);
prober.CreateProbeCluster(kBitrateBps, 0, 0);
prober.OnIncomingPacket(kPacketSizeBytes);
int bytes_sent = 0;
while (bytes_sent < kExpectedBytesSent) {
@ -172,15 +172,15 @@ TEST(BitrateProberTest, ProbeClusterTimeout) {
constexpr int64_t kTimeoutMs = 5000;
int64_t now_ms = 0;
prober.CreateProbeCluster(kBitrateBps, now_ms);
prober.CreateProbeCluster(kBitrateBps, now_ms, /*cluster_id=*/0);
prober.OnIncomingPacket(kSmallPacketSize);
EXPECT_FALSE(prober.IsProbing());
now_ms += kTimeoutMs;
prober.CreateProbeCluster(kBitrateBps / 10, now_ms);
prober.CreateProbeCluster(kBitrateBps / 10, now_ms, /*cluster_id=*/1);
prober.OnIncomingPacket(kSmallPacketSize);
EXPECT_FALSE(prober.IsProbing());
now_ms += 1;
prober.CreateProbeCluster(kBitrateBps / 10, now_ms);
prober.CreateProbeCluster(kBitrateBps / 10, now_ms, /*cluster_id=*/2);
prober.OnIncomingPacket(kSmallPacketSize);
EXPECT_TRUE(prober.IsProbing());
int bytes_sent = 0;

2
modules/pacing/mock/mock_paced_sender.h
View File

@ -30,7 +30,7 @@ class MockPacedSender : public PacedSender {
int64_t capture_time_ms,
size_t bytes,
bool retransmission));
MOCK_METHOD1(CreateProbeCluster, void(int));
MOCK_METHOD2(CreateProbeCluster, void(int, int));
MOCK_METHOD1(SetEstimatedBitrate, void(uint32_t));
MOCK_METHOD2(SetPacingRates, void(uint32_t, uint32_t));
MOCK_CONST_METHOD0(QueueInMs, int64_t());

7
modules/pacing/paced_sender.cc
View File

@ -83,8 +83,13 @@ PacedSender::PacedSender(Clock* clock,
PacedSender::~PacedSender() {}
void PacedSender::CreateProbeCluster(int bitrate_bps) {
CreateProbeCluster(bitrate_bps, next_probe_cluster_id_);
next_probe_cluster_id_++;
}
void PacedSender::CreateProbeCluster(int bitrate_bps, int cluster_id) {
rtc::CritScope cs(&critsect_);
prober_.CreateProbeCluster(bitrate_bps, TimeMilliseconds());
prober_.CreateProbeCluster(bitrate_bps, TimeMilliseconds(), cluster_id);
}
void PacedSender::Pause() {

7
modules/pacing/paced_sender.h
View File

@ -25,6 +25,7 @@
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/utility/include/process_thread.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/deprecation.h"
#include "rtc_base/experiments/field_trial_parser.h"
#include "rtc_base/thread_annotations.h"
@ -74,7 +75,8 @@ class PacedSender : public Pacer {
~PacedSender() override;
virtual void CreateProbeCluster(int bitrate_bps);
RTC_DEPRECATED virtual void CreateProbeCluster(int bitrate_bps);
virtual void CreateProbeCluster(int bitrate_bps, int cluster_id);
// Temporarily pause all sending.
void Pause();
@ -206,6 +208,9 @@ class PacedSender : public Pacer {
RoundRobinPacketQueue packets_ RTC_GUARDED_BY(critsect_);
uint64_t packet_counter_ RTC_GUARDED_BY(critsect_);
// TODO(psla): Used by the RTC_DEPRECATED method, to be removed.
int next_probe_cluster_id_ = 1;
int64_t congestion_window_bytes_ RTC_GUARDED_BY(critsect_) =
kNoCongestionWindow;
int64_t outstanding_bytes_ RTC_GUARDED_BY(critsect_) = 0;

10
modules/pacing/paced_sender_unittest.cc
View File

@ -112,8 +112,8 @@ class PacedSenderTest : public testing::TestWithParam<std::string> {
srand(0);
// Need to initialize PacedSender after we initialize clock.
send_bucket_.reset(new PacedSender(&clock_, &callback_, nullptr));
send_bucket_->CreateProbeCluster(kFirstClusterBps);
send_bucket_->CreateProbeCluster(kSecondClusterBps);
send_bucket_->CreateProbeCluster(kFirstClusterBps, /*cluster_id=*/0);
send_bucket_->CreateProbeCluster(kSecondClusterBps, /*cluster_id=*/1);
// Default to bitrate probing disabled for testing purposes. Probing tests
// have to enable probing, either by creating a new PacedSender instance or
// by calling SetProbingEnabled(true).
@ -1055,8 +1055,8 @@ TEST_F(PacedSenderTest, ProbingWithInsertedPackets) {
PacedSenderProbing packet_sender;
send_bucket_.reset(new PacedSender(&clock_, &packet_sender, nullptr));
send_bucket_->CreateProbeCluster(kFirstClusterBps);
send_bucket_->CreateProbeCluster(kSecondClusterBps);
send_bucket_->CreateProbeCluster(kFirstClusterBps, /*cluster_id=*/0);
send_bucket_->CreateProbeCluster(kSecondClusterBps, /*cluster_id=*/1);
send_bucket_->SetPacingRates(kInitialBitrateBps * kPaceMultiplier, 0);
for (int i = 0; i < 10; ++i) {
@ -1101,7 +1101,7 @@ TEST_F(PacedSenderTest, ProbingWithPaddingSupport) {
PacedSenderProbing packet_sender;
send_bucket_.reset(new PacedSender(&clock_, &packet_sender, nullptr));
send_bucket_->CreateProbeCluster(kFirstClusterBps);
send_bucket_->CreateProbeCluster(kFirstClusterBps, /*cluster_id=*/0);
send_bucket_->SetPacingRates(kInitialBitrateBps * kPaceMultiplier, 0);
for (int i = 0; i < 3; ++i) {