From 482b3ef2ace3615a66696cd214f3c2d22bd477e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Tue, 8 Jan 2019 16:19:11 +0100 Subject: [PATCH] Account for packetization overhead when setting target bitrate. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That is, the payload packetization overhead (eg. vp8 payload header), not the RTP headers, extensions, etc. The encoder and pacer both look at payload rate, but are currently not aware of the bytes that are added in between them. Bug: webrtc:10155 Change-Id: I4cdb04849d762360374d47a496983c8c6df191d2 Reviewed-on: https://webrtc-review.googlesource.com/c/115410 Commit-Queue: Erik Språng Reviewed-by: Niels Moller Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/master@{#26163} --- call/rtp_video_sender.cc | 21 +++++++++++++++++ call/rtp_video_sender.h | 1 + modules/rtp_rtcp/include/rtp_rtcp.h | 7 +++++- modules/rtp_rtcp/mocks/mock_rtp_rtcp.h | 1 + modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 4 ++++ modules/rtp_rtcp/source/rtp_rtcp_impl.h | 1 + modules/rtp_rtcp/source/rtp_sender.cc | 4 ++++ modules/rtp_rtcp/source/rtp_sender.h | 1 + modules/rtp_rtcp/source/rtp_sender_video.cc | 25 +++++++++++++++++++++ modules/rtp_rtcp/source/rtp_sender_video.h | 2 ++ video/picture_id_tests.cc | 11 ++++----- 11 files changed, 72 insertions(+), 6 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 1bb2439e96..a3804d7b5f 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -480,6 +480,16 @@ bool RtpVideoSender::NackEnabled() const { return nack_enabled; } +uint32_t RtpVideoSender::GetPacketizationOverheadRate() const { + uint32_t packetization_overhead_bps = 0; + for (auto& rtp_rtcp : rtp_modules_) { + if (rtp_rtcp->SendingMedia()) { + packetization_overhead_bps += rtp_rtcp->PacketizationOverheadBps(); + } + } + return packetization_overhead_bps; +} + void RtpVideoSender::DeliverRtcp(const uint8_t* packet, size_t length) { // Runs on a network thread. for (auto& rtp_rtcp : rtp_modules_) @@ -622,6 +632,17 @@ void RtpVideoSender::OnBitrateUpdated(uint32_t bitrate_bps, // protection overhead. encoder_target_rate_bps_ = fec_controller_->UpdateFecRates( payload_bitrate_bps, framerate, fraction_loss, loss_mask_vector_, rtt); + + // Subtract packetization overhead from the encoder target. If rate is really + // low, cap the overhead at 50%. Since packetization is measured over an + // averaging window, it might intermittently be higher than encoder target + // (eg encoder pause event), so cap it to target. + const uint32_t packetization_rate_bps = + std::min(GetPacketizationOverheadRate(), encoder_target_rate_bps_); + encoder_target_rate_bps_ = + std::max(encoder_target_rate_bps_ - packetization_rate_bps, + encoder_target_rate_bps_ / 2); + loss_mask_vector_.clear(); uint32_t encoder_overhead_rate_bps = diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index dac43b1918..2eab569678 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -128,6 +128,7 @@ class RtpVideoSender : public RtpVideoSenderInterface, void ConfigureRids(const RtpConfig& rtp_config); bool FecEnabled() const; bool NackEnabled() const; + uint32_t GetPacketizationOverheadRate() const; const bool send_side_bwe_with_overhead_; diff --git a/modules/rtp_rtcp/include/rtp_rtcp.h b/modules/rtp_rtcp/include/rtp_rtcp.h index a927e41770..f6b0c0f68f 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/modules/rtp_rtcp/include/rtp_rtcp.h @@ -238,12 +238,17 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface { // bitrate estimate since the stream participates in the bitrate allocation. virtual void SetAsPartOfAllocation(bool part_of_allocation) = 0; - // Returns current bitrate in Kbit/s. + // Fetches the current send bitrates in bits/s. virtual void BitrateSent(uint32_t* total_rate, uint32_t* video_rate, uint32_t* fec_rate, uint32_t* nack_rate) const = 0; + // Returns the current packetization overhead rate, in bps. Note that this is + // the payload overhead, eg the VP8 payload headers, not the RTP headers + // or extension/ + virtual uint32_t PacketizationOverheadBps() const = 0; + // Used by the codec module to deliver a video or audio frame for // packetization. // |frame_type| - type of frame to send diff --git a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h index b9a5d37408..a76f65a2de 100644 --- a/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h +++ b/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h @@ -85,6 +85,7 @@ class MockRtpRtcp : public RtpRtcp { uint32_t* video_rate, uint32_t* fec_rate, uint32_t* nack_rate)); + MOCK_CONST_METHOD0(PacketizationOverheadBps, uint32_t()); MOCK_CONST_METHOD1(EstimatedReceiveBandwidth, int(uint32_t* available_bandwidth)); MOCK_METHOD9(SendOutgoingData, diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index e4fbe71d57..69aed2d071 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -826,6 +826,10 @@ void ModuleRtpRtcpImpl::BitrateSent(uint32_t* total_rate, *nack_rate = rtp_sender_->NackOverheadRate(); } +uint32_t ModuleRtpRtcpImpl::PacketizationOverheadBps() const { + return rtp_sender_->PacketizationOverheadBps(); +} + void ModuleRtpRtcpImpl::OnRequestSendReport() { SendRTCP(kRtcpSr); } diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 2327d6406a..78fa70ac67 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -299,6 +299,7 @@ class ModuleRtpRtcpImpl : public RtpRtcp, public RTCPReceiver::ModuleRtpRtcp { uint32_t* video_rate, uint32_t* fec_rate, uint32_t* nackRate) const override; + uint32_t PacketizationOverheadBps() const override; void RegisterSendChannelRtpStatisticsCallback( StreamDataCountersCallback* callback) override; diff --git a/modules/rtp_rtcp/source/rtp_sender.cc b/modules/rtp_rtcp/source/rtp_sender.cc index cbf6b65f72..87f1c00a2b 100644 --- a/modules/rtp_rtcp/source/rtp_sender.cc +++ b/modules/rtp_rtcp/source/rtp_sender.cc @@ -246,6 +246,10 @@ uint32_t RTPSender::NackOverheadRate() const { return nack_bitrate_sent_.Rate(clock_->TimeInMilliseconds()).value_or(0); } +uint32_t RTPSender::PacketizationOverheadBps() const { + return video_ ? video_->PacketizationOverheadBps() : 0; +} + void RTPSender::SetExtmapAllowMixed(bool extmap_allow_mixed) { rtc::CritScope lock(&send_critsect_); rtp_header_extension_map_.SetExtmapAllowMixed(extmap_allow_mixed); diff --git a/modules/rtp_rtcp/source/rtp_sender.h b/modules/rtp_rtcp/source/rtp_sender.h index 254a7e5f1a..d3c6599da9 100644 --- a/modules/rtp_rtcp/source/rtp_sender.h +++ b/modules/rtp_rtcp/source/rtp_sender.h @@ -78,6 +78,7 @@ class RTPSender { uint32_t VideoBitrateSent() const; uint32_t FecOverheadRate() const; uint32_t NackOverheadRate() const; + uint32_t PacketizationOverheadBps() const; int32_t RegisterPayload(absl::string_view payload_name, const int8_t payload_type, diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc index ec65a452aa..2f592b3d27 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -170,6 +170,7 @@ RTPSenderVideo::RTPSenderVideo(Clock* clock, key_fec_params_{0, 1, kFecMaskRandom}, fec_bitrate_(1000, RateStatistics::kBpsScale), video_bitrate_(1000, RateStatistics::kBpsScale), + packetization_overhead_bitrate_(1000, RateStatistics::kBpsScale), frame_encryptor_(frame_encryptor), require_frame_encryption_(require_frame_encryption), generic_descriptor_auth_experiment_( @@ -546,6 +547,17 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, expected_retransmission_time_ms); size_t num_packets = packetizer->NumPackets(); + size_t unpacketized_payload_size; + if (fragmentation && fragmentation->fragmentationVectorSize > 0) { + unpacketized_payload_size = 0; + for (uint16_t i = 0; i < fragmentation->fragmentationVectorSize; ++i) { + unpacketized_payload_size += fragmentation->fragmentationLength[i]; + } + } else { + unpacketized_payload_size = payload_size; + } + size_t packetized_payload_size = 0; + if (num_packets == 0) return false; @@ -576,6 +588,7 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, RTC_DCHECK_LE(packet->payload_size(), expected_payload_capacity); if (!rtp_sender_->AssignSequenceNumber(packet.get())) return false; + packetized_payload_size += packet->payload_size(); // No FEC protection for upper temporal layers, if used. bool protect_packet = temporal_id == 0 || temporal_id == kNoTemporalIdx; @@ -615,6 +628,12 @@ bool RTPSenderVideo::SendVideo(enum VideoCodecType video_type, } } + rtc::CritScope cs(&stats_crit_); + RTC_DCHECK_GE(packetized_payload_size, unpacketized_payload_size); + packetization_overhead_bitrate_.Update( + packetized_payload_size - unpacketized_payload_size, + clock_->TimeInMilliseconds()); + TRACE_EVENT_ASYNC_END1("webrtc", "Video", capture_time_ms, "timestamp", rtp_timestamp); return true; @@ -630,6 +649,12 @@ uint32_t RTPSenderVideo::FecOverheadRate() const { return fec_bitrate_.Rate(clock_->TimeInMilliseconds()).value_or(0); } +uint32_t RTPSenderVideo::PacketizationOverheadBps() const { + rtc::CritScope cs(&stats_crit_); + return packetization_overhead_bitrate_.Rate(clock_->TimeInMilliseconds()) + .value_or(0); +} + int RTPSenderVideo::SelectiveRetransmissions() const { rtc::CritScope cs(&crit_); return retransmission_settings_; diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h index 3e6dfd58cd..5821b12109 100644 --- a/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/modules/rtp_rtcp/source/rtp_sender_video.h @@ -77,6 +77,7 @@ class RTPSenderVideo { uint32_t VideoBitrateSent() const; uint32_t FecOverheadRate() const; + uint32_t PacketizationOverheadBps() const; int SelectiveRetransmissions() const; void SetSelectiveRetransmissions(uint8_t settings); @@ -159,6 +160,7 @@ class RTPSenderVideo { RateStatistics fec_bitrate_ RTC_GUARDED_BY(stats_crit_); // Bitrate used for video payload and RTP headers. RateStatistics video_bitrate_ RTC_GUARDED_BY(stats_crit_); + RateStatistics packetization_overhead_bitrate_ RTC_GUARDED_BY(stats_crit_); std::map frame_stats_by_temporal_layer_ RTC_GUARDED_BY(stats_crit_); diff --git a/video/picture_id_tests.cc b/video/picture_id_tests.cc index 4d08373da1..98c10f23ee 100644 --- a/video/picture_id_tests.cc +++ b/video/picture_id_tests.cc @@ -259,11 +259,12 @@ class VideoStreamFactory std::vector streams = test::CreateVideoStreams(width, height, encoder_config); - // Use the same total bitrates when sending a single stream to avoid - // lowering the bitrate estimate and requiring a subsequent rampup. - const int encoder_stream_bps = - kEncoderBitrateBps / - rtc::checked_cast(encoder_config.number_of_streams); + // Always divide the same total bitrate across all streams so that sending a + // single stream avoids lowering the bitrate estimate and requiring a + // subsequent rampup. Also reduce the target by 10% to account for overhead + // that might sometimes otherwise cause streams to not be enabled. + const int encoder_stream_bps = rtc::checked_cast( + 0.9 * (kEncoderBitrateBps / encoder_config.number_of_streams)); for (size_t i = 0; i < encoder_config.number_of_streams; ++i) { streams[i].min_bitrate_bps = encoder_stream_bps;