From 1bca65bdc9014fa274fe7aee5c331755c71504bc Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Wed, 10 Oct 2018 09:58:08 +0200 Subject: [PATCH] Makes RtpSender indicate allocation and feedback status on packets. Streams that are part of transport feedback are assumed to be part of allocation. A SetAsPartOfAllocation method is introduced to be used by media streams that are part of bitrate allocation but not included in feedback. This is part of a series of CLs that allows GoogCC to track sent bitrate that is included in bitrate allocation but without transport feedback. Bug: webrtc:9796 Change-Id: If7ac1ad3e6f3c28b2ada2aef1607a42689d899b2 Reviewed-on: https://webrtc-review.googlesource.com/c/104881 Commit-Queue: Sebastian Jansson Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#25079} --- modules/rtp_rtcp/include/rtp_rtcp.h | 4 ++ modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 1 + modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 5 +++ modules/rtp_rtcp/source/rtp_rtcp_impl.h | 2 + modules/rtp_rtcp/source/rtp_sender.cc | 17 +++++++- modules/rtp_rtcp/source/rtp_sender.h | 4 +- .../rtp_rtcp/source/rtp_sender_unittest.cc | 40 ++++++++++++++++++- 7 files changed, 70 insertions(+), 3 deletions(-) diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index 9b29f7622e..a99544cfa6 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -213,6 +213,10 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { // Returns current media sending status. virtual bool SendingMedia() const = 0; + // Indicate that the packets sent by this module should be counted towards the + // bitrate estimate since the stream participates in the bitrate allocation. + virtual void SetAsPartOfAllocation(bool part_of_allocation) = 0; + // Returns current bitrate in Kbit/s. virtual void BitrateSent(uint32_t* total_rate, uint32_t* video_rate, diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index 6d5bde4788..e95bdd38b8 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -83,6 +83,7 @@ class MockRtpRtcp : public RtpRtcp { MOCK_CONST_METHOD0(Sending, bool()); MOCK_METHOD1(SetSendingMediaStatus, void(bool sending)); MOCK_CONST_METHOD0(SendingMedia, bool()); + MOCK_METHOD1(SetAsPartOfAllocation, void(bool)); MOCK_CONST_METHOD4(BitrateSent, void(uint32_t* total_rate, uint32_t* video_rate, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index 175a94be80..392b91ddcf 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -392,6 +392,11 @@ bool ModuleRtpRtcpImpl::SendingMedia() const { return rtp_sender_ ? rtp_sender_->SendingMedia() : false; } +void ModuleRtpRtcpImpl::SetAsPartOfAllocation(bool part_of_allocation) { + RTC_CHECK(rtp_sender_); + rtp_sender_->SetAsPartOfAllocation(part_of_allocation); +} + bool ModuleRtpRtcpImpl::SendOutgoingData( FrameType frame_type, int8_t payload_type, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 604414adf2..9c2b085840 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -115,6 +115,8 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { bool SendingMedia() const override; + void SetAsPartOfAllocation(bool part_of_allocation) override; + // Used by the codec module to deliver a video or audio frame for // packetization. bool SendOutgoingData(FrameType frame_type, diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index c79f33d089..fa46e74704 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -132,7 +132,8 @@ RTPSender::RTPSender( transport_feedback_observer_(transport_feedback_observer), last_capture_time_ms_sent_(0), transport_(transport), - sending_media_(true), // Default to sending media. + sending_media_(true), // Default to sending media. + force_part_of_allocation_(false), max_packet_size_(IP_PACKET_SIZE - 28), // Default is IP-v4/UDP. last_payload_type_(-1), payload_type_map_(), @@ -624,6 +625,9 @@ size_t RTPSender::SendPadData(size_t bytes, rtc::CritScope lock(&send_critsect_); has_transport_seq_num = UpdateTransportSequenceNumber(&padding_packet, &options.packet_id); + options.included_in_allocation = + has_transport_seq_num || force_part_of_allocation_; + options.included_in_feedback = has_transport_seq_num; } padding_packet.SetPadding(padding_bytes_in_packet, &random_); if (has_transport_seq_num) { @@ -846,6 +850,9 @@ bool RTPSender::PrepareAndSendPacket(std::unique_ptr packet, rtc::CritScope lock(&send_critsect_); has_transport_seq_num = UpdateTransportSequenceNumber(packet_to_send, &options.packet_id); + options.included_in_allocation = + has_transport_seq_num || force_part_of_allocation_; + options.included_in_feedback = has_transport_seq_num; } if (has_transport_seq_num) { AddPacketToTransportFeedback(options.packet_id, *packet_to_send, @@ -986,6 +993,9 @@ bool RTPSender::SendToNetwork(std::unique_ptr packet, rtc::CritScope lock(&send_critsect_); has_transport_seq_num = UpdateTransportSequenceNumber(packet.get(), &options.packet_id); + options.included_in_allocation = + has_transport_seq_num || force_part_of_allocation_; + options.included_in_feedback = has_transport_seq_num; } if (has_transport_seq_num) { AddPacketToTransportFeedback(options.packet_id, *packet.get(), @@ -1224,6 +1234,11 @@ bool RTPSender::SendingMedia() const { return sending_media_; } +void RTPSender::SetAsPartOfAllocation(bool part_of_allocation) { + rtc::CritScope lock(&send_critsect_); + force_part_of_allocation_ = part_of_allocation; +} + void RTPSender::SetTimestampOffset(uint32_t timestamp) { rtc::CritScope lock(&send_critsect_); timestamp_offset_ = timestamp; diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 1adf6fe8c7..e410f97a71 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -86,6 +86,8 @@ class RTPSender { void SetSendingMediaStatus(bool enabled); bool SendingMedia() const; + void SetAsPartOfAllocation(bool part_of_allocation); + void GetDataCounters(StreamDataCounters* rtp_stats, StreamDataCounters* rtx_stats) const; @@ -277,7 +279,7 @@ class RTPSender { Transport* transport_; bool sending_media_ RTC_GUARDED_BY(send_critsect_); - + bool force_part_of_allocation_ RTC_GUARDED_BY(send_critsect_); size_t max_packet_size_; int8_t last_payload_type_ RTC_GUARDED_BY(send_critsect_); diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc index 86b92da2e0..1b7e99298b 100644 --- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc @@ -479,6 +479,7 @@ TEST_P(RtpSenderTestWithoutPacer, SendsPacketsWithTransportSequenceNumber) { ASSERT_TRUE(packet.GetExtension(&transport_seq_no)); EXPECT_EQ(kTransportSequenceNumber, transport_seq_no); EXPECT_EQ(transport_.last_options_.packet_id, transport_seq_no); + EXPECT_TRUE(transport_.last_options_.included_in_allocation); } TEST_P(RtpSenderTestWithoutPacer, PacketOptionsNoRetransmission) { @@ -493,8 +494,45 @@ TEST_P(RtpSenderTestWithoutPacer, PacketOptionsNoRetransmission) { EXPECT_FALSE(transport_.last_options_.is_retransmit); } -TEST_P(RtpSenderTestWithoutPacer, NoAllocationIfNotRegistered) { +TEST_P(RtpSenderTestWithoutPacer, + SetsIncludedInFeedbackWhenTransportSequenceNumberExtensionIsRegistered) { + SetUpRtpSender(false, false); + rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber, + kTransportSequenceNumberExtensionId); + EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) + .WillOnce(testing::Return(kTransportSequenceNumber)); + EXPECT_CALL(send_packet_observer_, OnSendPacket).Times(1); SendGenericPayload(); + EXPECT_TRUE(transport_.last_options_.included_in_feedback); +} + +TEST_P( + RtpSenderTestWithoutPacer, + SetsIncludedInAllocationWhenTransportSequenceNumberExtensionIsRegistered) { + SetUpRtpSender(false, false); + rtp_sender_->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber, + kTransportSequenceNumberExtensionId); + EXPECT_CALL(seq_num_allocator_, AllocateSequenceNumber()) + .WillOnce(testing::Return(kTransportSequenceNumber)); + EXPECT_CALL(send_packet_observer_, OnSendPacket).Times(1); + SendGenericPayload(); + EXPECT_TRUE(transport_.last_options_.included_in_allocation); +} + +TEST_P(RtpSenderTestWithoutPacer, + SetsIncludedInAllocationWhenForcedAsPartOfAllocation) { + SetUpRtpSender(false, false); + rtp_sender_->SetAsPartOfAllocation(true); + SendGenericPayload(); + EXPECT_FALSE(transport_.last_options_.included_in_feedback); + EXPECT_TRUE(transport_.last_options_.included_in_allocation); +} + +TEST_P(RtpSenderTestWithoutPacer, DoesnSetIncludedInAllocationByDefault) { + SetUpRtpSender(false, false); + SendGenericPayload(); + EXPECT_FALSE(transport_.last_options_.included_in_feedback); + EXPECT_FALSE(transport_.last_options_.included_in_allocation); } TEST_P(RtpSenderTestWithoutPacer, OnSendSideDelayUpdated) {