Add histogram stats for average QP per frame for VP8 (for sent video streams):

webrtc/video/send_statistics_proxy.cc

 /*
  *  Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/video/send_statistics_proxy.h"
 
 #include <algorithm>
 #include <cmath>
 #include <map>
+#include <vector>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/system_wrappers/include/metrics.h"
 
 namespace webrtc {
 namespace {
 const float kEncodeTimeWeigthFactor = 0.5f;
 
 // Used by histograms. Values of entries should not be changed.
@@ skipping 159 matching lines @@
   if (delay_ms != -1)
     RTC_HISTOGRAMS_COUNTS_100000(kIndex, uma_prefix_ + "SendSideDelayInMs",
                                  delay_ms);
 
   int max_delay_ms = max_delay_counter_.Avg(kMinRequiredSamples);
   if (max_delay_ms != -1) {
     RTC_HISTOGRAMS_COUNTS_100000(kIndex, uma_prefix_ + "SendSideDelayMaxInMs",
                                  max_delay_ms);
   }
 
+  for (const auto& it : qp_counters_) {
+    int qp = it.second.vp8.Avg(kMinRequiredSamples);
+    if (qp != -1) {
+      int spatial_idx = it.first;
+      if (spatial_idx == -1) {

[mflodman, 2016/03/23 12:25:20]

I think it would be nice to rather build a string based on 'spatial_idx' and
only have one call to RTC_HISTOGRAMS_COUNT. That would mean less duplicated
code.

wdyt?

[pbos-webrtc, 2016/03/23 12:34:03]

We can't since RTC_HISTOGRAMS_COUNTS requires the string to be unique per macro
for performance reasons.

[åsapersson, 2016/03/23 13:09:26]

Right the name should not vary for a macro.

+        RTC_HISTOGRAMS_COUNTS_200(kIndex, uma_prefix_ + "Encoded.Qp.Vp8", qp);
+      } else if (spatial_idx == 0) {
+        RTC_HISTOGRAMS_COUNTS_200(kIndex, uma_prefix_ + "Encoded.Qp.Vp8.S0",
+                                  qp);
+      } else if (spatial_idx == 1) {
+        RTC_HISTOGRAMS_COUNTS_200(kIndex, uma_prefix_ + "Encoded.Qp.Vp8.S1",
+                                  qp);
+      } else if (spatial_idx == 2) {
+        RTC_HISTOGRAMS_COUNTS_200(kIndex, uma_prefix_ + "Encoded.Qp.Vp8.S2",
+                                  qp);
+      }

[pbos-webrtc, 2016/03/23 12:34:03]

Should we have an else case here that should do something in case spatial_idx
goes above 2 to prevent dropping some unknown stats in the future?

[åsapersson, 2016/03/23 13:09:26]

Added a warning...

+    }
+  }
+
   if (first_rtcp_stats_time_ms_ != -1) {
     int64_t elapsed_sec =
         (clock_->TimeInMilliseconds() - first_rtcp_stats_time_ms_) / 1000;
     if (elapsed_sec >= metrics::kMinRunTimeInSeconds) {
       int fraction_lost = report_block_stats_.FractionLostInPercent();
       if (fraction_lost != -1) {
         RTC_HISTOGRAMS_PERCENTAGE(
             kIndex, uma_prefix_ + "SentPacketsLostInPercent", fraction_lost);
       }
 
@@ skipping 214 matching lines @@
   }
   if (encoded_image.adapt_reason_.bw_resolutions_disabled != -1) {
     bool bw_limited = encoded_image.adapt_reason_.bw_resolutions_disabled > 0;
     uma_container_->bw_limited_frame_counter_.Add(bw_limited);
     if (bw_limited) {
       uma_container_->bw_resolutions_disabled_counter_.Add(
           encoded_image.adapt_reason_.bw_resolutions_disabled);
     }
   }
 
+  if (encoded_image.qp_ != -1 && rtp_video_header != nullptr) {
+    if (rtp_video_header->codec == kRtpVideoVp8) {

[mflodman, 2016/03/23 12:25:20]

This can be an additional condition in the if above rather than having two ifs.

[pbos-webrtc, 2016/03/23 12:34:03]

I think so as well, if ssrcs.size() is 1 then we ignore what spatial_idx
actually is

[åsapersson, 2016/03/23 13:09:26]

-1 might be used when not using spatial layers for VP9 (therefore used here to
keep it similar..).

[åsapersson, 2016/03/23 13:09:26]

Done.

+      int spatial_idx = (config_.rtp.ssrcs.size() == 1)
+                            ? -1
+                            : static_cast<int>(simulcast_idx);
+      uma_container_->qp_counters_[spatial_idx].vp8.Add(encoded_image.qp_);
+    }
+  }
+
   // TODO(asapersson): This is incorrect if simulcast layers are encoded on
   // different threads and there is no guarantee that one frame of all layers
   // are encoded before the next start.
   if (last_sent_frame_timestamp_ > 0 &&
       encoded_image._timeStamp != last_sent_frame_timestamp_) {
     uma_container_->sent_frame_rate_tracker_.AddSamples(1);
     uma_container_->sent_width_counter_.Add(
         uma_container_->max_sent_width_per_timestamp_);
     uma_container_->sent_height_counter_.Add(
         uma_container_->max_sent_height_per_timestamp_);
@@ skipping 118 matching lines @@
   return Fraction(min_required_samples, 1000.0f);
 }
 
 int SendStatisticsProxy::BoolSampleCounter::Fraction(
     int min_required_samples, float multiplier) const {
   if (num_samples < min_required_samples || num_samples == 0)
     return -1;
   return static_cast<int>((sum * multiplier / num_samples) + 0.5f);
 }
 }  // namespace webrtc