VideoProcessorIntegrationTest: Group member variables into two structs containing target/actual rate

modules/video_coding/codecs/test/videoprocessor_integrationtest.h

 /*
  *  Copyright (c) 2017 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.
  */
 
@@ skipping 13 matching lines @@
 #include "modules/video_coding/utility/ivf_file_writer.h"
 #include "test/gtest.h"
 #include "test/testsupport/frame_reader.h"
 #include "test/testsupport/frame_writer.h"
 #include "test/testsupport/packet_reader.h"
 
 namespace webrtc {
 namespace test {
 
 // The sequence of bit rate and frame rate changes for the encoder, the frame
-// number where the changes are made, and the total number of frames for the
-// test.
+// number where the changes are made, and the total number of frames to process.
 struct RateProfile {
   static const int kMaxNumRateUpdates = 3;
 
   int target_bit_rate[kMaxNumRateUpdates];
   int input_frame_rate[kMaxNumRateUpdates];
   int frame_index_rate_update[kMaxNumRateUpdates + 1];
   int num_frames;
 };
 
-// Thresholds for the rate control metrics. The rate mismatch thresholds are
-// defined as percentages. |max_time_hit_target| is defined as number of frames,
-// after a rate update is made to the encoder, for the encoder to reach within
-// |kPercTargetvsActualMismatch| of new target rate. The thresholds are defined
-// for each rate update sequence.
+// Thresholds for the rate control metrics. The thresholds are defined for each
+// rate update sequence. |max_num_frames_to_hit_target| is defined as number of
+// frames, after a rate update is made to the encoder, for the encoder to reach
+// |kMaxBitrateMismatchPercent| of new target rate.
 struct RateControlThresholds {
   int max_num_dropped_frames;
-  int max_key_frame_size_mismatch;
-  int max_delta_frame_size_mismatch;
-  int max_encoding_rate_mismatch;
-  int max_time_hit_target;
+  int max_key_framesize_mismatch_percent;
+  int max_delta_framesize_mismatch_percent;
+  int max_bitrate_mismatch_percent;
+  int max_num_frames_to_hit_target;
   int num_spatial_resizes;
   int num_key_frames;
 };
 
 // Thresholds for the quality metrics.
 struct QualityThresholds {
   QualityThresholds(double min_avg_psnr,
                     double min_min_psnr,
                     double min_avg_ssim,
                     double min_min_ssim)
@@ skipping 39 matching lines @@
                                int height);
 
   static void SetRateProfile(RateProfile* rate_profile,
                              int rate_update_index,
                              int bitrate_kbps,
                              int framerate_fps,
                              int frame_index_rate_update);
 
   static void AddRateControlThresholds(
       int max_num_dropped_frames,
-      int max_key_frame_size_mismatch,
-      int max_delta_frame_size_mismatch,
-      int max_encoding_rate_mismatch,
-      int max_time_hit_target,
+      int max_key_framesize_mismatch_percent,
+      int max_delta_framesize_mismatch_percent,
+      int max_bitrate_mismatch_percent,
+      int max_num_frames_to_hit_target,
       int num_spatial_resizes,
       int num_key_frames,
       std::vector<RateControlThresholds>* rc_thresholds);
 
   void ProcessFramesAndMaybeVerify(
       const RateProfile& rate_profile,
       const std::vector<RateControlThresholds>* rc_thresholds,
       const QualityThresholds* quality_thresholds,
       const VisualizationParams* visualization_params);
 
   // Config.
   TestConfig config_;
 
  private:
   static const int kMaxNumTemporalLayers = 3;
 
   void CreateEncoderAndDecoder();
   void DestroyEncoderAndDecoder();
   void SetUpAndInitObjects(rtc::TaskQueue* task_queue,
                            const int initial_bitrate_kbps,
                            const int initial_framerate_fps,
                            const VisualizationParams* visualization_params);
   void ReleaseAndCloseObjects(rtc::TaskQueue* task_queue);
+  int TemporalLayerIndexForFrame(int frame_number) const;
+
+  // Rate control metrics.
+  void ResetRateControlMetrics(int rate_update_index,
+                               const RateProfile& rate_profile);
   void UpdateRateControlMetrics(int frame_number);
   void PrintAndMaybeVerifyRateControlMetrics(
       int rate_update_index,
       const std::vector<RateControlThresholds>* rc_thresholds,
       const std::vector<int>& num_dropped_frames,
-      const std::vector<int>& num_resize_actions);
-  int TemporalLayerIndexForFrame(int frame_number) const;
-  void ResetRateControlMetrics(int rate_update_index,
-                               const RateProfile& rate_profile);
+      const std::vector<int>& num_spatial_resizes) const;
+  void PrintRateControlMetrics(

[brandtr, 2017/09/21 08:40:10]

Declare above PrintAndMaybeVerifyRateControlMetrics, as that functions uses this
function.

[åsapersson, 2017/09/22 08:06:50]

Done.

+      int rate_update_index,
+      const std::vector<int>& num_dropped_frames,
+      const std::vector<int>& num_spatial_resizes) const;
 
   // Codecs.
   std::unique_ptr<VideoEncoder> encoder_;
   std::unique_ptr<cricket::WebRtcVideoDecoderFactory> decoder_factory_;
   VideoDecoder* decoder_;
 
   // Helper objects.
   std::unique_ptr<FrameReader> analysis_frame_reader_;
   std::unique_ptr<FrameWriter> analysis_frame_writer_;
   std::unique_ptr<IvfFileWriter> encoded_frame_writer_;
   std::unique_ptr<FrameWriter> decoded_frame_writer_;
   PacketReader packet_reader_;
   std::unique_ptr<PacketManipulator> packet_manipulator_;
   Stats stats_;
   std::unique_ptr<VideoProcessor> processor_;
 
-  // Quantities defined/updated for every encoder rate update.
-  int num_frames_per_update_[kMaxNumTemporalLayers];
-  float sum_frame_size_mismatch_[kMaxNumTemporalLayers];
-  float sum_encoded_frame_size_[kMaxNumTemporalLayers];
-  float encoding_bitrate_[kMaxNumTemporalLayers];
-  float per_frame_bandwidth_[kMaxNumTemporalLayers];
-  float bitrate_layer_[kMaxNumTemporalLayers];
-  float framerate_layer_[kMaxNumTemporalLayers];
-  int num_frames_total_;
-  float sum_encoded_frame_size_total_;
-  float encoding_bitrate_total_;
-  float perc_encoding_rate_mismatch_;
-  int num_frames_to_hit_target_;
-  bool encoding_rate_within_target_;
-  int bitrate_kbps_;
-  int framerate_;
-  float target_size_key_frame_initial_;
-  float target_size_key_frame_;
-  float sum_key_frame_size_mismatch_;
-  int num_key_frames_;
+  // Quantities updated for every encoded frame.
+  struct TestResults {

[brandtr, 2017/09/21 08:40:10]

Move definition to the top of the private: section.

https://google.github.io/styleguide/cppguide.html#Declaration_Order

[åsapersson, 2017/09/22 08:06:50]

Done.

+    int KeyFrameSizeMismatchPercent() const {
+      if (num_key_frames == 0) {
+        return -1;
+      }
+      return 100 * sum_key_framesize_mismatch / num_key_frames;
+    }
+    int DeltaFrameSizeMismatchPercent(int i) const {

[brandtr, 2017/09/21 08:40:10]

In a followup CL, it would be nice to output the percent as a float instead. I
don't see a good reason to round to the nearest integer really.

[åsapersson, 2017/09/22 08:06:50]

Acknowledged.

+      return 100 * sum_delta_framesize_mismatch_layer[i] / num_frames_layer[i];
+    }
+    int BitrateMismatchPercent(float target_kbps) const {
+      return 100 * fabs(kbps - target_kbps) / target_kbps;
+    }
+    int BitrateMismatchPercent(int i, float target_kbps_layer) const {
+      return 100 * fabs(kbps_layer[i] - target_kbps_layer) / target_kbps_layer;
+    }
+    int num_frames = 0;
+    int num_frames_layer[kMaxNumTemporalLayers] = {0};
+    int num_key_frames = 0;
+    int num_frames_to_hit_target = 0;
+    float sum_framesize_kbits = 0.0f;
+    float sum_framesize_kbits_layer[kMaxNumTemporalLayers] = {0};
+    float kbps = 0.0f;
+    float kbps_layer[kMaxNumTemporalLayers] = {0};
+    float sum_key_framesize_mismatch = 0.0f;
+    float sum_delta_framesize_mismatch_layer[kMaxNumTemporalLayers] = {0};
+  };
+  TestResults actual_;

[brandtr, 2017/09/21 08:40:10]

What do you think about getting rid of |actual_| and |target_| as member
variables? We could either just pass them into the corresponding rate control
metrics functions, or split out the entire rate control metric calculation into
a utility class.

Let's discuss what the right thing to do is, and then we can think of some
followup CL.

[åsapersson, 2017/09/22 08:06:50]

Sounds good.

+
+  // Rates set for every encoder rate update.
+  struct TargetRates {
+    int kbps;
+    int fps;
+    float kbps_layer[kMaxNumTemporalLayers];
+    float fps_layer[kMaxNumTemporalLayers];
+    float framesize_kbits_layer[kMaxNumTemporalLayers];
+    float key_framesize_kbits_initial;
+    float key_framesize_kbits;
+  };
+  void SetRatesPerTemporalLayer();
+  TargetRates target_;
 };
 
 }  // namespace test
 }  // namespace webrtc
 
 #endif  // MODULES_VIDEO_CODING_CODECS_TEST_VIDEOPROCESSOR_INTEGRATIONTEST_H_