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

Always post RemovePacketsForSsrc

Browse Source 
If during shutdown the pacer has packets enqueued during pause these
packets will be posted to the pacer after the worker thread is free -
after the ssrcs should have been cleared. This fixes flakes in
picutre_id_tests.

Bug: webrtc:14985
Change-Id: Ib5547a501670fc145543df32fdc43bbc6596375f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297401
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39640}
This commit is contained in:
Evan Shrubsole 2023-03-22 12:36:48 +00:00 committed by WebRTC LUCI CQ
parent 48aa2b29db
commit a7b691499b
4 changed files with 56 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
modules/pacing/BUILD.gn
View File

@ -98,6 +98,7 @@ if (rtc_include_tests) {
"../../api/task_queue:task_queue",
"../../api/transport:network_control",
"../../api/units:data_rate",
"../../api/units:data_size",
"../../api/units:time_delta",
"../../api/units:timestamp",
"../../rtc_base:checks",

11
modules/pacing/task_queue_paced_sender.cc
View File

@ -155,11 +155,12 @@ void TaskQueuePacedSender::EnqueuePackets(
}
void TaskQueuePacedSender::RemovePacketsForSsrc(uint32_t ssrc) {
task_queue_.RunOrPost([this, ssrc]() {
RTC_DCHECK_RUN_ON(&task_queue_);
pacing_controller_.RemovePacketsForSsrc(ssrc);
MaybeProcessPackets(Timestamp::MinusInfinity());
});
task_queue_.TaskQueueForPost()->PostTask(
task_queue_.MaybeSafeTask(safety_.flag(), [this, ssrc] {
RTC_DCHECK_RUN_ON(&task_queue_);
pacing_controller_.RemovePacketsForSsrc(ssrc);
MaybeProcessPackets(Timestamp::MinusInfinity());
}));
}
void TaskQueuePacedSender::SetAccountForAudioPackets(bool account_for_audio) {

47
modules/pacing/task_queue_paced_sender_unittest.cc
View File

@ -18,11 +18,14 @@
#include <utility>
#include <vector>
#include "absl/functional/any_invocable.h"
#include "api/task_queue/task_queue_base.h"
#include "api/task_queue/task_queue_factory.h"
#include "api/transport/network_types.h"
#include "api/units/data_rate.h"
#include "api/units/data_size.h"
#include "api/units/time_delta.h"
#include "modules/pacing/packet_router.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/scoped_key_value_config.h"
@ -667,6 +670,48 @@ TEST_P(TaskQueuePacedSenderTest, ProbingStopDuringSendLoop) {
time_controller.AdvanceTime(kPacketsPacedTime + TimeDelta::Millis(1));
}
TEST_P(TaskQueuePacedSenderTest, PostedPacketsNotSendFromRemovePacketsForSsrc) {
static constexpr Timestamp kStartTime = Timestamp::Millis(1234);
GlobalSimulatedTimeController time_controller(kStartTime);
ScopedKeyValueConfig trials(GetParam());
MockPacketRouter packet_router;
TaskQueuePacedSender pacer(time_controller.GetClock(), &packet_router, trials,
time_controller.GetTaskQueueFactory(),
PacingController::kMinSleepTime,
TaskQueuePacedSender::kNoPacketHoldback);
static constexpr DataRate kPacingRate =
DataRate::BytesPerSec(kDefaultPacketSize * 10);
pacer.SetPacingRates(kPacingRate, DataRate::Zero());
pacer.EnsureStarted();
auto encoder_queue = time_controller.GetTaskQueueFactory()->CreateTaskQueue(
"encoder_queue", TaskQueueFactory::Priority::HIGH);
EXPECT_CALL(packet_router, SendPacket).Times(5);
encoder_queue->PostTask([&pacer] {
pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 6));
});
time_controller.AdvanceTime(TimeDelta::Millis(400));
// 1 packet left.
EXPECT_EQ(pacer.OldestPacketWaitTime(), TimeDelta::Millis(400));
EXPECT_EQ(pacer.FirstSentPacketTime(), kStartTime);
// Enqueue packets while removing ssrcs should not send any more packets.
encoder_queue->PostTask(
[&pacer, worker_thread = time_controller.GetMainThread()] {
worker_thread->PostTask(
[&pacer] { pacer.RemovePacketsForSsrc(kVideoSsrc); });
pacer.EnqueuePackets(GeneratePackets(RtpPacketMediaType::kVideo, 5));
});
time_controller.AdvanceTime(TimeDelta::Seconds(1));
EXPECT_EQ(pacer.OldestPacketWaitTime(), TimeDelta::Zero());
EXPECT_EQ(pacer.FirstSentPacketTime(), kStartTime);
EXPECT_EQ(pacer.QueueSizeData(), DataSize::Zero());
EXPECT_EQ(pacer.ExpectedQueueTime(), TimeDelta::Zero());
}
TEST_P(TaskQueuePacedSenderTest, Stats) {
static constexpr Timestamp kStartTime = Timestamp::Millis(1234);
GlobalSimulatedTimeController time_controller(kStartTime);

10
video/picture_id_tests.cc
View File

@ -367,8 +367,7 @@ TEST_P(PictureIdTest, ContinuousAfterReconfigureVp8) {
TestPictureIdContinuousAfterReconfigure({1, 3, 3, 1, 1});
}
// TODO(bugs.webrtc.org/14985): Investigate and reenable.
TEST_P(PictureIdTest, DISABLED_IncreasingAfterRecreateStreamVp8) {
TEST_P(PictureIdTest, IncreasingAfterRecreateStreamVp8) {
test::FunctionVideoEncoderFactory encoder_factory(
[]() { return VP8Encoder::Create(); });
SetupEncoder(&encoder_factory, "VP8");
@ -395,9 +394,7 @@ TEST_P(PictureIdTest, ContinuousAfterReconfigureSimulcastEncoderAdapter) {
TestPictureIdContinuousAfterReconfigure({1, 3, 3, 1, 1});
}
// TODO(bugs.webrtc.org/14985): Investigate and reenable.
TEST_P(PictureIdTest,
DISABLED_IncreasingAfterRecreateStreamSimulcastEncoderAdapter) {
TEST_P(PictureIdTest, IncreasingAfterRecreateStreamSimulcastEncoderAdapter) {
InternalEncoderFactory internal_encoder_factory;
test::FunctionVideoEncoderFactory encoder_factory(
[&internal_encoder_factory]() {
@ -421,8 +418,7 @@ TEST_P(PictureIdTest, ContinuousAfterStreamCountChangeSimulcastEncoderAdapter) {
TestPictureIdContinuousAfterReconfigure({3, 1, 3});
}
// TODO(bugs.webrtc.org/14985): Investigate and reenable.
TEST_P(PictureIdTest, DISABLED_IncreasingAfterRecreateStreamVp9) {
TEST_P(PictureIdTest, IncreasingAfterRecreateStreamVp9) {
test::FunctionVideoEncoderFactory encoder_factory(
[]() { return VP9Encoder::Create(); });
SetupEncoder(&encoder_factory, "VP9");