From f5dca48dc0bce89f18b9a90f5c7c1a03bcf95233 Mon Sep 17 00:00:00 2001 From: Stefan Holmer Date: Wed, 27 Jan 2016 12:58:51 +0100 Subject: [PATCH] Add transport sequence number on the non-pacer path of the rtp sender. BUG=4173 R=sprang@webrtc.org Review URL: https://codereview.webrtc.org/1635093002 . Cr-Commit-Position: refs/heads/master@{#11395} --- webrtc/modules/rtp_rtcp/source/rtp_sender.cc | 28 ++++++++-- .../rtp_rtcp/source/rtp_sender_unittest.cc | 55 ++++++++++++++++++- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc index f2d45eb4c8..ba5b98148d 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -642,11 +642,15 @@ size_t RTPSender::SendPadData(size_t bytes, payload_type = payload_type_; over_rtx = false; } else { - // Without abs-send-time a media packet must be sent before padding so - // that the timestamps used for estimation are correct. - if (!media_has_been_sent_ && !rtp_header_extension_map_.IsRegistered( - kRtpExtensionAbsoluteSendTime)) + // Without abs-send-time or transport sequence number a media packet + // must be sent before padding so that the timestamps used for + // estimation are correct. + if (!media_has_been_sent_ && + !(rtp_header_extension_map_.IsRegistered( + kRtpExtensionAbsoluteSendTime) || + using_transport_seq)) { return 0; + } // Only change change the timestamp of padding packets sent over RTX. // Padding only packets over RTP has to be sent as part of a media // frame (and therefore the same timestamp). @@ -1079,7 +1083,21 @@ int32_t RTPSender::SendToNetwork(uint8_t* buffer, UpdateDelayStatistics(capture_time_ms, now_ms); } - bool sent = SendPacketToNetwork(buffer, length, PacketOptions()); + // TODO(sprang): Potentially too much overhead in IsRegistered()? + bool using_transport_seq = rtp_header_extension_map_.IsRegistered( + kRtpExtensionTransportSequenceNumber) && + transport_sequence_number_allocator_; + + PacketOptions options; + if (using_transport_seq) { + options.packet_id = + UpdateTransportSequenceNumber(buffer, length, rtp_header); + if (transport_feedback_observer_) { + transport_feedback_observer_->AddPacket(options.packet_id, length, true); + } + } + + bool sent = SendPacketToNetwork(buffer, length, options); // Mark the packet as sent in the history even if send failed. Dropping a // packet here should be treated as any other packet drop so we should be diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 2bf0254111..a0d6145909 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -76,7 +76,8 @@ class LoopbackTransportTest : public webrtc::Transport { : packets_sent_(0), last_sent_packet_len_(0), total_bytes_sent_(0), - last_sent_packet_(nullptr) {} + last_sent_packet_(nullptr), + last_packet_id_(-1) {} ~LoopbackTransportTest() { STLDeleteContainerPointers(sent_packets_.begin(), sent_packets_.end()); @@ -89,6 +90,7 @@ class LoopbackTransportTest : public webrtc::Transport { new rtc::Buffer(reinterpret_cast(data), len); last_sent_packet_ = buffer->data(); last_sent_packet_len_ = len; + last_packet_id_ = options.packet_id; total_bytes_sent_ += len; sent_packets_.push_back(buffer); return true; @@ -98,6 +100,7 @@ class LoopbackTransportTest : public webrtc::Transport { size_t last_sent_packet_len_; size_t total_bytes_sent_; uint8_t* last_sent_packet_; + int last_packet_id_; std::vector sent_packets_; }; @@ -117,6 +120,12 @@ class MockRtpPacketSender : public RtpPacketSender { bool retransmission)); }; +class MockTransportSequenceNumberAllocator + : public TransportSequenceNumberAllocator { + public: + MOCK_METHOD0(AllocateSequenceNumber, uint16_t()); +}; + class RtpSenderTest : public ::testing::Test { protected: RtpSenderTest() @@ -134,14 +143,15 @@ class RtpSenderTest : public ::testing::Test { void SetUpRtpSender(bool pacer) { rtp_sender_.reset(new RTPSender(false, &fake_clock_, &transport_, nullptr, pacer ? &mock_paced_sender_ : nullptr, - nullptr, nullptr, nullptr, nullptr, nullptr, - &mock_rtc_event_log_)); + &seq_num_allocator_, nullptr, nullptr, + nullptr, nullptr, &mock_rtc_event_log_)); rtp_sender_->SetSequenceNumber(kSeqNum); } SimulatedClock fake_clock_; MockRtcEventLog mock_rtc_event_log_; MockRtpPacketSender mock_paced_sender_; + MockTransportSequenceNumberAllocator seq_num_allocator_; rtc::scoped_ptr rtp_sender_; int payload_; LoopbackTransportTest transport_; @@ -464,6 +474,45 @@ TEST_F(RtpSenderTestWithoutPacer, BuildRTPPacketWithAbsoluteSendTimeExtension) { EXPECT_EQ(0u, rtp_header2.extension.absoluteSendTime); } +TEST_F(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { + // Ignore rtc event calls. + EXPECT_CALL(mock_rtc_event_log_, + LogRtpHeader(PacketDirection::kOutgoingPacket, _, _, _)); + + EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension( + kRtpExtensionTransportSequenceNumber, + kTransportSequenceNumberExtensionId)); + + char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC"; + const uint8_t payload_type = 127; + ASSERT_EQ(0, rtp_sender_->RegisterPayload(payload_name, payload_type, 90000, + 0, 1500)); + // Create a dummy payload of 5 bytes. + uint8_t payload[] = {47, 11, 32, 93, 89}; + + const uint16_t kTransportSequenceNumber = 17; + EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) + .WillOnce(testing::Return(kTransportSequenceNumber)); + const uint32_t kTimestamp = 1234; + const int64_t kCaptureTimeMs = 4321; + ASSERT_EQ(0, rtp_sender_->SendOutgoingData( + kVideoFrameKey, payload_type, kTimestamp, kCaptureTimeMs, + payload, sizeof(payload), nullptr)); + + RtpUtility::RtpHeaderParser rtp_parser(transport_.last_sent_packet_, + transport_.last_sent_packet_len_); + webrtc::RTPHeader rtp_header; + RtpHeaderExtensionMap map; + map.Register(kRtpExtensionTransportSequenceNumber, + kTransportSequenceNumberExtensionId); + EXPECT_TRUE(rtp_parser.Parse(&rtp_header, &map)); + EXPECT_TRUE(rtp_header.extension.hasTransportSequenceNumber); + EXPECT_EQ(kTransportSequenceNumber, + rtp_header.extension.transportSequenceNumber); + EXPECT_EQ(transport_.last_packet_id_, + rtp_header.extension.transportSequenceNumber); +} + // Test CVO header extension is only set when marker bit is true. TEST_F(RtpSenderTestWithoutPacer, BuildRTPPacketWithVideoRotation_MarkerBit) { rtp_sender_->SetVideoRotation(kRotation);