From bd09a46aa1b649ed5c58a9544fe1a400b60acc6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 12 May 2021 15:34:22 +0200 Subject: [PATCH] Move some tests out from rtp_sender_unittest. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves OnSendSideDelayUpdated and OnSendPacketUpdated out from rtp_sender_unittest and into rtp_sender_egress_unittest and rtp_rtcp_impl2_unittest. The former test now only tests the logic for updating send-side-delay stats. The latter is now on a proper RtpRtcp-level and also verifies that frame timestamps makes it to the egress (as assumed by the first test). Bug: webrtc:11340 Change-Id: I784042ad91eb66a4d1eebdbbc625f9522528bfb5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/218502 Commit-Queue: Erik Språng Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#33996} --- .../source/rtp_rtcp_impl2_unittest.cc | 41 ++++++++- .../source/rtp_sender_egress_unittest.cc | 54 ++++++++++- .../rtp_rtcp/source/rtp_sender_unittest.cc | 89 ------------------- 3 files changed, 93 insertions(+), 91 deletions(-) diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc index 00f8edb468..8ea87c5d88 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl2_unittest.cc @@ -31,6 +31,7 @@ #include "test/run_loop.h" #include "test/time_controller/simulated_time_controller.h" +using ::testing::AllOf; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::Field; @@ -152,8 +153,17 @@ class FieldTrialConfig : public WebRtcKeyValueConfig { double max_padding_factor_; }; -class RtpRtcpModule : public RtcpPacketTypeCounterObserver { +class RtpRtcpModule : public RtcpPacketTypeCounterObserver, + public SendPacketObserver { public: + struct SentPacket { + SentPacket(uint16_t packet_id, int64_t capture_time_ms, uint32_t ssrc) + : packet_id(packet_id), capture_time_ms(capture_time_ms), ssrc(ssrc) {} + uint16_t packet_id; + int64_t capture_time_ms; + uint32_t ssrc; + }; + RtpRtcpModule(TimeController* time_controller, bool is_sender, const FieldTrialConfig& trials) @@ -182,6 +192,16 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { counter_map_[ssrc] = packet_counter; } + void OnSendPacket(uint16_t packet_id, + int64_t capture_time_ms, + uint32_t ssrc) override { + last_sent_packet_.emplace(packet_id, capture_time_ms, ssrc); + } + + absl::optional last_sent_packet() const { + return last_sent_packet_; + } + RtcpPacketTypeCounter RtcpSent() { // RTCP counters for remote SSRC. return counter_map_[is_sender_ ? kReceiverSsrc : kSenderSsrc]; @@ -221,6 +241,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { config.need_rtp_packet_infos = true; config.non_sender_rtt_measurement = true; config.field_trials = &trials_; + config.send_packet_observer = this; impl_.reset(new ModuleRtpRtcpImpl2(config)); impl_->SetRemoteSSRC(is_sender_ ? kReceiverSsrc : kSenderSsrc); @@ -229,6 +250,7 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver { TimeController* const time_controller_; std::map counter_map_; + absl::optional last_sent_packet_; }; } // namespace @@ -915,6 +937,23 @@ TEST_P(RtpRtcpImpl2Test, AssignsAbsoluteSendTime) { EXPECT_NE(sender_.last_packet().GetExtension(), 0u); } +TEST_P(RtpRtcpImpl2Test, PropagatesSentPacketInfo) { + sender_.RegisterHeaderExtension(TransportSequenceNumber::kUri, + kTransportSequenceNumberExtensionId); + int64_t now_ms = time_controller_.GetClock()->TimeInMilliseconds(); + const int kCaptureTimeMsToRtpTimestamp = 90; // 90 kHz clock + EXPECT_TRUE(SendFrame(&sender_, sender_video_.get(), kBaseLayerTid, + /*timestamp=*/kCaptureTimeMsToRtpTimestamp * now_ms)); + EXPECT_THAT( + sender_.last_sent_packet(), + Optional( + AllOf(Field(&RtpRtcpModule::SentPacket::packet_id, + Eq(sender_.last_packet() + .GetExtension())), + Field(&RtpRtcpModule::SentPacket::capture_time_ms, Eq(now_ms)), + Field(&RtpRtcpModule::SentPacket::ssrc, Eq(kSenderSsrc))))); +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpRtcpImpl2Test, ::testing::Values(TestConfig{false}, diff --git a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc index fffb737b3b..63f013a79b 100644 --- a/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_egress_unittest.cc @@ -71,6 +71,14 @@ class MockStreamDataCountersCallback : public StreamDataCountersCallback { (override)); }; +class MockSendSideDelayObserver : public SendSideDelayObserver { + public: + MOCK_METHOD(void, + SendSideDelayUpdated, + (int, int, uint64_t, uint32_t), + (override)); +}; + class FieldTrialConfig : public WebRtcKeyValueConfig { public: FieldTrialConfig() : overhead_enabled_(false) {} @@ -180,7 +188,7 @@ class RtpSenderEgressTest : public ::testing::TestWithParam { GlobalSimulatedTimeController time_controller_; Clock* const clock_; NiceMock mock_rtc_event_log_; - StrictMock mock_rtp_stats_callback_; + NiceMock mock_rtp_stats_callback_; NiceMock send_packet_observer_; NiceMock feedback_observer_; RtpHeaderExtensionMap header_extensions_; @@ -277,6 +285,50 @@ TEST_P(RtpSenderEgressTest, EXPECT_TRUE(transport_.last_packet()->options.included_in_allocation); } +TEST_P(RtpSenderEgressTest, OnSendSideDelayUpdated) { + StrictMock send_side_delay_observer; + RtpRtcpInterface::Configuration config = DefaultConfig(); + config.send_side_delay_observer = &send_side_delay_observer; + auto sender = std::make_unique(config, &packet_history_); + + // Send packet with 10 ms send-side delay. The average, max and total should + // be 10 ms. + EXPECT_CALL(send_side_delay_observer, + SendSideDelayUpdated(10, 10, 10, kSsrc)); + int64_t capture_time_ms = clock_->TimeInMilliseconds(); + time_controller_.AdvanceTime(TimeDelta::Millis(10)); + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + PacedPacketInfo()); + + // Send another packet with 20 ms delay. The average, max and total should be + // 15, 20 and 30 ms respectively. + EXPECT_CALL(send_side_delay_observer, + SendSideDelayUpdated(15, 20, 30, kSsrc)); + capture_time_ms = clock_->TimeInMilliseconds(); + time_controller_.AdvanceTime(TimeDelta::Millis(20)); + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + PacedPacketInfo()); + + // Send another packet at the same time, which replaces the last packet. + // Since this packet has 0 ms delay, the average is now 5 ms and max is 10 ms. + // The total counter stays the same though. + // TODO(terelius): Is is not clear that this is the right behavior. + EXPECT_CALL(send_side_delay_observer, SendSideDelayUpdated(5, 10, 30, kSsrc)); + capture_time_ms = clock_->TimeInMilliseconds(); + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + PacedPacketInfo()); + + // Send a packet 1 second later. The earlier packets should have timed + // out, so both max and average should be the delay of this packet. The total + // keeps increasing. + time_controller_.AdvanceTime(TimeDelta::Seconds(1)); + EXPECT_CALL(send_side_delay_observer, SendSideDelayUpdated(1, 1, 31, kSsrc)); + capture_time_ms = clock_->TimeInMilliseconds(); + time_controller_.AdvanceTime(TimeDelta::Millis(1)); + sender->SendPacket(BuildRtpPacket(/*marker=*/true, capture_time_ms).get(), + PacedPacketInfo()); +} + INSTANTIATE_TEST_SUITE_P(WithAndWithoutOverhead, RtpSenderEgressTest, ::testing::Values(TestConfig(false), diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index aaf3adde76..8b8ef1e48d 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -545,95 +545,6 @@ TEST_P(RtpSenderTest, PaddingAlwaysAllowedOnAudio) { EXPECT_EQ(kMinPaddingSize, GenerateAndSendPadding(kMinPaddingSize - 5)); } -TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) { - StrictMock send_side_delay_observer_; - - RtpRtcpInterface::Configuration config; - config.clock = clock_; - config.outgoing_transport = &transport_; - config.local_media_ssrc = kSsrc; - config.send_side_delay_observer = &send_side_delay_observer_; - config.event_log = &mock_rtc_event_log_; - rtp_sender_context_ = - std::make_unique(config, &time_controller_); - - FieldTrialBasedConfig field_trials; - RTPSenderVideo::Config video_config; - video_config.clock = clock_; - video_config.rtp_sender = rtp_sender(); - video_config.field_trials = &field_trials; - RTPSenderVideo rtp_sender_video(video_config); - - const uint8_t kPayloadType = 127; - const absl::optional kCodecType = - VideoCodecType::kVideoCodecGeneric; - - const uint32_t kCaptureTimeMsToRtpTimestamp = 90; // 90 kHz clock - RTPVideoHeader video_header; - - // Send packet with 10 ms send-side delay. The average, max and total should - // be 10 ms. - EXPECT_CALL(send_side_delay_observer_, - SendSideDelayUpdated(10, 10, 10, kSsrc)) - .Times(1); - int64_t capture_time_ms = clock_->TimeInMilliseconds(); - time_controller_.AdvanceTime(TimeDelta::Millis(10)); - video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(rtp_sender_video.SendVideo( - kPayloadType, kCodecType, capture_time_ms * kCaptureTimeMsToRtpTimestamp, - capture_time_ms, kPayloadData, video_header, - kDefaultExpectedRetransmissionTimeMs)); - - // Send another packet with 20 ms delay. The average, max and total should be - // 15, 20 and 30 ms respectively. - EXPECT_CALL(send_side_delay_observer_, - SendSideDelayUpdated(15, 20, 30, kSsrc)) - .Times(1); - time_controller_.AdvanceTime(TimeDelta::Millis(10)); - video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(rtp_sender_video.SendVideo( - kPayloadType, kCodecType, capture_time_ms * kCaptureTimeMsToRtpTimestamp, - capture_time_ms, kPayloadData, video_header, - kDefaultExpectedRetransmissionTimeMs)); - - // Send another packet at the same time, which replaces the last packet. - // Since this packet has 0 ms delay, the average is now 5 ms and max is 10 ms. - // The total counter stays the same though. - // TODO(terelius): Is is not clear that this is the right behavior. - EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(5, 10, 30, kSsrc)) - .Times(1); - capture_time_ms = clock_->TimeInMilliseconds(); - video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(rtp_sender_video.SendVideo( - kPayloadType, kCodecType, capture_time_ms * kCaptureTimeMsToRtpTimestamp, - capture_time_ms, kPayloadData, video_header, - kDefaultExpectedRetransmissionTimeMs)); - - // Send a packet 1 second later. The earlier packets should have timed - // out, so both max and average should be the delay of this packet. The total - // keeps increasing. - time_controller_.AdvanceTime(TimeDelta::Millis(1000)); - capture_time_ms = clock_->TimeInMilliseconds(); - time_controller_.AdvanceTime(TimeDelta::Millis(1)); - EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(1, 1, 31, kSsrc)) - .Times(1); - video_header.frame_type = VideoFrameType::kVideoFrameKey; - EXPECT_TRUE(rtp_sender_video.SendVideo( - kPayloadType, kCodecType, capture_time_ms * kCaptureTimeMsToRtpTimestamp, - capture_time_ms, kPayloadData, video_header, - kDefaultExpectedRetransmissionTimeMs)); -} - -TEST_P(RtpSenderTestWithoutPacer, OnSendPacketUpdated) { - EXPECT_TRUE(rtp_sender()->RegisterRtpHeaderExtension( - TransportSequenceNumber::kUri, kTransportSequenceNumberExtensionId)); - EXPECT_CALL(send_packet_observer_, - OnSendPacket(kTransportSequenceNumber, _, _)) - .Times(1); - - SendGenericPacket(); -} - TEST_P(RtpSenderTest, SendsPacketsWithTransportSequenceNumber) { RtpRtcpInterface::Configuration config; config.clock = clock_;