zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Reland "[Battery]: Delay start of TaskQueuePacedSender." Take 2"

Browse Source 
This reverts commit 2072b87261a6505a88561bdeab3e7405d7038eaa.

Reason for revert: Causing test failure.

Original change's description:
> Reland "[Battery]: Delay start of TaskQueuePacedSender." Take 2
>
> This is a reland of 89cb65ed663a9000b9f7c90a78039bd85731e9ae
> ... and f28aade91dcc2cb8f590dc1379ac7ab5c1981909
>
> Reason for revert: crashes due to uninitialized pacing_bitrate_
> crbug.com/1190547
> Apparently pacer() is sometimes being used before EnsureStarted()
> Fix: Instead of delaying first call to SetPacingRates(),
> this CL no-ops MaybeProcessPackets() until EnsureStarted()
> is called for the first time.
>
> Original change's description:
> > [Battery]: Delay start of TaskQueuePacedSender.
> >
> > To avoid unnecessary repeating tasks, TaskQueuePacedSender is started
> > only upon RtpTransportControllerSend::EnsureStarted().
> >
> > More specifically, the repeating task happens in
> > TaskQueuePacedSender::MaybeProcessPackets() every 500ms, using a self
> > task_queue_.PostDelayedTask().
> >
> > Bug: chromium:1152887
> > Change-Id: I72c96d2c4b491d5edb45a30b210b3797165cbf48
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/208560
> > Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Reviewed-by: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#33421}
>
> Bug: chromium:1152887
> Change-Id: I9aba4882a64bbee7d97ace9059dea8a24c144f93
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/212880
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#33554}

TBR=hbos@webrtc.org,sprang@webrtc.org,etiennep@chromium.org

Change-Id: I430fd31c7602702c8ec44b9e38e68266abba8854
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1152887
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/212965
Reviewed-by: Ying Wang <yinwa@webrtc.org>
Commit-Queue: Ying Wang <yinwa@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33559}
This commit is contained in:
Ying Wang
2021-03-25 10:50:39 +00:00
committed by Commit Bot
parent 02b1321b47
commit 4c555cca2d
5 changed files with 6 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
call/rtp_transport_controller_send.cc
View File

