From 9a12ee5d37ced49a3b84357ac7ae8a9ddd8d01a6 Mon Sep 17 00:00:00 2001 From: Jakob Ivarsson Date: Thu, 26 Nov 2020 16:04:18 +0100 Subject: [PATCH] Use frame rate in video overhead calculation. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assume bitrate is evenly distributed between frames. This is wrong for uneven frame sizes and will underestimate the overhead for simulcast. However, it will be more correct than the current calculation, especially for low bitrates when each frame is smaller than one packet. This is also when overhead matters more since it is a larger fraction of the total bitrate. It is also unclear what will happen when using FEC. Bug: b/166341943 Change-Id: I247b9d0fc7a8ad5daa9b577f55ec16c56efa34c3 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195221 Reviewed-by: Erik Språng Commit-Queue: Jakob Ivarsson Cr-Commit-Position: refs/heads/master@{#32725} --- call/rtp_video_sender.cc | 34 ++++++++++------ call/rtp_video_sender.h | 5 +++ call/rtp_video_sender_unittest.cc | 67 ++++++++++++++++++++++--------- 3 files changed, 75 insertions(+), 31 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 9dad424c86..041427a02e 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -290,15 +290,6 @@ std::vector CreateRtpStreamSenders( return rtp_streams; } -DataRate CalculateOverheadRate(DataRate data_rate, - DataSize packet_size, - DataSize overhead_per_packet) { - Frequency packet_rate = data_rate / packet_size; - // TOSO(srte): We should not need to round to nearest whole packet per second - // rate here. - return packet_rate.RoundUpTo(Frequency::Hertz(1)) * overhead_per_packet; -} - absl::optional GetVideoCodecType(const RtpConfig& config) { if (config.raw_payload) { return absl::nullopt; @@ -330,6 +321,9 @@ RtpVideoSender::RtpVideoSender( : send_side_bwe_with_overhead_(!absl::StartsWith( field_trials_.Lookup("WebRTC-SendSideBwe-WithOverhead"), "Disabled")), + use_frame_rate_for_overhead_(absl::StartsWith( + field_trials_.Lookup("WebRTC-Video-UseFrameRateForOverhead"), + "Enabled")), has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)), active_(false), module_process_thread_(nullptr), @@ -766,8 +760,9 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_); uint32_t payload_bitrate_bps = update.target_bitrate.bps(); if (send_side_bwe_with_overhead_ && has_packet_feedback_) { - DataRate overhead_rate = CalculateOverheadRate( - update.target_bitrate, max_total_packet_size, packet_overhead); + DataRate overhead_rate = + CalculateOverheadRate(update.target_bitrate, max_total_packet_size, + packet_overhead, Frequency::Hertz(framerate)); // TODO(srte): We probably should not accept 0 payload bitrate here. payload_bitrate_bps = rtc::saturated_cast(payload_bitrate_bps - overhead_rate.bps()); @@ -806,7 +801,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update, DataRate encoder_overhead_rate = CalculateOverheadRate( DataRate::BitsPerSec(encoder_target_rate_bps_), max_total_packet_size - DataSize::Bytes(overhead_bytes_per_packet), - packet_overhead); + packet_overhead, Frequency::Hertz(framerate)); encoder_overhead_rate_bps = std::min( encoder_overhead_rate.bps(), update.target_bitrate.bps() - encoder_target_rate_bps_); @@ -927,4 +922,19 @@ void RtpVideoSender::SetEncodingData(size_t width, fec_controller_->SetEncodingData(width, height, num_temporal_layers, rtp_config_.max_packet_size); } + +DataRate RtpVideoSender::CalculateOverheadRate(DataRate data_rate, + DataSize packet_size, + DataSize overhead_per_packet, + Frequency framerate) const { + Frequency packet_rate = data_rate / packet_size; + if (use_frame_rate_for_overhead_) { + framerate = std::max(framerate, Frequency::Hertz(1)); + DataSize frame_size = data_rate / framerate; + int packets_per_frame = ceil(frame_size / packet_size); + packet_rate = packets_per_frame * framerate; + } + return packet_rate.RoundUpTo(Frequency::Hertz(1)) * overhead_per_packet; +} + } // namespace webrtc diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 49fd3cc0d2..a8fb0ab59c 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -169,9 +169,14 @@ class RtpVideoSender : public RtpVideoSenderInterface, void ConfigureRids(); bool NackEnabled() const; uint32_t GetPacketizationOverheadRate() const; + DataRate CalculateOverheadRate(DataRate data_rate, + DataSize packet_size, + DataSize overhead_per_packet, + Frequency framerate) const; const FieldTrialBasedConfig field_trials_; const bool send_side_bwe_with_overhead_; + const bool use_frame_rate_for_overhead_; const bool has_packet_feedback_; // TODO(holmer): Remove mutex_ once RtpVideoSender runs on the diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index af0b5032f3..5ea403475e 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -203,6 +203,14 @@ class RtpVideoSenderTestFixture { RateLimiter retransmission_rate_limiter_; std::unique_ptr router_; }; + +BitrateAllocationUpdate CreateBitrateAllocationUpdate(int target_bitrate_bps) { + BitrateAllocationUpdate update; + update.target_bitrate = DataRate::BitsPerSec(target_bitrate_bps); + update.round_trip_time = TimeDelta::Zero(); + return update; +} + } // namespace TEST(RtpVideoSenderTest, SendOnOneModule) { @@ -768,26 +776,10 @@ TEST(RtpVideoSenderTest, SupportsStoppingUsingDependencyDescriptor) { sent_packets.back().HasExtension()); } -TEST(RtpVideoSenderTest, CanSetZeroBitrateWithOverhead) { - test::ScopedFieldTrials trials("WebRTC-SendSideBwe-WithOverhead/Enabled/"); +TEST(RtpVideoSenderTest, CanSetZeroBitrate) { RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); - BitrateAllocationUpdate update; - update.target_bitrate = DataRate::Zero(); - update.packet_loss_ratio = 0; - update.round_trip_time = TimeDelta::Zero(); - - test.router()->OnBitrateUpdated(update, /*framerate*/ 0); -} - -TEST(RtpVideoSenderTest, CanSetZeroBitrateWithoutOverhead) { - RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); - - BitrateAllocationUpdate update; - update.target_bitrate = DataRate::Zero(); - update.packet_loss_ratio = 0; - update.round_trip_time = TimeDelta::Zero(); - - test.router()->OnBitrateUpdated(update, /*framerate*/ 0); + test.router()->OnBitrateUpdated(CreateBitrateAllocationUpdate(0), + /*framerate*/ 0); } TEST(RtpVideoSenderTest, SimulcastSenderRegistersFrameTransformers) { @@ -802,4 +794,41 @@ TEST(RtpVideoSenderTest, SimulcastSenderRegistersFrameTransformers) { EXPECT_CALL(*transformer, UnregisterTransformedFrameSinkCallback(kSsrc1)); EXPECT_CALL(*transformer, UnregisterTransformedFrameSinkCallback(kSsrc2)); } + +TEST(RtpVideoSenderTest, OverheadIsSubtractedFromTargetBitrate) { + test::ScopedFieldTrials field_trials( + "WebRTC-Video-UseFrameRateForOverhead/Enabled/"); + + // TODO(jakobi): RTP header size should not be hard coded. + constexpr uint32_t kRtpHeaderSizeBytes = 20; + constexpr uint32_t kTransportPacketOverheadBytes = 40; + constexpr uint32_t kOverheadPerPacketBytes = + kRtpHeaderSizeBytes + kTransportPacketOverheadBytes; + RtpVideoSenderTestFixture test({kSsrc1}, {kRtxSsrc1}, kPayloadType, {}); + test.router()->OnTransportOverheadChanged(kTransportPacketOverheadBytes); + test.router()->SetActive(true); + + { + test.router()->OnBitrateUpdated(CreateBitrateAllocationUpdate(300000), + /*framerate*/ 15); + // 1 packet per frame. + EXPECT_EQ(test.router()->GetPayloadBitrateBps(), + 300000 - kOverheadPerPacketBytes * 8 * 30); + } + { + test.router()->OnBitrateUpdated(CreateBitrateAllocationUpdate(150000), + /*framerate*/ 15); + // 1 packet per frame. + EXPECT_EQ(test.router()->GetPayloadBitrateBps(), + 150000 - kOverheadPerPacketBytes * 8 * 15); + } + { + test.router()->OnBitrateUpdated(CreateBitrateAllocationUpdate(1000000), + /*framerate*/ 30); + // 3 packets per frame. + EXPECT_EQ(test.router()->GetPayloadBitrateBps(), + 1000000 - kOverheadPerPacketBytes * 8 * 30 * 3); + } +} + } // namespace webrtc