From c7bf32a110af7f427e14c0dc32e3f3789e51c096 Mon Sep 17 00:00:00 2001 From: philipel Date: Fri, 17 Feb 2017 03:59:43 -0800 Subject: [PATCH] Propagate packet pacing information to SenTimeHistory. In order to not make this CL too large I have broken it down into at least two steps. In this CL we only propagate the pacing information part of the way: webrtc::PacedSender::Process <--- propagate from here webrtc::PacedSender::SendPacket webrtc::PacketRouter::TimeToSendPacket webrtc::ModuleRtpRtcpImpl::TimeToSendPacket <--- to here webrtc::RTPSender::TimeToSendPacket webrtc::RTPSender::PrepareAndSendPacket webrtc::RTPSender::AddPacketToTransportFeedback webrtc::TransportFeedbackAdapter::AddPacket webrtc::SendTimeHistory::AddAndRemoveOld <--- goal is to propagte it here BUG=webrtc:6822 Review-Url: https://codereview.webrtc.org/2628563003 Cr-Commit-Position: refs/heads/master@{#16664} --- .../congestion_controller/delay_based_bwe.cc | 2 +- .../delay_based_bwe_unittest.cc | 3 +- .../delay_based_bwe_unittest_helper.cc | 10 ++- .../probe_bitrate_estimator.cc | 2 +- .../transport_feedback_adapter_unittest.cc | 12 +-- webrtc/modules/include/module_common_types.h | 16 ++++ webrtc/modules/pacing/bitrate_prober.cc | 34 +++---- webrtc/modules/pacing/bitrate_prober.h | 11 +-- .../modules/pacing/bitrate_prober_unittest.cc | 6 +- webrtc/modules/pacing/paced_sender.cc | 20 +++-- webrtc/modules/pacing/paced_sender.h | 10 ++- .../modules/pacing/paced_sender_unittest.cc | 35 +++++--- webrtc/modules/pacing/packet_router.cc | 8 +- webrtc/modules/pacing/packet_router.h | 5 +- .../modules/pacing/packet_router_unittest.cc | 89 ++++++++++++------- .../send_time_history_unittest.cc | 29 +++--- .../test/estimators/send_side.cc | 2 +- .../test/packet_sender.cc | 5 +- .../test/packet_sender.h | 5 +- webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 5 +- .../rtp_rtcp/include/rtp_rtcp_defines.h | 5 +- webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 5 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 12 +-- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 5 +- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 4 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 47 +++++----- webrtc/tools/event_log_visualizer/analyzer.cc | 5 +- 27 files changed, 235 insertions(+), 157 deletions(-) diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index b89daab7d1..d18847fb08 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -361,7 +361,7 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo( } int probing_bps = 0; - if (info.probe_cluster_id != PacketInfo::kNotAProbe) { + if (info.probe_cluster_id != PacedPacketInfo::kNotAProbe) { probing_bps = probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); } rtc::Optional acked_bitrate_bps = diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc index dd24b53272..e0c4b1d1f6 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest.cc @@ -57,7 +57,8 @@ TEST_F(DelayBasedBweTest, ProbeDetectionNonPacedPackets) { IncomingFeedback(now_ms, now_ms, seq_num++, 1000, 0); // Non-paced packet, arriving 5 ms after. clock_.AdvanceTimeMilliseconds(5); - IncomingFeedback(now_ms, now_ms, seq_num++, 100, PacketInfo::kNotAProbe); + IncomingFeedback(now_ms, now_ms, seq_num++, 100, + PacedPacketInfo::kNotAProbe); } EXPECT_TRUE(bitrate_observer_.updated()); diff --git a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc index 4eaaad2920..298a73023c 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe_unittest_helper.cc @@ -60,7 +60,7 @@ int64_t RtpStream::GenerateFrame(int64_t time_now_us, PacketInfo packet(-1, sequence_number_++); packet.send_time_ms = (time_now_us + kSendSideOffsetUs) / 1000; packet.payload_size = payload_size; - packet.probe_cluster_id = PacketInfo::kNotAProbe; + packet.probe_cluster_id = PacedPacketInfo::kNotAProbe; packets->push_back(packet); } next_rtp_time_ = time_now_us + (1000000 + fps_ / 2) / fps_; @@ -169,7 +169,7 @@ void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, uint16_t sequence_number, size_t payload_size) { IncomingFeedback(arrival_time_ms, send_time_ms, sequence_number, payload_size, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); } void DelayBasedBweTest::IncomingFeedback(int64_t arrival_time_ms, @@ -280,7 +280,8 @@ void DelayBasedBweTest::InitialBehaviorTestHelper( for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) { // NOTE!!! If the following line is moved under the if case then this test // wont work on windows realease bots. - int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe; + int cluster_id = + i < kInitialProbingPackets ? 0 : PacedPacketInfo::kNotAProbe; if (i == kNumInitialPackets) { EXPECT_FALSE(bitrate_estimator_->LatestEstimate(&ssrcs, &bitrate_bps)); @@ -312,7 +313,8 @@ void DelayBasedBweTest::RateIncreaseReorderingTestHelper( for (int i = 0; i < 5 * kFramerate + 1 + kNumInitialPackets; ++i) { // NOTE!!! If the following line is moved under the if case then this test // wont work on windows realease bots. - int cluster_id = i < kInitialProbingPackets ? 0 : PacketInfo::kNotAProbe; + int cluster_id = + i < kInitialProbingPackets ? 0 : PacedPacketInfo::kNotAProbe; // TODO(sprang): Remove this hack once the single stream estimator is gone, // as it doesn't do anything in Process(). diff --git a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc index c69bd1a429..b9307e20b4 100644 --- a/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc +++ b/webrtc/modules/congestion_controller/probe_bitrate_estimator.cc @@ -38,7 +38,7 @@ ProbeBitrateEstimator::ProbeBitrateEstimator() {} int ProbeBitrateEstimator::HandleProbeAndEstimateBitrate( const PacketInfo& packet_info) { - RTC_DCHECK_NE(packet_info.probe_cluster_id, PacketInfo::kNotAProbe); + RTC_DCHECK_NE(packet_info.probe_cluster_id, PacedPacketInfo::kNotAProbe); EraseOldClusters(packet_info.arrival_time_ms - kMaxClusterHistoryMs); diff --git a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc index 184b416e3b..f4589c2a49 100644 --- a/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc +++ b/webrtc/modules/congestion_controller/transport_feedback_adapter_unittest.cc @@ -258,11 +258,11 @@ TEST_F(TransportFeedbackAdapterTest, SendTimeWrapsBothWays) { static_cast((1 << 23) - 1) / 1000; std::vector packets; packets.push_back(PacketInfo(kHighArrivalTimeMs - 64, 200, 0, 1500, - PacketInfo::kNotAProbe)); + PacedPacketInfo::kNotAProbe)); packets.push_back(PacketInfo(kHighArrivalTimeMs + 64, 210, 1, 1500, - PacketInfo::kNotAProbe)); - packets.push_back( - PacketInfo(kHighArrivalTimeMs, 220, 2, 1500, PacketInfo::kNotAProbe)); + PacedPacketInfo::kNotAProbe)); + packets.push_back(PacketInfo(kHighArrivalTimeMs, 220, 2, 1500, + PacedPacketInfo::kNotAProbe)); for (const PacketInfo& packet : packets) OnSentPacket(packet); @@ -329,7 +329,7 @@ TEST_F(TransportFeedbackAdapterTest, TimestampDeltas) { rtcp::TransportFeedback::kDeltaScaleFactor * std::numeric_limits::min(); - PacketInfo info(100, 200, 0, 1500, true, PacketInfo::kNotAProbe); + PacketInfo info(100, 200, 0, 1500, true, PacedPacketInfo::kNotAProbe); sent_packets.push_back(info); info.send_time_ms += kSmallDeltaUs / 1000; @@ -418,7 +418,7 @@ TEST_F(TransportFeedbackAdapterTest, UpdatesDelayBasedEstimate) { int64_t start_time_ms = clock_.TimeInMilliseconds(); while (clock_.TimeInMilliseconds() - start_time_ms < kRunTimeMs) { PacketInfo packet(clock_.TimeInMilliseconds(), clock_.TimeInMilliseconds(), - seq_num, kPayloadSize, PacketInfo::kNotAProbe); + seq_num, kPayloadSize, PacedPacketInfo::kNotAProbe); OnSentPacket(packet); // Create expected feedback and send into adapter. std::unique_ptr feedback( diff --git a/webrtc/modules/include/module_common_types.h b/webrtc/modules/include/module_common_types.h index c7854322fd..1826610437 100644 --- a/webrtc/modules/include/module_common_types.h +++ b/webrtc/modules/include/module_common_types.h @@ -529,6 +529,22 @@ class SequenceNumberUnwrapper { int64_t last_seq_; }; +struct PacedPacketInfo { + PacedPacketInfo() {} + PacedPacketInfo(int probe_cluster_id, + int probe_cluster_min_probes, + int probe_cluster_min_bytes) + : probe_cluster_id(probe_cluster_id), + probe_cluster_min_probes(probe_cluster_min_probes), + probe_cluster_min_bytes(probe_cluster_min_bytes) {} + + static constexpr int kNotAProbe = -1; + int send_bitrate_bps = -1; + int probe_cluster_id = kNotAProbe; + int probe_cluster_min_probes = -1; + int probe_cluster_min_bytes = -1; +}; + } // namespace webrtc #endif // WEBRTC_MODULES_INCLUDE_MODULE_COMMON_TYPES_H_ diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc index 39fe0f4fd2..dd9ee33257 100644 --- a/webrtc/modules/pacing/bitrate_prober.cc +++ b/webrtc/modules/pacing/bitrate_prober.cc @@ -88,16 +88,18 @@ void BitrateProber::CreateProbeCluster(int bitrate_bps, int64_t now_ms) { } ProbeCluster cluster; - cluster.min_probes = kMinProbePacketsSent; - cluster.min_bytes = bitrate_bps * kMinProbeDurationMs / 8000; - cluster.bitrate_bps = bitrate_bps; cluster.time_created_ms = now_ms; - cluster.id = next_cluster_id_++; + cluster.pace_info.probe_cluster_min_probes = kMinProbePacketsSent; + cluster.pace_info.probe_cluster_min_bytes = + bitrate_bps * kMinProbeDurationMs / 8000; + cluster.pace_info.send_bitrate_bps = bitrate_bps; + cluster.pace_info.probe_cluster_id = next_cluster_id_++; clusters_.push(cluster); LOG(LS_INFO) << "Probe cluster (bitrate:min bytes:min packets): (" - << cluster.bitrate_bps << ":" << cluster.min_bytes << ":" - << cluster.min_probes << ")"; + << cluster.pace_info.send_bitrate_bps << ":" + << cluster.pace_info.probe_cluster_min_bytes << ":" + << cluster.pace_info.probe_cluster_min_probes << ")"; // If we are already probing, continue to do so. Otherwise set it to // kInactive and wait for OnIncomingPacket to start the probing. if (probing_state_ != ProbingState::kActive) @@ -112,7 +114,7 @@ void BitrateProber::ResetState(int64_t now_ms) { clusters.swap(clusters_); while (!clusters.empty()) { if (clusters.front().retries < kMaxRetryAttempts) { - CreateProbeCluster(clusters.front().bitrate_bps, now_ms); + CreateProbeCluster(clusters.front().pace_info.send_bitrate_bps, now_ms); clusters_.back().retries = clusters.front().retries + 1; } clusters.pop(); @@ -138,10 +140,10 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) { return std::max(time_until_probe_ms, 0); } -int BitrateProber::CurrentClusterId() const { +PacedPacketInfo BitrateProber::CurrentCluster() const { RTC_DCHECK(!clusters_.empty()); RTC_DCHECK(ProbingState::kActive == probing_state_); - return clusters_.front().id; + return clusters_.front().pace_info; } // Probe size is recommended based on the probe bitrate required. We choose @@ -149,7 +151,8 @@ int BitrateProber::CurrentClusterId() const { // feasible. size_t BitrateProber::RecommendedMinProbeSize() const { RTC_DCHECK(!clusters_.empty()); - return clusters_.front().bitrate_bps * 2 * kMinProbeDeltaMs / (8 * 1000); + return clusters_.front().pace_info.send_bitrate_bps * 2 * kMinProbeDeltaMs / + (8 * 1000); } void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) { @@ -165,8 +168,8 @@ void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) { cluster->sent_bytes += static_cast(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) { + if (cluster->sent_bytes >= cluster->pace_info.probe_cluster_min_bytes && + cluster->sent_probes >= cluster->pace_info.probe_cluster_min_probes) { clusters_.pop(); } if (clusters_.empty()) @@ -175,13 +178,14 @@ void BitrateProber::ProbeSent(int64_t now_ms, size_t bytes) { } int64_t BitrateProber::GetNextProbeTime(const ProbeCluster& cluster) { - RTC_CHECK_GT(cluster.bitrate_bps, 0); + RTC_CHECK_GT(cluster.pace_info.send_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; + int64_t delta_ms = + (8000ll * cluster.sent_bytes + cluster.pace_info.send_bitrate_bps / 2) / + cluster.pace_info.send_bitrate_bps; return cluster.time_started_ms + delta_ms; } diff --git a/webrtc/modules/pacing/bitrate_prober.h b/webrtc/modules/pacing/bitrate_prober.h index 875c4c2302..0690750bbd 100644 --- a/webrtc/modules/pacing/bitrate_prober.h +++ b/webrtc/modules/pacing/bitrate_prober.h @@ -14,6 +14,7 @@ #include #include "webrtc/base/basictypes.h" +#include "webrtc/modules/include/module_common_types.h" #include "webrtc/typedefs.h" namespace webrtc { @@ -44,8 +45,8 @@ class BitrateProber { // get accurate probing. int TimeUntilNextProbe(int64_t now_ms); - // Which cluster that is currently being used for probing. - int CurrentClusterId() const; + // Information about the current probing cluster. + PacedPacketInfo CurrentCluster() const; // Returns the minimum number of bytes that the prober recommends for // the next probe. @@ -74,16 +75,12 @@ 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. struct ProbeCluster { - int min_probes = 0; - int min_bytes = 0; - int bitrate_bps = 0; - int id = -1; + PacedPacketInfo pace_info; int sent_probes = 0; int sent_bytes = 0; int64_t time_created_ms = -1; int64_t time_started_ms = -1; - int retries = 0; }; diff --git a/webrtc/modules/pacing/bitrate_prober_unittest.cc b/webrtc/modules/pacing/bitrate_prober_unittest.cc index d36d8108b9..7040bed40f 100644 --- a/webrtc/modules/pacing/bitrate_prober_unittest.cc +++ b/webrtc/modules/pacing/bitrate_prober_unittest.cc @@ -33,7 +33,7 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { prober.OnIncomingPacket(kProbeSize); EXPECT_TRUE(prober.IsProbing()); - EXPECT_EQ(0, prober.CurrentClusterId()); + EXPECT_EQ(0, prober.CurrentCluster().probe_cluster_id); // First packet should probe as soon as possible. EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); @@ -41,7 +41,7 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { for (int i = 0; i < kClusterSize; ++i) { now_ms += prober.TimeUntilNextProbe(now_ms); EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); - EXPECT_EQ(0, prober.CurrentClusterId()); + EXPECT_EQ(0, prober.CurrentCluster().probe_cluster_id); prober.ProbeSent(now_ms, kProbeSize); } @@ -57,7 +57,7 @@ TEST(BitrateProberTest, VerifyStatesAndTimeBetweenProbes) { for (int i = 0; i < kClusterSize; ++i) { now_ms += prober.TimeUntilNextProbe(now_ms); EXPECT_EQ(0, prober.TimeUntilNextProbe(now_ms)); - EXPECT_EQ(1, prober.CurrentClusterId()); + EXPECT_EQ(1, prober.CurrentCluster().probe_cluster_id); prober.ProbeSent(now_ms, kProbeSize); } diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index b13cc7a53a..6a97d025ab 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -410,11 +410,11 @@ void PacedSender::Process() { } bool is_probing = prober_->IsProbing(); - int probe_cluster_id = PacketInfo::kNotAProbe; + PacedPacketInfo pacing_info; size_t bytes_sent = 0; size_t recommended_probe_size = 0; if (is_probing) { - probe_cluster_id = prober_->CurrentClusterId(); + pacing_info = prober_->CurrentCluster(); recommended_probe_size = prober_->RecommendedMinProbeSize(); } while (!packets_->Empty()) { @@ -423,7 +423,7 @@ void PacedSender::Process() { // reinsert it if send fails. const paced_sender::Packet& packet = packets_->BeginPop(); - if (SendPacket(packet, probe_cluster_id)) { + if (SendPacket(packet, pacing_info)) { // Send succeeded, remove it from the queue. bytes_sent += packet.bytes; packets_->FinalizePop(packet); @@ -445,7 +445,7 @@ void PacedSender::Process() { : padding_budget_->bytes_remaining()); if (padding_needed > 0) - bytes_sent += SendPadding(padding_needed, probe_cluster_id); + bytes_sent += SendPadding(padding_needed, pacing_info); } } if (is_probing && bytes_sent > 0) @@ -454,17 +454,18 @@ void PacedSender::Process() { } bool PacedSender::SendPacket(const paced_sender::Packet& packet, - int probe_cluster_id) { + const PacedPacketInfo& pacing_info) { if (paused_) return false; if (media_budget_->bytes_remaining() == 0 && - probe_cluster_id == PacketInfo::kNotAProbe) { + pacing_info.probe_cluster_id == PacedPacketInfo::kNotAProbe) { return false; } + critsect_->Leave(); const bool success = packet_sender_->TimeToSendPacket( packet.ssrc, packet.sequence_number, packet.capture_time_ms, - packet.retransmission, probe_cluster_id); + packet.retransmission, pacing_info); critsect_->Enter(); if (success) { @@ -479,10 +480,11 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet, return success; } -size_t PacedSender::SendPadding(size_t padding_needed, int probe_cluster_id) { +size_t PacedSender::SendPadding(size_t padding_needed, + const PacedPacketInfo& pacing_info) { critsect_->Leave(); size_t bytes_sent = - packet_sender_->TimeToSendPadding(padding_needed, probe_cluster_id); + packet_sender_->TimeToSendPadding(padding_needed, pacing_info); critsect_->Enter(); if (bytes_sent > 0) { diff --git a/webrtc/modules/pacing/paced_sender.h b/webrtc/modules/pacing/paced_sender.h index 7fd65111f2..10d65c006c 100644 --- a/webrtc/modules/pacing/paced_sender.h +++ b/webrtc/modules/pacing/paced_sender.h @@ -46,10 +46,11 @@ class PacedSender : public Module, public RtpPacketSender { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) = 0; + const PacedPacketInfo& cluster_info) = 0; // Called when it's a good time to send a padding data. // Returns the number of bytes sent. - virtual size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) = 0; + virtual size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& cluster_info) = 0; protected: virtual ~PacketSender() {} @@ -145,9 +146,10 @@ class PacedSender : public Module, public RtpPacketSender { void UpdateBudgetWithBytesSent(size_t bytes) EXCLUSIVE_LOCKS_REQUIRED(critsect_); - bool SendPacket(const paced_sender::Packet& packet, int probe_cluster_id) + bool SendPacket(const paced_sender::Packet& packet, + const PacedPacketInfo& cluster_info) EXCLUSIVE_LOCKS_REQUIRED(critsect_); - size_t SendPadding(size_t padding_needed, int probe_cluster_id) + size_t SendPadding(size_t padding_needed, const PacedPacketInfo& cluster_info) EXCLUSIVE_LOCKS_REQUIRED(critsect_); Clock* const clock_; diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index c1421a57c1..cc4769e667 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -17,6 +17,7 @@ #include "webrtc/test/gtest.h" using testing::_; +using testing::Field; using testing::Return; namespace { @@ -41,8 +42,9 @@ class MockPacedSenderCallback : public PacedSender::PacketSender { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id)); - MOCK_METHOD2(TimeToSendPadding, size_t(size_t bytes, int probe_cluster_id)); + const PacedPacketInfo& pacing_info)); + MOCK_METHOD2(TimeToSendPadding, + size_t(size_t bytes, const PacedPacketInfo& pacing_info)); }; class PacedSenderPadding : public PacedSender::PacketSender { @@ -53,11 +55,12 @@ class PacedSenderPadding : public PacedSender::PacketSender { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) override { + const PacedPacketInfo& pacing_info) override { return true; } - size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override { + size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& pacing_info) override { const size_t kPaddingPacketSize = 224; size_t num_packets = (bytes + kPaddingPacketSize - 1) / kPaddingPacketSize; padding_sent_ += kPaddingPacketSize * num_packets; @@ -78,12 +81,13 @@ class PacedSenderProbing : public PacedSender::PacketSender { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) override { + const PacedPacketInfo& pacing_info) override { packets_sent_++; return true; } - size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override { + size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& pacing_info) override { padding_sent_ += bytes; return padding_sent_; } @@ -127,7 +131,6 @@ class PacedSenderTest : public ::testing::Test { .Times(1) .WillRepeatedly(Return(true)); } - SimulatedClock clock_; MockPacedSenderCallback callback_; std::unique_ptr send_bucket_; @@ -1011,7 +1014,9 @@ TEST_F(PacedSenderTest, ProbeClusterId) { } // First probing cluster. - EXPECT_CALL(callback_, TimeToSendPacket(_, _, _, _, 0)) + EXPECT_CALL(callback_, + TimeToSendPacket(_, _, _, _, + Field(&PacedPacketInfo::probe_cluster_id, 0))) .Times(5) .WillRepeatedly(Return(true)); for (int i = 0; i < 5; ++i) { @@ -1020,7 +1025,9 @@ TEST_F(PacedSenderTest, ProbeClusterId) { } // Second probing cluster. - EXPECT_CALL(callback_, TimeToSendPacket(_, _, _, _, 1)) + EXPECT_CALL(callback_, + TimeToSendPacket(_, _, _, _, + Field(&PacedPacketInfo::probe_cluster_id, 1))) .Times(5) .WillRepeatedly(Return(true)); for (int i = 0; i < 5; ++i) { @@ -1028,10 +1035,14 @@ TEST_F(PacedSenderTest, ProbeClusterId) { send_bucket_->Process(); } + // Needed for the Field comparer below. + const int kNotAProbe = PacedPacketInfo::kNotAProbe; // No more probing packets. - EXPECT_CALL(callback_, TimeToSendPadding(_, PacketInfo::kNotAProbe)) - .Times(1) - .WillRepeatedly(Return(500)); + EXPECT_CALL(callback_, + TimeToSendPadding( + _, Field(&PacedPacketInfo::probe_cluster_id, kNotAProbe))) + .Times(1) + .WillRepeatedly(Return(500)); send_bucket_->Process(); } diff --git a/webrtc/modules/pacing/packet_router.cc b/webrtc/modules/pacing/packet_router.cc index 3deb3a8faf..fcb1e4976c 100644 --- a/webrtc/modules/pacing/packet_router.cc +++ b/webrtc/modules/pacing/packet_router.cc @@ -50,7 +50,7 @@ bool PacketRouter::TimeToSendPacket(uint32_t ssrc, uint16_t sequence_number, int64_t capture_timestamp, bool retransmission, - int probe_cluster_id) { + const PacedPacketInfo& pacing_info) { RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread()); rtc::CritScope cs(&modules_crit_); for (auto* rtp_module : rtp_modules_) { @@ -59,14 +59,14 @@ bool PacketRouter::TimeToSendPacket(uint32_t ssrc, if (ssrc == rtp_module->SSRC() || ssrc == rtp_module->FlexfecSsrc()) { return rtp_module->TimeToSendPacket(ssrc, sequence_number, capture_timestamp, retransmission, - probe_cluster_id); + pacing_info); } } return true; } size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, - int probe_cluster_id) { + const PacedPacketInfo& pacing_info) { RTC_DCHECK(pacer_thread_checker_.CalledOnValidThread()); size_t total_bytes_sent = 0; rtc::CritScope cs(&modules_crit_); @@ -74,7 +74,7 @@ size_t PacketRouter::TimeToSendPadding(size_t bytes_to_send, for (RtpRtcp* module : rtp_modules_) { if (module->SendingMedia() && module->HasBweExtensions()) { size_t bytes_sent = module->TimeToSendPadding( - bytes_to_send - total_bytes_sent, probe_cluster_id); + bytes_to_send - total_bytes_sent, pacing_info); total_bytes_sent += bytes_sent; if (total_bytes_sent >= bytes_to_send) break; diff --git a/webrtc/modules/pacing/packet_router.h b/webrtc/modules/pacing/packet_router.h index 95d630ace4..0dc3fb53ba 100644 --- a/webrtc/modules/pacing/packet_router.h +++ b/webrtc/modules/pacing/packet_router.h @@ -44,9 +44,10 @@ class PacketRouter : public PacedSender::PacketSender, uint16_t sequence_number, int64_t capture_timestamp, bool retransmission, - int probe_cluster_id) override; + const PacedPacketInfo& packet_info) override; - size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override; + size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& packet_info) override; void SetTransportWideSequenceNumber(uint16_t sequence_number); uint16_t AllocateSequenceNumber() override; diff --git a/webrtc/modules/pacing/packet_router_unittest.cc b/webrtc/modules/pacing/packet_router_unittest.cc index a0688ffbbe..967fdccae0 100644 --- a/webrtc/modules/pacing/packet_router_unittest.cc +++ b/webrtc/modules/pacing/packet_router_unittest.cc @@ -21,6 +21,7 @@ using ::testing::_; using ::testing::AnyNumber; +using ::testing::Field; using ::testing::NiceMock; using ::testing::Return; @@ -30,6 +31,8 @@ class PacketRouterTest : public ::testing::Test { public: PacketRouterTest() : packet_router_(new PacketRouter()) {} protected: + static const int kProbeMinProbes = 5; + static const int kProbeMinBytes = 1000; const std::unique_ptr packet_router_; }; @@ -47,13 +50,15 @@ TEST_F(PacketRouterTest, TimeToSendPacket) { // Send on the first module by letting rtp_1 be sending with correct ssrc. EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_1, SSRC()).Times(1).WillOnce(Return(kSsrc1)); - EXPECT_CALL(rtp_1, TimeToSendPacket(kSsrc1, sequence_number, timestamp, - retransmission, 1)) + EXPECT_CALL(rtp_1, TimeToSendPacket( + kSsrc1, sequence_number, timestamp, retransmission, + Field(&PacedPacketInfo::probe_cluster_id, 1))) .Times(1) .WillOnce(Return(true)); EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1, sequence_number, - timestamp, retransmission, 1)); + EXPECT_TRUE(packet_router_->TimeToSendPacket( + kSsrc1, sequence_number, timestamp, retransmission, + PacedPacketInfo(1, kProbeMinProbes, kProbeMinBytes))); // Send on the second module by letting rtp_2 be sending, but not rtp_1. ++sequence_number; @@ -64,20 +69,23 @@ TEST_F(PacketRouterTest, TimeToSendPacket) { EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_CALL(rtp_2, TimeToSendPacket(kSsrc2, sequence_number, timestamp, - retransmission, 2)) + EXPECT_CALL(rtp_2, TimeToSendPacket( + kSsrc2, sequence_number, timestamp, retransmission, + Field(&PacedPacketInfo::probe_cluster_id, 2))) .Times(1) .WillOnce(Return(true)); - EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc2, sequence_number, - timestamp, retransmission, 2)); + EXPECT_TRUE(packet_router_->TimeToSendPacket( + kSsrc2, sequence_number, timestamp, retransmission, + PacedPacketInfo(2, kProbeMinProbes, kProbeMinBytes))); // No module is sending, hence no packet should be sent. EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _, _)).Times(0); EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1, sequence_number, - timestamp, retransmission, 1)); + EXPECT_TRUE(packet_router_->TimeToSendPacket( + kSsrc1, sequence_number, timestamp, retransmission, + PacedPacketInfo(1, kProbeMinProbes, kProbeMinBytes))); // Add a packet with incorrect ssrc and test it's dropped in the router. EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); @@ -86,8 +94,9 @@ TEST_F(PacketRouterTest, TimeToSendPacket) { EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); EXPECT_CALL(rtp_1, TimeToSendPacket(_, _, _, _, _)).Times(0); EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1 + kSsrc2, sequence_number, - timestamp, retransmission, 1)); + EXPECT_TRUE(packet_router_->TimeToSendPacket( + kSsrc1 + kSsrc2, sequence_number, timestamp, retransmission, + PacedPacketInfo(1, kProbeMinProbes, kProbeMinBytes))); packet_router_->RemoveRtpModule(&rtp_1); @@ -96,9 +105,10 @@ TEST_F(PacketRouterTest, TimeToSendPacket) { EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_2, SSRC()).Times(1).WillOnce(Return(kSsrc2)); EXPECT_CALL(rtp_2, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc1, sequence_number, - timestamp, retransmission, - PacketInfo::kNotAProbe)); + EXPECT_TRUE(packet_router_->TimeToSendPacket( + kSsrc1, sequence_number, timestamp, retransmission, + PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, + kProbeMinBytes))); packet_router_->RemoveRtpModule(&rtp_2); } @@ -123,17 +133,22 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { const size_t sent_padding_bytes = 890; EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, 111)) + EXPECT_CALL(rtp_2, + TimeToSendPadding(requested_padding_bytes, + Field(&PacedPacketInfo::probe_cluster_id, 111))) .Times(1) .WillOnce(Return(sent_padding_bytes)); EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_1, HasBweExtensions()).Times(1).WillOnce(Return(true)); - EXPECT_CALL(rtp_1, TimeToSendPadding( - requested_padding_bytes - sent_padding_bytes, 111)) + EXPECT_CALL(rtp_1, + TimeToSendPadding(requested_padding_bytes - sent_padding_bytes, + Field(&PacedPacketInfo::probe_cluster_id, 111))) .Times(1) .WillOnce(Return(requested_padding_bytes - sent_padding_bytes)); EXPECT_EQ(requested_padding_bytes, - packet_router_->TimeToSendPadding(requested_padding_bytes, 111)); + packet_router_->TimeToSendPadding( + requested_padding_bytes, + PacedPacketInfo(111, kProbeMinBytes, kProbeMinBytes))); // Let only the lower priority module be sending and verify the padding // request is routed there. @@ -145,16 +160,21 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { .Times(1) .WillOnce(Return(sent_padding_bytes)); EXPECT_EQ(sent_padding_bytes, - packet_router_->TimeToSendPadding(requested_padding_bytes, - PacketInfo::kNotAProbe)); + packet_router_->TimeToSendPadding( + requested_padding_bytes, + PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, + kProbeMinBytes))); // No sending module at all. EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(false)); EXPECT_CALL(rtp_1, TimeToSendPadding(requested_padding_bytes, _)).Times(0); EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(false)); EXPECT_CALL(rtp_2, TimeToSendPadding(_, _)).Times(0); - EXPECT_EQ(0u, packet_router_->TimeToSendPadding(requested_padding_bytes, - PacketInfo::kNotAProbe)); + EXPECT_EQ(0u, + packet_router_->TimeToSendPadding( + requested_padding_bytes, + PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, + kProbeMinBytes))); // Only one module has BWE extensions. EXPECT_CALL(rtp_1, SendingMedia()).Times(1).WillOnce(Return(true)); @@ -166,8 +186,10 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { .Times(1) .WillOnce(Return(sent_padding_bytes)); EXPECT_EQ(sent_padding_bytes, - packet_router_->TimeToSendPadding(requested_padding_bytes, - PacketInfo::kNotAProbe)); + packet_router_->TimeToSendPadding( + requested_padding_bytes, + PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, + kProbeMinBytes))); packet_router_->RemoveRtpModule(&rtp_1); @@ -176,8 +198,11 @@ TEST_F(PacketRouterTest, TimeToSendPadding) { EXPECT_CALL(rtp_2, SendingMedia()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_2, HasBweExtensions()).Times(1).WillOnce(Return(true)); EXPECT_CALL(rtp_2, TimeToSendPadding(requested_padding_bytes, _)).Times(1); - EXPECT_EQ(0u, packet_router_->TimeToSendPadding(requested_padding_bytes, - PacketInfo::kNotAProbe)); + EXPECT_EQ(0u, + packet_router_->TimeToSendPadding( + requested_padding_bytes, + PacedPacketInfo(PacedPacketInfo::kNotAProbe, kProbeMinBytes, + kProbeMinBytes))); packet_router_->RemoveRtpModule(&rtp_2); } @@ -191,11 +216,15 @@ TEST_F(PacketRouterTest, SenderOnlyFunctionsRespectSendingMedia) { // Verify that TimeToSendPacket does not end up in a receiver. EXPECT_CALL(rtp, TimeToSendPacket(_, _, _, _, _)).Times(0); - EXPECT_TRUE(packet_router_->TimeToSendPacket(kSsrc, 1, 1, false, - PacketInfo::kNotAProbe)); + EXPECT_TRUE(packet_router_->TimeToSendPacket( + kSsrc, 1, 1, false, PacedPacketInfo(PacedPacketInfo::kNotAProbe, + kProbeMinBytes, kProbeMinBytes))); // Verify that TimeToSendPadding does not end up in a receiver. EXPECT_CALL(rtp, TimeToSendPadding(_, _)).Times(0); - EXPECT_EQ(0u, packet_router_->TimeToSendPadding(200, PacketInfo::kNotAProbe)); + EXPECT_EQ(0u, + packet_router_->TimeToSendPadding( + 200, PacedPacketInfo(PacedPacketInfo::kNotAProbe, + kProbeMinBytes, kProbeMinBytes))); packet_router_->RemoveRtpModule(&rtp); } diff --git a/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc b/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc index 843f36e4f2..ce7d021587 100644 --- a/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc +++ b/webrtc/modules/remote_bitrate_estimator/send_time_history_unittest.cc @@ -52,7 +52,7 @@ class PacketInfo : public webrtc::PacketInfo { 0, sequence_number, 0, - PacketInfo::kNotAProbe) {} + PacedPacketInfo::kNotAProbe) {} PacketInfo(int64_t arrival_time_ms, int64_t send_time_ms, uint16_t sequence_number, @@ -98,7 +98,7 @@ TEST_F(SendTimeHistoryTest, PopulatesExpectedFields) { const size_t kPayloadSize = 42; AddPacketWithSendTime(kSeqNo, kPayloadSize, kSendTime, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); PacketInfo info(kReceiveTime, kSeqNo); EXPECT_TRUE(history_.GetInfo(&info, true)); @@ -121,7 +121,7 @@ TEST_F(SendTimeHistoryTest, AddThenRemoveOutOfOrder) { kProbeClusterId)); received_packets.push_back(PacketInfo( static_cast(i) + kTransmissionTime, 0, - static_cast(i), kPacketSize, PacketInfo::kNotAProbe)); + static_cast(i), kPacketSize, PacedPacketInfo::kNotAProbe)); } for (size_t i = 0; i < num_items; ++i) { history_.AddAndRemoveOld(sent_packets[i].sequence_number, @@ -148,19 +148,21 @@ TEST_F(SendTimeHistoryTest, HistorySize) { const int kItems = kDefaultHistoryLengthMs / 100; for (int i = 0; i < kItems; ++i) { clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(i, 0, i * 100, PacketInfo::kNotAProbe); + AddPacketWithSendTime(i, 0, i * 100, PacedPacketInfo::kNotAProbe); } for (int i = 0; i < kItems; ++i) { - PacketInfo info(0, 0, static_cast(i), 0, PacketInfo::kNotAProbe); + PacketInfo info(0, 0, static_cast(i), 0, + PacedPacketInfo::kNotAProbe); EXPECT_TRUE(history_.GetInfo(&info, false)); EXPECT_EQ(i * 100, info.send_time_ms); } clock_.AdvanceTimeMilliseconds(101); - AddPacketWithSendTime(kItems, 0, kItems * 101, PacketInfo::kNotAProbe); - PacketInfo info(0, 0, 0, 0, PacketInfo::kNotAProbe); + AddPacketWithSendTime(kItems, 0, kItems * 101, PacedPacketInfo::kNotAProbe); + PacketInfo info(0, 0, 0, 0, PacedPacketInfo::kNotAProbe); EXPECT_FALSE(history_.GetInfo(&info, false)); for (int i = 1; i < (kItems + 1); ++i) { - PacketInfo info2(0, 0, static_cast(i), 0, PacketInfo::kNotAProbe); + PacketInfo info2(0, 0, static_cast(i), 0, + PacedPacketInfo::kNotAProbe); EXPECT_TRUE(history_.GetInfo(&info2, false)); int64_t expected_time_ms = (i == kItems) ? i * 101 : i * 100; EXPECT_EQ(expected_time_ms, info2.send_time_ms); @@ -169,16 +171,17 @@ TEST_F(SendTimeHistoryTest, HistorySize) { TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) { const uint16_t kMaxSeqNo = std::numeric_limits::max(); - AddPacketWithSendTime(kMaxSeqNo - 2, 0, 0, PacketInfo::kNotAProbe); + AddPacketWithSendTime(kMaxSeqNo - 2, 0, 0, PacedPacketInfo::kNotAProbe); clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(kMaxSeqNo - 1, 1, 100, PacketInfo::kNotAProbe); + AddPacketWithSendTime(kMaxSeqNo - 1, 1, 100, PacedPacketInfo::kNotAProbe); clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(kMaxSeqNo, 0, 200, PacketInfo::kNotAProbe); + AddPacketWithSendTime(kMaxSeqNo, 0, 200, PacedPacketInfo::kNotAProbe); clock_.AdvanceTimeMilliseconds(kDefaultHistoryLengthMs - 200 + 1); - AddPacketWithSendTime(0, 0, kDefaultHistoryLengthMs, PacketInfo::kNotAProbe); + AddPacketWithSendTime(0, 0, kDefaultHistoryLengthMs, + PacedPacketInfo::kNotAProbe); PacketInfo info(0, static_cast(kMaxSeqNo - 2)); EXPECT_FALSE(history_.GetInfo(&info, false)); @@ -194,7 +197,7 @@ TEST_F(SendTimeHistoryTest, HistorySizeWithWraparound) { EXPECT_TRUE(history_.GetInfo(&info5, true)); clock_.AdvanceTimeMilliseconds(100); - AddPacketWithSendTime(1, 0, 1100, PacketInfo::kNotAProbe); + AddPacketWithSendTime(1, 0, 1100, PacedPacketInfo::kNotAProbe); PacketInfo info6(0, static_cast(kMaxSeqNo - 2)); EXPECT_FALSE(history_.GetInfo(&info6, false)); diff --git a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc index 4a384a0705..a53ca6834b 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/estimators/send_side.cc @@ -112,7 +112,7 @@ void SendSideBweSender::OnPacketsSent(const Packets& packets) { // to create tests for probing using cluster ids. send_time_history_.AddAndRemoveOld(media_packet->header().sequenceNumber, media_packet->payload_size(), - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); send_time_history_.OnSentPacket(media_packet->header().sequenceNumber, media_packet->sender_timestamp_ms()); } diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc index 06fbc6e913..f0f57b4e70 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.cc @@ -275,7 +275,7 @@ bool PacedVideoSender::TimeToSendPacket(uint32_t ssrc, uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) { + const PacedPacketInfo& pacing_info) { for (Packets::iterator it = pacer_queue_.begin(); it != pacer_queue_.end(); ++it) { MediaPacket* media_packet = static_cast(*it); @@ -297,7 +297,8 @@ bool PacedVideoSender::TimeToSendPacket(uint32_t ssrc, return false; } -size_t PacedVideoSender::TimeToSendPadding(size_t bytes, int probe_cluster_id) { +size_t PacedVideoSender::TimeToSendPadding(size_t bytes, + const PacedPacketInfo& pacing_info) { return 0; } diff --git a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h index 0fccb6f1d4..2bc3d01ae7 100644 --- a/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h +++ b/webrtc/modules/remote_bitrate_estimator/test/packet_sender.h @@ -114,8 +114,9 @@ class PacedVideoSender : public VideoSender, public PacedSender::PacketSender { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) override; - size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override; + const PacedPacketInfo& pacing_info) override; + size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& pacing_info) override; // Implements BitrateObserver. void OnNetworkChanged(uint32_t target_bitrate_bps, diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 5ebb252c25..186e135b34 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -249,9 +249,10 @@ class RtpRtcp : public Module { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) = 0; + const PacedPacketInfo& pacing_info) = 0; - virtual size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) = 0; + virtual size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& pacing_info) = 0; // Called on generation of new statistics after an RTP send. virtual void RegisterSendChannelRtpStatisticsCallback( diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h index 567fa0bc25..364d971837 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -251,7 +251,7 @@ struct PacketInfo { -1, sequence_number, 0, - kNotAProbe) {} + PacedPacketInfo::kNotAProbe) {} PacketInfo(int64_t arrival_time_ms, int64_t send_time_ms, @@ -278,8 +278,6 @@ struct PacketInfo { payload_size(payload_size), probe_cluster_id(probe_cluster_id) {} - static constexpr int kNotAProbe = -1; - // Time corresponding to when this object was created. int64_t creation_time_ms; // Time corresponding to when the packet was received. Timestamped with the @@ -294,6 +292,7 @@ struct PacketInfo { // Size of the packet excluding RTP headers. size_t payload_size; // Which probing cluster this packets belongs to. + // TODO(philipel): replace this with pacing information when it is available. int probe_cluster_id; }; diff --git a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index a2e14f9a3c..cc7058a4e4 100644 --- a/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -114,8 +114,9 @@ class MockRtpRtcp : public RtpRtcp { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id)); - MOCK_METHOD2(TimeToSendPadding, size_t(size_t bytes, int probe_cluster_id)); + const PacedPacketInfo& pacing_info)); + MOCK_METHOD2(TimeToSendPadding, + size_t(size_t bytes, const PacedPacketInfo& pacing_info)); MOCK_METHOD2(RegisterRtcpObservers, void(RtcpIntraFrameObserver* intra_frame_callback, RtcpBandwidthObserver* bandwidth_callback)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 50885f457f..f74ebba0c0 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -408,14 +408,16 @@ bool ModuleRtpRtcpImpl::TimeToSendPacket(uint32_t ssrc, uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) { + const PacedPacketInfo& pacing_info) { return rtp_sender_.TimeToSendPacket(ssrc, sequence_number, capture_time_ms, - retransmission, probe_cluster_id); + retransmission, + pacing_info.probe_cluster_id); } -size_t ModuleRtpRtcpImpl::TimeToSendPadding(size_t bytes, - int probe_cluster_id) { - return rtp_sender_.TimeToSendPadding(bytes, probe_cluster_id); +size_t ModuleRtpRtcpImpl::TimeToSendPadding( + size_t bytes, + const PacedPacketInfo& pacing_info) { + return rtp_sender_.TimeToSendPadding(bytes, pacing_info.probe_cluster_id); } size_t ModuleRtpRtcpImpl::MaxPayloadSize() const { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 188bd9b743..f1af359137 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -128,11 +128,12 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission, - int probe_cluster_id) override; + const PacedPacketInfo& pacing_info) override; // Returns the number of padding bytes actually sent, which can be more or // less than |bytes|. - size_t TimeToSendPadding(size_t bytes, int probe_cluster_id) override; + size_t TimeToSendPadding(size_t bytes, + const PacedPacketInfo& pacing_info) override; // RTCP part. diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index 9ceebd99e0..47ec31b918 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -622,7 +622,7 @@ int32_t RTPSender::ReSendPacket(uint16_t packet_id, int64_t min_resend_time) { bool rtx = (RtxStatus() & kRtxRetransmitted) > 0; int32_t packet_size = static_cast(packet->size()); if (!PrepareAndSendPacket(std::move(packet), rtx, true, - PacketInfo::kNotAProbe)) + PacedPacketInfo::kNotAProbe)) return -1; return packet_size; } @@ -880,7 +880,7 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, PacketOptions options; if (UpdateTransportSequenceNumber(packet.get(), &options.packet_id)) { AddPacketToTransportFeedback(options.packet_id, *packet.get(), - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); } UpdateDelayStatistics(packet->capture_time_ms(), now_ms); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index a6a886be94..5c0ff1f1ae 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -355,7 +355,7 @@ TEST_F(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { EXPECT_CALL(feedback_observer_, AddPacket(kTransportSequenceNumber, sizeof(kPayloadData) + kGenericHeaderLength, - PacketInfo::kNotAProbe)) + PacedPacketInfo::kNotAProbe)) .Times(1); SendGenericPayload(); @@ -450,7 +450,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingWithExtensions) { fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); // Process send bucket. Packet should now be sent. EXPECT_EQ(1, transport_.packets_sent()); @@ -501,7 +501,7 @@ TEST_F(RtpSenderTest, TrafficSmoothingRetransmits) { EXPECT_EQ(0, transport_.packets_sent()); rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, capture_time_ms, false, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); // Process send bucket. Packet should now be sent. EXPECT_EQ(1, transport_.packets_sent()); @@ -560,7 +560,7 @@ TEST_F(RtpSenderTest, SendPadding) { const int kStoredTimeInMs = 100; fake_clock_.AdvanceTimeMilliseconds(kStoredTimeInMs); rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); // Packet should now be sent. This test doesn't verify the regular video // packet, since it is tested in another test. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); @@ -572,8 +572,9 @@ TEST_F(RtpSenderTest, SendPadding) { const size_t kPaddingBytes = 100; const size_t kMaxPaddingLength = 224; // Value taken from rtp_sender.cc. // Padding will be forced to full packets. - EXPECT_EQ(kMaxPaddingLength, rtp_sender_->TimeToSendPadding( - kPaddingBytes, PacketInfo::kNotAProbe)); + EXPECT_EQ(kMaxPaddingLength, + rtp_sender_->TimeToSendPadding(kPaddingBytes, + PacedPacketInfo::kNotAProbe)); // Process send bucket. Padding should now be sent. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); @@ -611,7 +612,7 @@ TEST_F(RtpSenderTest, SendPadding) { RtpPacketSender::kNormalPriority)); rtp_sender_->TimeToSendPacket(kSsrc, seq_num, capture_time_ms, false, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); // Process send bucket. EXPECT_EQ(++total_packets_sent, transport_.packets_sent()); EXPECT_EQ(packet_size, transport_.last_sent_packet().size()); @@ -645,7 +646,7 @@ TEST_F(RtpSenderTest, OnSendPacketUpdated) { const bool kIsRetransmit = false; rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), kIsRetransmit, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); EXPECT_EQ(1, transport_.packets_sent()); } @@ -665,7 +666,7 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedForRetransmits) { const bool kIsRetransmit = true; rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), kIsRetransmit, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); EXPECT_EQ(1, transport_.packets_sent()); } @@ -691,7 +692,7 @@ TEST_F(RtpSenderTest, OnSendPacketNotUpdatedWithoutSeqNumAllocator) { const bool kIsRetransmit = false; rtp_sender_->TimeToSendPacket(kSsrc, kSeqNum, fake_clock_.TimeInMilliseconds(), kIsRetransmit, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); EXPECT_EQ(1, transport_.packets_sent()); } @@ -734,7 +735,7 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { EXPECT_CALL(transport, SendRtp(_, _, _)).WillOnce(testing::Return(true)); SendPacket(capture_time_ms, kPayloadSizes[i]); rtp_sender_->TimeToSendPacket(kSsrc, seq_num++, capture_time_ms, false, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); fake_clock_.AdvanceTimeMilliseconds(33); } @@ -746,13 +747,13 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { EXPECT_CALL(transport, SendRtp(_, kMaxPaddingSize + rtp_header_len, _)) .WillOnce(testing::Return(true)); EXPECT_EQ(kMaxPaddingSize, - rtp_sender_->TimeToSendPadding(49, PacketInfo::kNotAProbe)); + rtp_sender_->TimeToSendPadding(49, PacedPacketInfo::kNotAProbe)); EXPECT_CALL(transport, SendRtp(_, kPayloadSizes[0] + rtp_header_len + kRtxHeaderSize, _)) .WillOnce(testing::Return(true)); EXPECT_EQ(kPayloadSizes[0], - rtp_sender_->TimeToSendPadding(500, PacketInfo::kNotAProbe)); + rtp_sender_->TimeToSendPadding(500, PacedPacketInfo::kNotAProbe)); EXPECT_CALL(transport, SendRtp(_, kPayloadSizes[kNumPayloadSizes - 1] + rtp_header_len + kRtxHeaderSize, @@ -761,7 +762,7 @@ TEST_F(RtpSenderTest, SendRedundantPayloads) { EXPECT_CALL(transport, SendRtp(_, kMaxPaddingSize + rtp_header_len, _)) .WillOnce(testing::Return(true)); EXPECT_EQ(kPayloadSizes[kNumPayloadSizes - 1] + kMaxPaddingSize, - rtp_sender_->TimeToSendPadding(999, PacketInfo::kNotAProbe)); + rtp_sender_->TimeToSendPadding(999, PacedPacketInfo::kNotAProbe)); } TEST_F(RtpSenderTestWithoutPacer, SendGenericVideo) { @@ -1155,7 +1156,7 @@ TEST_F(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) { callback.Matches(ssrc, expected); // Send padding. - rtp_sender_->TimeToSendPadding(kMaxPaddingSize, PacketInfo::kNotAProbe); + rtp_sender_->TimeToSendPadding(kMaxPaddingSize, PacedPacketInfo::kNotAProbe); expected.transmitted.payload_bytes = 12; expected.transmitted.header_bytes = 36; expected.transmitted.padding_bytes = kMaxPaddingSize; @@ -1283,8 +1284,8 @@ TEST_F(RtpSenderTestWithoutPacer, BytesReportedCorrectly) { sizeof(payload), nullptr, nullptr, nullptr)); // Will send 2 full-size padding packets. - rtp_sender_->TimeToSendPadding(1, PacketInfo::kNotAProbe); - rtp_sender_->TimeToSendPadding(1, PacketInfo::kNotAProbe); + rtp_sender_->TimeToSendPadding(1, PacedPacketInfo::kNotAProbe); + rtp_sender_->TimeToSendPadding(1, PacedPacketInfo::kNotAProbe); StreamDataCounters rtp_stats; StreamDataCounters rtx_stats; @@ -1483,7 +1484,7 @@ TEST_F(RtpSenderTest, AddOverheadToTransportFeedbackObserver) { AddPacket(kTransportSequenceNumber, sizeof(kPayloadData) + kGenericHeaderLength + kRtpOverheadBytesPerPacket, - PacketInfo::kNotAProbe)) + PacedPacketInfo::kNotAProbe)) .Times(1); EXPECT_CALL(mock_overhead_observer, OnOverheadChanged(kRtpOverheadBytesPerPacket)) @@ -1506,14 +1507,16 @@ TEST_F(RtpSenderTest, SendAudioPadding) { const size_t kPaddingSize = 59; EXPECT_CALL(transport, SendRtp(_, kPaddingSize + kRtpHeaderSize, _)) .WillOnce(testing::Return(true)); - EXPECT_EQ(kPaddingSize, rtp_sender_->TimeToSendPadding( - kPaddingSize, PacketInfo::kNotAProbe)); + EXPECT_EQ(kPaddingSize, + rtp_sender_->TimeToSendPadding(kPaddingSize, + PacedPacketInfo::kNotAProbe)); // Requested padding size is too small, will send a larger one. const size_t kMinPaddingSize = 50; EXPECT_CALL(transport, SendRtp(_, kMinPaddingSize + kRtpHeaderSize, _)) .WillOnce(testing::Return(true)); - EXPECT_EQ(kMinPaddingSize, rtp_sender_->TimeToSendPadding( - kMinPaddingSize - 5, PacketInfo::kNotAProbe)); + EXPECT_EQ(kMinPaddingSize, + rtp_sender_->TimeToSendPadding(kMinPaddingSize - 5, + PacedPacketInfo::kNotAProbe)); } } // namespace webrtc diff --git a/webrtc/tools/event_log_visualizer/analyzer.cc b/webrtc/tools/event_log_visualizer/analyzer.cc index f49356e2a5..2f8b7ed02f 100644 --- a/webrtc/tools/event_log_visualizer/analyzer.cc +++ b/webrtc/tools/event_log_visualizer/analyzer.cc @@ -1054,7 +1054,7 @@ void EventLogAnalyzer::CreateBweSimulationGraph(Plot* plot) { RTC_DCHECK(rtp.header.extension.hasTransportSequenceNumber); cc.GetTransportFeedbackObserver()->AddPacket( rtp.header.extension.transportSequenceNumber, rtp.total_length, - PacketInfo::kNotAProbe); + PacedPacketInfo::kNotAProbe); rtc::SentPacket sent_packet( rtp.header.extension.transportSequenceNumber, rtp.timestamp / 1000); cc.OnSentPacket(sent_packet); @@ -1184,7 +1184,8 @@ void EventLogAnalyzer::CreateNetworkDelayFeedbackGraph(Plot* plot) { if (rtp.header.extension.hasTransportSequenceNumber) { RTC_DCHECK(rtp.header.extension.hasTransportSequenceNumber); feedback_adapter.AddPacket(rtp.header.extension.transportSequenceNumber, - rtp.total_length, PacketInfo::kNotAProbe); + rtp.total_length, + PacedPacketInfo::kNotAProbe); feedback_adapter.OnSentPacket( rtp.header.extension.transportSequenceNumber, rtp.timestamp / 1000); }