@ -87,7 +87,7 @@ RtpTransportControllerSend::RtpTransportControllerSend(
: clock_(clock),
event_log_(event_log),
bitrate_configurator_(bitrate_config),
pacer_started_(false),
process_thread_started_(false),
process_thread_(std::move(process_thread)),
use_task_queue_pacer_(IsEnabled(trials, "WebRTC-TaskQueuePacer")),
process_thread_pacer_(use_task_queue_pacer_
@ -496,13 +496,9 @@ void RtpTransportControllerSend::IncludeOverheadInPacedSender() {
}
void RtpTransportControllerSend::EnsureStarted() {
if (!pacer_started_) {
pacer_started_ = true;
if (use_task_queue_pacer_) {
task_queue_pacer_->EnsureStarted();
} else {
process_thread_->Start();
}
if (!use_task_queue_pacer_ && !process_thread_started_) {
process_thread_started_ = true;
process_thread_->Start();
}
}

2
call/rtp_transport_controller_send.h
View File

@ -152,7 +152,7 @@ class RtpTransportControllerSend final
std::vector<std::unique_ptr<RtpVideoSenderInterface>> video_rtp_senders_;
RtpBitrateConfigurator bitrate_configurator_;
std::map<std::string, rtc::NetworkRoute> network_routes_;
bool pacer_started_;
bool process_thread_started_;
const std::unique_ptr<ProcessThread> process_thread_;
const bool use_task_queue_pacer_;
std::unique_ptr<PacedSender> process_thread_pacer_;

10
modules/pacing/task_queue_paced_sender.cc
View File

@ -62,14 +62,6 @@ TaskQueuePacedSender::~TaskQueuePacedSender() {
});
}
void TaskQueuePacedSender::EnsureStarted() {
task_queue_.PostTask([this]() {
RTC_DCHECK_RUN_ON(&task_queue_);
is_started_ = true;
MaybeProcessPackets(Timestamp::MinusInfinity());
});
}
void TaskQueuePacedSender::CreateProbeCluster(DataRate bitrate,
int cluster_id) {
task_queue_.PostTask([this, bitrate, cluster_id]() {
@ -205,7 +197,7 @@ void TaskQueuePacedSender::MaybeProcessPackets(
Timestamp scheduled_process_time) {
RTC_DCHECK_RUN_ON(&task_queue_);
if (is_shutdown_ || !is_started_) {
if (is_shutdown_) {
return;
}

7
modules/pacing/task_queue_paced_sender.h
View File

@ -55,9 +55,6 @@ class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender {
~TaskQueuePacedSender() override;
// Ensure that necessary delayed tasks are scheduled.
void EnsureStarted();
// Methods implementing RtpPacketSender.
// Adds the packet to the queue and calls PacketRouter::SendPacket() when
@ -153,10 +150,6 @@ class TaskQueuePacedSender : public RtpPacketPacer, public RtpPacketSender {
// Last time stats were updated.
Timestamp last_stats_time_ RTC_GUARDED_BY(task_queue_);
// Indicates if this task queue is started. If not, don't allow
// posting delayed tasks yet.
bool is_started_ RTC_GUARDED_BY(task_queue_) = false;
// Indicates if this task queue is shutting down. If so, don't allow
// posting any more delayed tasks as that can cause the task queue to
// never drain.

37
modules/pacing/task_queue_paced_sender_unittest.cc
View File

@ -157,7 +157,6 @@ namespace test {
pacer.SetPacingRates(
DataRate::BitsPerSec(kDefaultPacketSize * 8 * kPacketsToSend),
DataRate::Zero());
pacer.EnsureStarted();
pacer.EnqueuePackets(
GeneratePackets(RtpPacketMediaType::kVideo, kPacketsToSend));
@ -197,7 +196,6 @@ namespace test {
const DataRate kPacingRate =
DataRate::BitsPerSec(kDefaultPacketSize * 8 * kPacketsPerSecond);
pacer.SetPacingRates(kPacingRate, DataRate::Zero());
pacer.EnsureStarted();
// Send some initial packets to be rid of any probes.
EXPECT_CALL(packet_router, SendPacket).Times(kPacketsPerSecond);
@ -249,7 +247,6 @@ namespace test {
const TimeDelta kPacketPacingTime = kPacketSize / kPacingDataRate;
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
// Add some initial video packets, only one should be sent.
EXPECT_CALL(packet_router, SendPacket);
@ -283,7 +280,6 @@ namespace test {
const DataRate kPacingDataRate = kPacketSize / kPacketPacingTime;
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
// Add 10 packets. The first should be sent immediately since the buffers
// are clear.
@ -320,7 +316,6 @@ namespace test {
const DataRate kPacingDataRate = kPacketSize / kPacketPacingTime;
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
// Add 10 packets. The first should be sent immediately since the buffers
// are clear. This will also trigger the probe to start.
@ -347,7 +342,6 @@ namespace test {
kCoalescingWindow);
const DataRate kPacingDataRate = DataRate::KilobitsPerSec(300);
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
const TimeDelta kMinTimeBetweenStatsUpdates = TimeDelta::Millis(1);
@ -394,7 +388,6 @@ namespace test {
size_t num_expected_stats_updates = 0;
EXPECT_EQ(pacer.num_stats_updates_, num_expected_stats_updates);
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
pacer.EnsureStarted();
time_controller.AdvanceTime(kMinTimeBetweenStatsUpdates);
// Updating pacing rates refreshes stats.
EXPECT_EQ(pacer.num_stats_updates_, ++num_expected_stats_updates);
@ -450,7 +443,6 @@ namespace test {
const TimeDelta kPacketPacingTime = TimeDelta::Millis(4);
const DataRate kPacingDataRate = kPacketSize / kPacketPacingTime;
pacer.SetPacingRates(kPacingDataRate, /*padding_rate=*/DataRate::Zero());
pacer.EnsureStarted();
EXPECT_CALL(packet_router, FetchFec).WillRepeatedly([]() {
return std::vector<std::unique_ptr<RtpPacketToSend>>();
});
@ -522,7 +514,6 @@ namespace test {
const TimeDelta kPacketPacingTime = TimeDelta::Millis(4);
const DataRate kPacingDataRate = kPacketSize / kPacketPacingTime;
pacer.SetPacingRates(kPacingDataRate, /*padding_rate=*/DataRate::Zero());
pacer.EnsureStarted();
EXPECT_CALL(packet_router, FetchFec).WillRepeatedly([]() {
return std::vector<std::unique_ptr<RtpPacketToSend>>();
});
@ -561,33 +552,5 @@ namespace test {
EXPECT_EQ(data_sent,
kProbingRate * TimeDelta::Millis(1) + DataSize::Bytes(1));
}
TEST(TaskQueuePacedSenderTest, NoStatsUpdatesBeforeStart) {
const TimeDelta kCoalescingWindow = TimeDelta::Millis(5);
GlobalSimulatedTimeController time_controller(Timestamp::Millis(1234));
MockPacketRouter packet_router;
TaskQueuePacedSenderForTest pacer(
time_controller.GetClock(), &packet_router,
/*event_log=*/nullptr,
/*field_trials=*/nullptr, time_controller.GetTaskQueueFactory(),
kCoalescingWindow);
const DataRate kPacingDataRate = DataRate::KilobitsPerSec(300);
pacer.SetPacingRates(kPacingDataRate, DataRate::Zero());
const TimeDelta kMinTimeBetweenStatsUpdates = TimeDelta::Millis(1);
// Nothing inserted, no stats updates yet.
EXPECT_EQ(pacer.num_stats_updates_, 0u);
// Insert one packet, stats should not be updated.
pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 1));
time_controller.AdvanceTime(TimeDelta::Zero());
EXPECT_EQ(pacer.num_stats_updates_, 0u);
// Advance time of the min stats update interval, and trigger a
// refresh - stats should not be updated still.
time_controller.AdvanceTime(kMinTimeBetweenStatsUpdates);
EXPECT_EQ(pacer.num_stats_updates_, 0u);
}
} // namespace test
} // namespace webrtc