Cleanup delayed packet handing in RtpRtcpImpl2 unittests
Module/ProcessThread architecture is long time gone, so delayed handling
can be done using TaskQueue, which looks simpler and resolves few TODOs
Bug: webrtc:42221687
Change-Id: Ie1cfc91eaa982097be6a4ffed2108295ef8e1e62
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/399581
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45118}
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
index 664b795..0698b34 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc
@@ -12,7 +12,6 @@
#include <cstddef>
#include <cstdint>
-#include <deque>
#include <map>
#include <memory>
#include <optional>
@@ -27,6 +26,7 @@
#include "api/rtp_headers.h"
#include "api/rtp_parameters.h"
#include "api/task_queue/task_queue_base.h"
+#include "api/task_queue/task_queue_factory.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "api/video/video_codec_type.h"
@@ -106,22 +106,19 @@
int64_t rtt_ms_;
};
-// TODO(bugs.webrtc.org/11581): remove inheritance once the ModuleRtpRtcpImpl2
-// Module/ProcessThread dependency is gone.
-class SendTransport : public Transport,
- public sim_time_impl::SimulatedSequenceRunner {
+class SendTransport : public Transport {
public:
- SendTransport(TimeDelta delay, GlobalSimulatedTimeController* time_controller)
+ SendTransport(TimeDelta delay, TaskQueueFactory& task_queue_factory)
: receiver_(nullptr),
- time_controller_(time_controller),
delay_(delay),
rtp_packets_sent_(0),
rtcp_packets_sent_(0),
- last_packet_(&header_extensions_) {
- time_controller_->Register(this);
- }
+ last_packet_(&header_extensions_),
+ rtcp_packets_(task_queue_factory.CreateTaskQueue(
+ "transport",
+ TaskQueueFactory::Priority::NORMAL)) {}
- ~SendTransport() override { time_controller_->Unregister(this); }
+ ~SendTransport() override = default;
void SetRtpRtcpModule(ModuleRtpRtcpImpl2* receiver) { receiver_ = receiver; }
void SimulateNetworkDelay(TimeDelta delay) { delay_ = delay; }
@@ -136,49 +133,33 @@
test::RtcpPacketParser parser;
parser.Parse(data);
last_nack_list_ = parser.nack()->packet_ids();
- Timestamp current_time = time_controller_->GetClock()->CurrentTime();
- Timestamp delivery_time = current_time + delay_;
- rtcp_packets_.push_back(
- Packet{delivery_time, std::vector<uint8_t>(data.begin(), data.end())});
- ++rtcp_packets_sent_;
- RunReady(current_time);
- return true;
- }
- // sim_time_impl::SimulatedSequenceRunner
- Timestamp GetNextRunTime() const override {
- if (!rtcp_packets_.empty())
- return rtcp_packets_.front().send_time;
- return Timestamp::PlusInfinity();
- }
- void RunReady(Timestamp at_time) override {
- while (!rtcp_packets_.empty() &&
- rtcp_packets_.front().send_time <= at_time) {
- Packet packet = std::move(rtcp_packets_.front());
- rtcp_packets_.pop_front();
- EXPECT_TRUE(receiver_);
- receiver_->IncomingRtcpPacket(packet.data);
+ if (delay_ == TimeDelta::Zero()) {
+ receiver_->IncomingRtcpPacket(data);
+ } else {
+ ModuleRtpRtcpImpl2* receiver = receiver_;
+ std::vector<uint8_t> packet(data.begin(), data.end());
+ rtcp_packets_->PostDelayedTask(
+ [receiver, packet = std::move(packet)] {
+ receiver->IncomingRtcpPacket(packet);
+ },
+ delay_);
}
- }
- TaskQueueBase* GetAsTaskQueue() override {
- return reinterpret_cast<TaskQueueBase*>(this);
+
+ ++rtcp_packets_sent_;
+ return true;
}
size_t NumRtcpSent() { return rtcp_packets_sent_; }
ModuleRtpRtcpImpl2* receiver_;
- GlobalSimulatedTimeController* const time_controller_;
TimeDelta delay_;
int rtp_packets_sent_;
size_t rtcp_packets_sent_;
std::vector<uint16_t> last_nack_list_;
RtpHeaderExtensionMap header_extensions_;
RtpPacketReceived last_packet_;
- struct Packet {
- Timestamp send_time;
- std::vector<uint8_t> data;
- };
- std::deque<Packet> rtcp_packets_;
+ std::unique_ptr<TaskQueueBase, TaskQueueDeleter> rtcp_packets_;
};
class RtpRtcpModule : public RtcpPacketTypeCounterObserver,
@@ -193,13 +174,11 @@
uint32_t ssrc;
};
- RtpRtcpModule(const Environment& env,
- GlobalSimulatedTimeController* time_controller,
- bool is_sender)
+ RtpRtcpModule(const Environment& env, bool is_sender)
: env_(env),
is_sender_(is_sender),
receive_statistics_(ReceiveStatistics::Create(&env.clock())),
- transport_(kOneWayNetworkDelay, time_controller) {
+ transport_(kOneWayNetworkDelay, env_.task_queue_factory()) {
CreateModuleImpl();
}
@@ -304,12 +283,8 @@
: time_controller_(Timestamp::Micros(133590000000000)),
env_(CreateEnvironment(time_controller_.GetClock(),
time_controller_.CreateTaskQueueFactory())),
- sender_(env_,
- &time_controller_,
- /*is_sender=*/true),
- receiver_(env_,
- &time_controller_,
- /*is_sender=*/false) {}
+ sender_(env_, /*is_sender=*/true),
+ receiver_(env_, /*is_sender=*/false) {}
void SetUp() override {
// Send module.
diff --git a/test/time_controller/simulated_time_controller.cc b/test/time_controller/simulated_time_controller.cc
index 6985157..31b8701 100644
--- a/test/time_controller/simulated_time_controller.cc
+++ b/test/time_controller/simulated_time_controller.cc
@@ -231,14 +231,4 @@
global_clock_.AdvanceTime(duration);
}
-void GlobalSimulatedTimeController::Register(
- sim_time_impl::SimulatedSequenceRunner* runner) {
- impl_.Register(runner);
-}
-
-void GlobalSimulatedTimeController::Unregister(
- sim_time_impl::SimulatedSequenceRunner* runner) {
- impl_.Unregister(runner);
-}
-
} // namespace webrtc
diff --git a/test/time_controller/simulated_time_controller.h b/test/time_controller/simulated_time_controller.h
index b934941..7db4883 100644
--- a/test/time_controller/simulated_time_controller.h
+++ b/test/time_controller/simulated_time_controller.h
@@ -151,17 +151,6 @@
// Useful for simulating contention on destination queues.
void SkipForwardBy(TimeDelta duration);
- // Makes the simulated time controller aware of a custom
- // SimulatedSequenceRunner.
- // TODO(bugs.webrtc.org/11581): remove method once the ModuleRtpRtcpImpl2 unit
- // test stops using it.
- void Register(sim_time_impl::SimulatedSequenceRunner* runner);
- // Removes a previously installed custom SimulatedSequenceRunner from the
- // simulated time controller.
- // TODO(bugs.webrtc.org/11581): remove method once the ModuleRtpRtcpImpl2 unit
- // test stops using it.
- void Unregister(sim_time_impl::SimulatedSequenceRunner* runner);
-
private:
ScopedBaseFakeClock global_clock_;
// Provides simulated CurrentNtpInMilliseconds()
