diff --git a/call/BUILD.gn b/call/BUILD.gn index 484b772e49..8833fe9ff4 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -438,6 +438,7 @@ rtc_library("video_send_stream_api") { "../api:transport_api", "../api/adaptation:resource_adaptation_api", "../api/crypto:options", + "../api/units:data_rate", "../api/video:video_frame", "../api/video:video_rtp_headers", "../api/video:video_stream_encoder", diff --git a/call/video_send_stream.h b/call/video_send_stream.h index 3bf13c5e74..02f3569dd2 100644 --- a/call/video_send_stream.h +++ b/call/video_send_stream.h @@ -25,6 +25,7 @@ #include "api/rtp_parameters.h" #include "api/rtp_sender_interface.h" #include "api/scoped_refptr.h" +#include "api/units/data_rate.h" #include "api/video/video_content_type.h" #include "api/video/video_frame.h" #include "api/video/video_source_interface.h" @@ -95,6 +96,9 @@ class VideoSendStream { uint64_t total_encoded_bytes_target = 0; uint32_t huge_frames_sent = 0; std::optional scalability_mode; + // The target bitrate is what we tell the encoder to produce. What the + // encoder actually produces is the sum of encoded bytes. + std::optional target_bitrate; }; struct Stats { @@ -118,8 +122,11 @@ class VideoSendStream { uint32_t frames_dropped_by_rate_limiter = 0; uint32_t frames_dropped_by_congestion_window = 0; uint32_t frames_dropped_by_encoder = 0; - // Bitrate the encoder is currently configured to use due to bandwidth - // limitations. + // Metric only used by legacy getStats()'s BWE. + // - Similar to `StreamStats::target_bitrate` except this is for the whole + // stream as opposed to being per substream (per SSRC). + // - Unlike what you would expect, it is not equal to the sum of all + // substream targets and may sometimes over-report e.g. webrtc:392424845. int target_media_bitrate_bps = 0; // Bitrate the encoder is actually producing. int media_bitrate_bps = 0; diff --git a/media/BUILD.gn b/media/BUILD.gn index 19c6cd0b22..0564eb745b 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -327,6 +327,7 @@ rtc_source_set("media_channel") { "../api/task_queue:pending_task_safety_flag", "../api/transport:datagram_transport_interface", "../api/transport/rtp:rtp_source", + "../api/units:data_rate", "../api/units:time_delta", "../api/units:timestamp", "../api/video:recordable_encoded_frame", diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 55ec4ff2e9..c85d6a3c51 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -39,6 +39,7 @@ #include "api/rtp_sender_interface.h" #include "api/scoped_refptr.h" #include "api/transport/rtp/rtp_source.h" +#include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "api/video/recordable_encoded_frame.h" @@ -386,7 +387,7 @@ struct MediaSenderInfo { // https://w3c.github.io/webrtc-stats/#dom-rtcoutboundrtpstreamstats-nackcount uint32_t nacks_received = 0; // https://w3c.github.io/webrtc-stats/#dom-rtcoutboundrtpstreamstats-targetbitrate - std::optional target_bitrate; + std::optional target_bitrate; int packets_lost = 0; float fraction_lost = 0.0f; int64_t rtt_ms = 0; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index b3e25c6c5b..358f27d9a5 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2461,7 +2461,6 @@ WebRtcVideoSendChannel::WebRtcVideoSendStream::GetPerLayerVideoSenderInfos( common_info.quality_limitation_resolution_changes = stats.quality_limitation_resolution_changes; common_info.encoder_implementation_name = stats.encoder_implementation_name; - common_info.target_bitrate = stats.target_media_bitrate_bps; common_info.ssrc_groups = ssrc_groups_; common_info.frames = stats.frames; common_info.framerate_input = stats.input_frame_rate; @@ -2544,6 +2543,7 @@ WebRtcVideoSendChannel::WebRtcVideoSendStream::GetPerLayerVideoSenderInfos( info.total_encoded_bytes_target = stream_stats.total_encoded_bytes_target; info.huge_frames_sent = stream_stats.huge_frames_sent; info.scalability_mode = stream_stats.scalability_mode; + info.target_bitrate = stream_stats.target_bitrate; infos.push_back(info); } return infos; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index f8c237173f..82d38f8a1c 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -1807,7 +1807,10 @@ bool WebRtcVoiceSendChannel::GetStats(VoiceMediaSendInfo* info) { sinfo.packets_lost = stats.packets_lost; sinfo.fraction_lost = stats.fraction_lost; sinfo.nacks_received = stats.nacks_received; - sinfo.target_bitrate = stats.target_bitrate_bps; + sinfo.target_bitrate = stats.target_bitrate_bps > 0 + ? std::optional(webrtc::DataRate::BitsPerSec( + stats.target_bitrate_bps)) + : std::nullopt; sinfo.codec_name = stats.codec_name; sinfo.codec_payload_type = stats.codec_payload_type; sinfo.jitter_ms = stats.jitter_ms; diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 451590fced..6ff5dabbd6 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -739,9 +739,8 @@ CreateOutboundRTPStreamStatsFromVoiceSenderInfo( outbound_audio->transport_id = transport_id; outbound_audio->mid = mid; outbound_audio->kind = "audio"; - if (voice_sender_info.target_bitrate.has_value() && - *voice_sender_info.target_bitrate > 0) { - outbound_audio->target_bitrate = *voice_sender_info.target_bitrate; + if (voice_sender_info.target_bitrate.has_value()) { + outbound_audio->target_bitrate = voice_sender_info.target_bitrate->bps(); } if (voice_sender_info.codec_payload_type.has_value()) { auto codec_param_it = voice_media_info.send_codecs.find( @@ -791,9 +790,8 @@ CreateOutboundRTPStreamStatsFromVideoSenderInfo( static_cast(video_sender_info.plis_received); if (video_sender_info.qp_sum.has_value()) outbound_video->qp_sum = *video_sender_info.qp_sum; - if (video_sender_info.target_bitrate.has_value() && - *video_sender_info.target_bitrate > 0) { - outbound_video->target_bitrate = *video_sender_info.target_bitrate; + if (video_sender_info.target_bitrate.has_value()) { + outbound_video->target_bitrate = video_sender_info.target_bitrate->bps(); } outbound_video->frames_encoded = video_sender_info.frames_encoded; outbound_video->key_frames_encoded = video_sender_info.key_frames_encoded; diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index 1737f26cc6..6d9dab4726 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -44,6 +44,7 @@ #include "api/stats/rtcstats_objects.h" #include "api/test/rtc_error_matchers.h" #include "api/transport/enums.h" +#include "api/units/data_rate.h" #include "api/units/time_delta.h" #include "api/units/timestamp.h" #include "api/video/recordable_encoded_frame.h" @@ -2557,7 +2558,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRtpStreamStats_Audio) { voice_media_info.senders[0].header_and_padding_bytes_sent = 12; voice_media_info.senders[0].retransmitted_bytes_sent = 30; voice_media_info.senders[0].nacks_received = 31; - voice_media_info.senders[0].target_bitrate = 32000; + voice_media_info.senders[0].target_bitrate = DataRate::BitsPerSec(32'000); voice_media_info.senders[0].codec_payload_type = 42; voice_media_info.senders[0].active = true; diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index 8c97a27081..412ff472f5 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -1241,6 +1241,26 @@ void SendStatisticsProxy::OnBitrateAllocationUpdated( bw_limited_layers_ = allocation.is_bw_limited(); UpdateAdaptationStats(); + // Store target bitrates per substream stats. + for (auto& [ssrc, substream] : stats_.substreams) { + std::optional simulcast_index; + for (size_t i = 0; i < rtp_config_.ssrcs.size(); ++i) { + if (rtp_config_.ssrcs[i] == ssrc) { + simulcast_index = i; + break; + } + } + if (!simulcast_index.has_value()) { + substream.target_bitrate = std::nullopt; + continue; + } + substream.target_bitrate = + DataRate::BitsPerSec(allocation.GetSpatialLayerSum(*simulcast_index)); + if (substream.target_bitrate == DataRate::Zero()) { + substream.target_bitrate = std::nullopt; + } + } + if (spatial_layers != last_spatial_layer_use_) { // If the number of spatial layers has changed, the resolution change is // not due to quality limitations, it is because the configuration diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index 0d022c7b20..f2a757c0ff 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -1508,6 +1508,36 @@ TEST_F(SendStatisticsProxyTest, 0u, statistics_proxy_->GetStats().quality_limitation_resolution_changes); } +TEST_F(SendStatisticsProxyTest, OnBitrateAllocationUpdatedSetsTargetBitrates) { + // We only update target bitrates for substreams that exist and these are + // created lazily in various places... calling OnInactiveSsrc() is one way to + // ensure the stats are reported. + statistics_proxy_->OnInactiveSsrc(kFirstSsrc); + statistics_proxy_->OnInactiveSsrc(kSecondSsrc); + + // Update target bitrates! + VideoBitrateAllocation allocation; + allocation.SetBitrate(0, 0, 123); + allocation.SetBitrate(1, 0, 321); + statistics_proxy_->OnBitrateAllocationUpdated(VideoCodec(), allocation); + EXPECT_EQ(statistics_proxy_->GetStats().substreams[kFirstSsrc].target_bitrate, + DataRate::BitsPerSec(123)); + EXPECT_EQ( + statistics_proxy_->GetStats().substreams[kSecondSsrc].target_bitrate, + DataRate::BitsPerSec(321)); + + // 0 bitrate = no target. + allocation.SetBitrate(0, 0, 0); + allocation.SetBitrate(1, 0, 0); + statistics_proxy_->OnBitrateAllocationUpdated(VideoCodec(), allocation); + EXPECT_FALSE(statistics_proxy_->GetStats() + .substreams[kFirstSsrc] + .target_bitrate.has_value()); + EXPECT_FALSE(statistics_proxy_->GetStats() + .substreams[kSecondSsrc] + .target_bitrate.has_value()); +} + TEST_F(SendStatisticsProxyTest, QualityLimitationResolutionDoesNotUpdateForSpatialLayerChanges) { VideoCodec codec;