diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc index 5475ef3f1d..1ed6298ff1 100644 --- a/webrtc/modules/pacing/bitrate_prober.cc +++ b/webrtc/modules/pacing/bitrate_prober.cc @@ -11,6 +11,7 @@ #include "webrtc/modules/pacing/bitrate_prober.h" #include +#include #include #include @@ -107,7 +108,11 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) { time_until_probe_ms = 0; } } - return time_until_probe_ms; + return std::max(time_until_probe_ms, 0); +} + +size_t BitrateProber::RecommendedPacketSize() const { + return packet_size_last_send_; } void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) { diff --git a/webrtc/modules/pacing/bitrate_prober.h b/webrtc/modules/pacing/bitrate_prober.h index 04a858058f..b3f52afeb6 100644 --- a/webrtc/modules/pacing/bitrate_prober.h +++ b/webrtc/modules/pacing/bitrate_prober.h @@ -38,6 +38,10 @@ class BitrateProber { // get accurate probing. int TimeUntilNextProbe(int64_t now_ms); + // Returns the number of bytes that the prober recommends for the next probe + // packet. + size_t RecommendedPacketSize() const; + // Called to report to the prober that a packet has been sent, which helps the // prober know when to move to the next packet in a probe. void PacketSent(int64_t now_ms, size_t packet_size); diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h index 645999d507..730d3b7028 100644 --- a/webrtc/modules/pacing/include/paced_sender.h +++ b/webrtc/modules/pacing/include/paced_sender.h @@ -126,6 +126,9 @@ class PacedSender : public Module { // Process any pending packets in the queue(s). int32_t Process() override; + protected: + virtual bool ProbingExperimentIsEnabled() const; + private: // Updates the number of bytes that can be sent for the next time interval. void UpdateBytesPerInterval(int64_t delta_time_in_ms) diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc index 6186f96c3f..f25134156b 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -193,7 +193,9 @@ class IntervalBudget { -500 * target_rate_kbps_ / 8); } - int bytes_remaining() const { return bytes_remaining_; } + size_t bytes_remaining() const { + return static_cast(std::max(0, bytes_remaining_)); + } int target_rate_kbps() const { return target_rate_kbps_; } @@ -334,7 +336,7 @@ int32_t PacedSender::Process() { UpdateBytesPerInterval(delta_time_ms); } while (!packets_->Empty()) { - if (media_budget_->bytes_remaining() <= 0 && !prober_->IsProbing()) { + if (media_budget_->bytes_remaining() == 0 && !prober_->IsProbing()) { return 0; } @@ -355,10 +357,14 @@ int32_t PacedSender::Process() { } } - int padding_needed = padding_budget_->bytes_remaining(); - if (padding_needed > 0) { + size_t padding_needed; + if (prober_->IsProbing() && ProbingExperimentIsEnabled()) + padding_needed = prober_->RecommendedPacketSize(); + else + padding_needed = padding_budget_->bytes_remaining(); + + if (padding_needed > 0) SendPadding(static_cast(padding_needed)); - } } return 0; } @@ -386,13 +392,20 @@ void PacedSender::SendPadding(size_t padding_needed) { size_t bytes_sent = callback_->TimeToSendPadding(padding_needed); critsect_->Enter(); - // Update padding bytes sent. - media_budget_->UseBudget(bytes_sent); - padding_budget_->UseBudget(bytes_sent); + if (bytes_sent > 0) { + prober_->PacketSent(clock_->TimeInMilliseconds(), bytes_sent); + media_budget_->UseBudget(bytes_sent); + padding_budget_->UseBudget(bytes_sent); + } } void PacedSender::UpdateBytesPerInterval(int64_t delta_time_ms) { media_budget_->IncreaseBudget(delta_time_ms); padding_budget_->IncreaseBudget(delta_time_ms); } + +bool PacedSender::ProbingExperimentIsEnabled() const { + return webrtc::field_trial::FindFullName("WebRTC-BitrateProbing") == + "Enabled"; +} } // namespace webrtc diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc index e82a49e7cc..0730062088 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -71,22 +71,26 @@ class PacedSenderProbing : public PacedSender::Callback { uint16_t sequence_number, int64_t capture_time_ms, bool retransmission) { + ExpectAndCountPacket(); + return true; + } + + size_t TimeToSendPadding(size_t bytes) { + ExpectAndCountPacket(); + return bytes; + } + + void ExpectAndCountPacket() { ++packets_sent_; EXPECT_FALSE(expected_deltas_.empty()); if (expected_deltas_.empty()) - return false; + return; int64_t now_ms = clock_->TimeInMilliseconds(); if (prev_packet_time_ms_ >= 0) { EXPECT_EQ(expected_deltas_.front(), now_ms - prev_packet_time_ms_); expected_deltas_.pop_front(); } prev_packet_time_ms_ = now_ms; - return true; - } - - size_t TimeToSendPadding(size_t bytes) { - EXPECT_TRUE(false); - return bytes; } int packets_sent() const { return packets_sent_; } @@ -714,8 +718,6 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) { std::list expected_deltas_list(expected_deltas, expected_deltas + kNumPackets - 1); PacedSenderProbing callback(expected_deltas_list, &clock_); - // Probing implicitly enabled by creating a new PacedSender which defaults to - // probing on. send_bucket_.reset( new PacedSender(&clock_, &callback, @@ -741,6 +743,58 @@ TEST_F(PacedSenderTest, ProbingWithInitialFrame) { } } +class ProbingPacedSender : public PacedSender { + public: + ProbingPacedSender(Clock* clock, + Callback* callback, + int bitrate_kbps, + int max_bitrate_kbps, + int min_bitrate_kbps) + : PacedSender(clock, + callback, + bitrate_kbps, + max_bitrate_kbps, + min_bitrate_kbps) {} + + bool ProbingExperimentIsEnabled() const override { return true; } +}; + +TEST_F(PacedSenderTest, ProbingWithTooSmallInitialFrame) { + const int kNumPackets = 11; + const int kNumDeltas = kNumPackets - 1; + const size_t kPacketSize = 1200; + const int kInitialBitrateKbps = 300; + uint32_t ssrc = 12346; + uint16_t sequence_number = 1234; + const int expected_deltas[kNumDeltas] = {10, 10, 10, 10, 10, 5, 5, 5, 5, 5}; + std::list expected_deltas_list(expected_deltas, + expected_deltas + kNumPackets - 1); + PacedSenderProbing callback(expected_deltas_list, &clock_); + send_bucket_.reset( + new ProbingPacedSender(&clock_, &callback, kInitialBitrateKbps, + kPaceMultiplier * kInitialBitrateKbps, 0)); + + for (int i = 0; i < kNumPackets - 5; ++i) { + EXPECT_FALSE(send_bucket_->SendPacket( + PacedSender::kNormalPriority, ssrc, sequence_number++, + clock_.TimeInMilliseconds(), kPacketSize, false)); + } + while (callback.packets_sent() < kNumPackets) { + int time_until_process = send_bucket_->TimeUntilNextProcess(); + if (time_until_process <= 0) { + send_bucket_->Process(); + } else { + clock_.AdvanceTimeMilliseconds(time_until_process); + } + } + + // Process one more time and make sure we don't send any more probes. + int time_until_process = send_bucket_->TimeUntilNextProcess(); + clock_.AdvanceTimeMilliseconds(time_until_process); + send_bucket_->Process(); + EXPECT_EQ(kNumPackets, callback.packets_sent()); +} + TEST_F(PacedSenderTest, PriorityInversion) { uint32_t ssrc = 12346; uint16_t sequence_number = 1234; diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index c260567102..1b4e990cd1 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -403,7 +403,8 @@ TEST_F(EndToEndTest, ReceivesAndRetransmitsNack) { if (sent_rtp_packets_ % kPacketsBetweenLossBursts == 0) packets_left_to_drop_ = kLossBurstSize; - if (packets_left_to_drop_ > 0) { + // Never drop padding packets as those won't be retransmitted. + if (packets_left_to_drop_ > 0 && header.paddingLength == 0) { --packets_left_to_drop_; dropped_packets_.insert(header.sequenceNumber); return DROP_PACKET; @@ -463,11 +464,15 @@ TEST_F(EndToEndTest, CanReceiveFec) { RTPHeader header; EXPECT_TRUE(parser_->Parse(packet, length, &header)); - EXPECT_EQ(kRedPayloadType, header.payloadType); - int encapsulated_payload_type = - static_cast(packet[header.headerLength]); - if (encapsulated_payload_type != kFakeSendPayloadType) - EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type); + int encapsulated_payload_type = -1; + if (header.payloadType == kRedPayloadType) { + encapsulated_payload_type = + static_cast(packet[header.headerLength]); + if (encapsulated_payload_type != kFakeSendPayloadType) + EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type); + } else { + EXPECT_EQ(kFakeSendPayloadType, header.payloadType); + } if (protected_sequence_numbers_.count(header.sequenceNumber) != 0) { // Retransmitted packet, should not count. @@ -571,12 +576,16 @@ void EndToEndTest::TestReceivedFecPacketsNotNacked( Action OnSendRtp(const uint8_t* packet, size_t length) override { RTPHeader header; EXPECT_TRUE(parser_->Parse(packet, length, &header)); - EXPECT_EQ(kRedPayloadType, header.payloadType); - int encapsulated_payload_type = - static_cast(packet[header.headerLength]); - if (encapsulated_payload_type != kFakeSendPayloadType) - EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type); + int encapsulated_payload_type = -1; + if (header.payloadType == kRedPayloadType) { + encapsulated_payload_type = + static_cast(packet[header.headerLength]); + if (encapsulated_payload_type != kFakeSendPayloadType) + EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type); + } else { + EXPECT_EQ(kFakeSendPayloadType, header.payloadType); + } if (has_last_sequence_number_ && !IsNewerSequenceNumber(header.sequenceNumber, diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index eb79e1b48c..547c811797 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -306,14 +306,22 @@ TEST_F(VideoSendStreamTest, SupportsFec) { EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpRr)); } - EXPECT_EQ(kRedPayloadType, header.payloadType); - - uint8_t encapsulated_payload_type = packet[header.headerLength]; - - if (encapsulated_payload_type == kUlpfecPayloadType) { - received_fec_ = true; + int encapsulated_payload_type = -1; + if (header.payloadType == kRedPayloadType) { + encapsulated_payload_type = + static_cast(packet[header.headerLength]); + if (encapsulated_payload_type != kFakeSendPayloadType) + EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type); } else { - received_media_ = true; + EXPECT_EQ(kFakeSendPayloadType, header.payloadType); + } + + if (encapsulated_payload_type != -1) { + if (encapsulated_payload_type == kUlpfecPayloadType) { + received_fec_ = true; + } else { + received_media_ = true; + } } if (received_media_ && received_fec_)