Report 'outbound-rtp.targetBitrate' correctly and per-RTP stream.

This CL fixes two issues with the old way targetBitrate was reported:
1. The target is per encoder, i.e. per SSRC, but the old way to report
   it was per sender and was approximately the sum of all encodings'
   targetBitrate in most cases.
2. The old value did not come directly from the VideoBitrateAllocation
   and tended to be greater than the sum of all targets (don't know
   why).

We know the old value was wrong and the new value correct because
the actual bytes produced by the encoder closely matches the configured
target, which wasn't always the case with the old metric implementation.

Tested with unit tests and manually in Chrome by going to
https://henbos.github.io/codec-quality/src/index.html and ensuring
target ~= actual bytes produced. It also matches the debug logging of
video_stream_encoder.cc.

Bug: webrtc:42225524, chromium:392424845
Change-Id: I7a6f69e053ebc3fd972c2c4b7712750e721c0acc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376460
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43854}

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",

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<ScalabilityMode> 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<DataRate> 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
+    // Metric only used by legacy getStats()'s BWE.
-    // limitations.
+    // - 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;

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",

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<double> target_bitrate;
+  std::optional<webrtc::DataRate> target_bitrate;
   int packets_lost = 0;
   float fraction_lost = 0.0f;
   int64_t rtt_ms = 0;

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;

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;

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() &&
+  if (voice_sender_info.target_bitrate.has_value()) {
-      *voice_sender_info.target_bitrate > 0) {
+    outbound_audio->target_bitrate = voice_sender_info.target_bitrate->bps();
-    outbound_audio->target_bitrate = *voice_sender_info.target_bitrate;
   }
   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<uint32_t>(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() &&
+  if (video_sender_info.target_bitrate.has_value()) {
-      *video_sender_info.target_bitrate > 0) {
+    outbound_video->target_bitrate = video_sender_info.target_bitrate->bps();
-    outbound_video->target_bitrate = *video_sender_info.target_bitrate;
   }
   outbound_video->frames_encoded = video_sender_info.frames_encoded;
   outbound_video->key_frames_encoded = video_sender_info.key_frames_encoded;

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;
 

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<size_t> 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

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;