From 9dbd084761208bd161f10a3b3bcb6295339c15e9 Mon Sep 17 00:00:00 2001 From: brandtr Date: Fri, 25 Aug 2017 07:11:19 -0700 Subject: [PATCH] Delete batch mode from VideoProcessorIntegrationTest. After landing https://codereview.webrtc.org/2997283002/, batch mode will no longer be needed. BUG=webrtc:6634 Review-Url: https://codereview.webrtc.org/3005593002 Cr-Commit-Position: refs/heads/master@{#19520} --- .../plot_videoprocessor_integrationtest.cc | 3 +- .../video_coding/codecs/test/videoprocessor.h | 6 -- .../test/videoprocessor_integrationtest.cc | 51 ++++--------- .../test/videoprocessor_integrationtest.h | 73 ++++++------------- 4 files changed, 37 insertions(+), 96 deletions(-) diff --git a/webrtc/modules/video_coding/codecs/test/plot_videoprocessor_integrationtest.cc b/webrtc/modules/video_coding/codecs/test/plot_videoprocessor_integrationtest.cc index 468a718a88..b8f152d7ab 100644 --- a/webrtc/modules/video_coding/codecs/test/plot_videoprocessor_integrationtest.cc +++ b/webrtc/modules/video_coding/codecs/test/plot_videoprocessor_integrationtest.cc @@ -30,7 +30,6 @@ const bool kSpatialResizeOn = false; const bool kFrameDropperOn = false; // Test settings. -const bool kBatchMode = true; const bool kVerboseLogging = true; const float kPacketLoss = 0.0f; const VisualizationParams kVisualizationParams = { @@ -59,7 +58,7 @@ class PlotVideoProcessorIntegrationTest int framerate, const std::string& filename) { SetTestConfig(&config_, hw_codec_, kUseSingleCore, kPacketLoss, filename, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, codec_type_, kNumTemporalLayers, kErrorConcealmentOn, kDenoisingOn, kFrameDropperOn, kSpatialResizeOn, kResilienceOn, width, height); diff --git a/webrtc/modules/video_coding/codecs/test/videoprocessor.h b/webrtc/modules/video_coding/codecs/test/videoprocessor.h index 49a7b2e2a0..a8c9858dd9 100644 --- a/webrtc/modules/video_coding/codecs/test/videoprocessor.h +++ b/webrtc/modules/video_coding/codecs/test/videoprocessor.h @@ -113,12 +113,6 @@ struct TestConfig { // If HW or SW codec should be used. bool hw_codec = false; - - // In batch mode, the VideoProcessor is fed all the frames for processing - // before any metrics are calculated. This is useful for pipelining HW codecs, - // for which some calculated metrics otherwise would be incorrect. The - // downside with batch mode is that mid-test rate allocation is not supported. - bool batch_mode = false; }; // Handles encoding/decoding of video using the VideoEncoder/VideoDecoder diff --git a/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc b/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc index 87dc5f3043..04609bf525 100644 --- a/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc +++ b/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.cc @@ -22,7 +22,6 @@ namespace { const bool kUseSingleCore = true; const bool kVerboseLogging = false; const bool kHwCodec = false; -const bool kBatchMode = false; // Codec settings. const bool kResilienceOn = true; @@ -49,7 +48,7 @@ const std::nullptr_t kNoVisualizationParams = nullptr; // not been added. TEST_F(VideoProcessorIntegrationTest, Process0PercentPacketLossH264) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecH264, 1, false, false, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -77,7 +76,7 @@ TEST_F(VideoProcessorIntegrationTest, Process0PercentPacketLossH264) { // One key frame (first frame only) in sequence. TEST_F(VideoProcessorIntegrationTest, Process0PercentPacketLossVP9) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP9, 1, false, false, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -99,7 +98,7 @@ TEST_F(VideoProcessorIntegrationTest, Process0PercentPacketLossVP9) { // lower. One key frame (first frame only) in sequence. TEST_F(VideoProcessorIntegrationTest, Process5PercentPacketLossVP9) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.05f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP9, 1, false, false, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -123,7 +122,7 @@ TEST_F(VideoProcessorIntegrationTest, Process5PercentPacketLossVP9) { // One key frame (first frame only) in sequence. TEST_F(VideoProcessorIntegrationTest, ProcessNoLossChangeBitRateVP9) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP9, 1, false, false, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -155,7 +154,7 @@ TEST_F(VideoProcessorIntegrationTest, ProcessNoLossChangeBitRateVP9) { TEST_F(VideoProcessorIntegrationTest, ProcessNoLossChangeFrameRateFrameDropVP9) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP9, 1, false, false, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -180,7 +179,7 @@ TEST_F(VideoProcessorIntegrationTest, // VP9: Run with no packet loss and denoiser on. One key frame (first frame). TEST_F(VideoProcessorIntegrationTest, ProcessNoLossDenoiserOnVP9) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP9, 1, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -204,7 +203,7 @@ TEST_F(VideoProcessorIntegrationTest, ProcessNoLossDenoiserOnVP9) { TEST_F(VideoProcessorIntegrationTest, DISABLED_ProcessNoLossSpatialResizeFrameDropVP9) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP9, 1, false, false, true, true, kResilienceOn, kCifWidth, kCifHeight); @@ -232,7 +231,7 @@ TEST_F(VideoProcessorIntegrationTest, // to -1 below means no periodic key frames in test. TEST_F(VideoProcessorIntegrationTest, ProcessZeroPacketLoss) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP8, 1, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -254,7 +253,7 @@ TEST_F(VideoProcessorIntegrationTest, ProcessZeroPacketLoss) { // lower. One key frame (first frame only) in sequence. TEST_F(VideoProcessorIntegrationTest, Process5PercentPacketLoss) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.05f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP8, 1, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -276,7 +275,7 @@ TEST_F(VideoProcessorIntegrationTest, Process5PercentPacketLoss) { // One key frame (first frame only) in sequence. TEST_F(VideoProcessorIntegrationTest, Process10PercentPacketLoss) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.1f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP8, 1, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -294,30 +293,6 @@ TEST_F(VideoProcessorIntegrationTest, Process10PercentPacketLoss) { kNoVisualizationParams); } -// This test is identical to VideoProcessorIntegrationTest.ProcessZeroPacketLoss -// except that |batch_mode| is turned on. The main point of this test is to see -// that the reported stats are not wildly varying between batch mode and the -// regular online mode. -TEST_F(VideoProcessorIntegrationTest, ProcessInBatchMode) { - SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, true /* batch_mode */); - SetCodecSettings(&config_, kVideoCodecVP8, 1, false, true, true, false, - kResilienceOn, kCifWidth, kCifHeight); - - RateProfile rate_profile; - SetRateProfile(&rate_profile, 0, 500, 30, 0); - rate_profile.frame_index_rate_update[1] = kNumFramesShort + 1; - rate_profile.num_frames = kNumFramesShort; - - std::vector rc_thresholds; - AddRateControlThresholds(0, 40, 20, 10, 15, 0, 1, &rc_thresholds); - - QualityThresholds quality_thresholds(34.95, 33.0, 0.90, 0.89); - - ProcessFramesAndMaybeVerify(rate_profile, &rc_thresholds, &quality_thresholds, - kNoVisualizationParams); -} - #endif // !defined(WEBRTC_IOS) // The tests below are currently disabled for Android. For ARM, the encoder @@ -342,7 +317,7 @@ TEST_F(VideoProcessorIntegrationTest, ProcessInBatchMode) { #endif TEST_F(VideoProcessorIntegrationTest, MAYBE_ProcessNoLossChangeBitRateVP8) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP8, 1, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -382,7 +357,7 @@ TEST_F(VideoProcessorIntegrationTest, MAYBE_ProcessNoLossChangeBitRateVP8) { TEST_F(VideoProcessorIntegrationTest, MAYBE_ProcessNoLossChangeFrameRateFrameDropVP8) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP8, 1, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); @@ -418,7 +393,7 @@ TEST_F(VideoProcessorIntegrationTest, #endif TEST_F(VideoProcessorIntegrationTest, MAYBE_ProcessNoLossTemporalLayersVP8) { SetTestConfig(&config_, kHwCodec, kUseSingleCore, 0.0f, kForemanCif, - kVerboseLogging, kBatchMode); + kVerboseLogging); SetCodecSettings(&config_, kVideoCodecVP8, 3, false, true, true, false, kResilienceOn, kCifWidth, kCifHeight); diff --git a/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h b/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h index d87ffed55d..f971a01c76 100644 --- a/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h +++ b/webrtc/modules/video_coding/codecs/test/videoprocessor_integrationtest.h @@ -492,59 +492,34 @@ class VideoProcessorIntegrationTest : public testing::Test { ResetRateControlMetrics( rate_profile.frame_index_rate_update[update_index + 1]); - if (config_.batch_mode) { - // In batch mode, we calculate the metrics for all frames after all frames - // have been sent for encoding. + while (frame_number < num_frames) { + processor_->ProcessFrame(frame_number); + VerifyQpParser(frame_number); + const int tl_idx = TemporalLayerIndexForFrame(frame_number); + ++num_frames_per_update_[tl_idx]; + ++num_frames_total_; + UpdateRateControlMetrics(frame_number); - for (frame_number = 0; frame_number < num_frames; ++frame_number) { - processor_->ProcessFrame(frame_number); - } + ++frame_number; - for (frame_number = 0; frame_number < num_frames; ++frame_number) { - const int tl_idx = TemporalLayerIndexForFrame(frame_number); - ++num_frames_per_update_[tl_idx]; - ++num_frames_total_; - UpdateRateControlMetrics(frame_number); - } - } else { - // In online mode, we calculate the metrics for a given frame right after - // it has been sent for encoding. + // If we hit another/next update, verify stats for current state and + // update layers and codec with new rates. + if (frame_number == + rate_profile.frame_index_rate_update[update_index + 1]) { + PrintAndMaybeVerifyRateControlMetrics(update_index, rc_thresholds); - if (config_.hw_codec) { - LOG(LS_WARNING) << "HW codecs should mostly be run in batch mode, " - "since they may be pipelining."; - } - - while (frame_number < num_frames) { - processor_->ProcessFrame(frame_number); - VerifyQpParser(frame_number); - const int tl_idx = TemporalLayerIndexForFrame(frame_number); - ++num_frames_per_update_[tl_idx]; - ++num_frames_total_; - UpdateRateControlMetrics(frame_number); - - ++frame_number; - - // If we hit another/next update, verify stats for current state and - // update layers and codec with new rates. - if (frame_number == - rate_profile.frame_index_rate_update[update_index + 1]) { - PrintAndMaybeVerifyRateControlMetrics(update_index, rc_thresholds); - - // Update layer rates and the codec with new rates. - ++update_index; - bitrate_kbps_ = rate_profile.target_bit_rate[update_index]; - framerate_ = rate_profile.input_frame_rate[update_index]; - SetTemporalLayerRates(); - ResetRateControlMetrics( - rate_profile.frame_index_rate_update[update_index + 1]); - processor_->SetRates(bitrate_kbps_, framerate_); - } + // Update layer rates and the codec with new rates. + ++update_index; + bitrate_kbps_ = rate_profile.target_bit_rate[update_index]; + framerate_ = rate_profile.input_frame_rate[update_index]; + SetTemporalLayerRates(); + ResetRateControlMetrics( + rate_profile.frame_index_rate_update[update_index + 1]); + processor_->SetRates(bitrate_kbps_, framerate_); } } - // Verify rate control metrics for all frames (if in batch mode), or for all - // frames since the last rate update (if not in batch mode). + // Verify rate control metrics for all frames since the last rate update. PrintAndMaybeVerifyRateControlMetrics(update_index, rc_thresholds); EXPECT_EQ(num_frames, frame_number); EXPECT_EQ(num_frames, static_cast(stats_.stats_.size())); @@ -592,8 +567,7 @@ class VideoProcessorIntegrationTest : public testing::Test { bool use_single_core, float packet_loss_probability, std::string filename, - bool verbose_logging, - bool batch_mode) { + bool verbose_logging) { config->filename = filename; config->input_filename = ResourcePath(filename, "yuv"); // Generate an output filename in a safe way. @@ -603,7 +577,6 @@ class VideoProcessorIntegrationTest : public testing::Test { config->use_single_core = use_single_core; config->verbose = verbose_logging; config->hw_codec = hw_codec; - config->batch_mode = batch_mode; } static void SetCodecSettings(TestConfig* config,