Remove old perf format conversion functions.

chrome/browser/media/chrome_webrtc_video_quality_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/environment.h"
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
@@ skipping 246 matching lines @@
     LOG(INFO) << "Output was:\n\n" << result;
     return ok;
   }
 
   // Compares the |captured_video_filename| with the |reference_video_filename|.
   //
   // The barcode decoder decodes the captured video containing barcodes overlaid
   // into every frame of the video (produced by rgba_to_i420_converter). It
   // produces a set of PNG images and a |stats_file| that maps each captured
   // frame to a frame in the reference video. The frames should be of size
-  // |width| x |height|. The output of compare_videos.py is returned.
+  // |width| x |height|.

[phoglund_chromium, 2013/09/12 07:32:10]

Add a comment which describes how the output gets printed, since that's not
really readily apparent. Additionally, perhaps we should rename this method
CompareVideosAndPrintResult?

[kjellander_chromium, 2013/09/12 09:02:36]

I agree with both, updated.

   bool CompareVideos(int width,
                      int height,
                      const base::FilePath& captured_video_filename,
                      const base::FilePath& reference_video_filename,
-                     const base::FilePath& stats_file,
-                     std::string* result) {
+                     const base::FilePath& stats_file) {
 
     base::FilePath path_to_analyzer = base::MakeAbsoluteFilePath(
         GetBrowserDir().Append(kFrameAnalyzerExecutable));
     base::FilePath path_to_compare_script = GetSourceDir().Append(
         FILE_PATH_LITERAL("third_party/webrtc/tools/compare_videos.py"));
 
     if (!base::PathExists(path_to_analyzer)) {
       LOG(ERROR) << "Missing frame analyzer: should be in "
           << path_to_analyzer.value();
       return false;
     }
     if (!base::PathExists(path_to_compare_script)) {
       LOG(ERROR) << "Missing video compare script: should be in "
           << path_to_compare_script.value();
       return false;
     }
 
     // Note: don't append switches to this command since it will mess up the
     // -u in the python invocation!
     CommandLine compare_command(CommandLine::NO_PROGRAM);
     EXPECT_TRUE(GetPythonCommand(&compare_command));
 
     compare_command.AppendArgPath(path_to_compare_script);
+    compare_command.AppendArg("--label=VGA");

[kjellander_chromium, 2013/09/11 15:30:58]

This is a new argument for the script, preparing for supporting both VGA and HD
(needs to separated in the output).

     compare_command.AppendArg("--ref_video");
     compare_command.AppendArgPath(reference_video_filename);
     compare_command.AppendArg("--test_video");
     compare_command.AppendArgPath(captured_video_filename);
     compare_command.AppendArg("--frame_analyzer");
     compare_command.AppendArgPath(path_to_analyzer);
     compare_command.AppendArg("--yuv_frame_width");
     compare_command.AppendArg(base::StringPrintf("%d", width));
     compare_command.AppendArg("--yuv_frame_height");
     compare_command.AppendArg(base::StringPrintf("%d", height));
     compare_command.AppendArg("--stats_file");
     compare_command.AppendArgPath(stats_file);
 
     LOG(INFO) << "Running " << compare_command.GetCommandLineString();
-    bool ok = base::GetAppOutput(compare_command, result);
-    LOG(INFO) << "Output was:\n\n" << *result;
+    std::string output;
+    bool ok = base::GetAppOutput(compare_command, &output);
+    LOG(INFO) << "Output was:\n\n" << output;

[phoglund_chromium, 2013/09/12 07:32:10]

Ok, so this LOG(INFO) is what really prints the output. I don't like putting a
program's formal output into a log statement, so I would rather have this
printed on stdout if possible. I can also imagine other potential problems, for
instance if we print

LOG(INFO) << "RESULT psnr...\nRESULT ssim\n...";

The printout will be something like

[29185:29185:0912/093019:INFO:...cc] RESULT psnr...
RESULT ssim...
...

I'm not sure if the perf tool will pick up the first line. One can solve this by
making sure the compare scripts prints nonimportant information on the first
line, but that seems hackish.

[kjellander_chromium, 2013/09/12 09:02:36]

Right, there's a ton of other stuff printed before the RESULT lines are printed,
but I changed it to std:cout just to be sure for the future.

     return ok;
   }
 
-  // Processes the |frame_analyzer_output| for the different frame counts.
-  //
-  // The frame analyzer outputs additional information about the number of
-  // unique frames captured, The max number of repeated frames in a sequence and
-  // the max number of skipped frames. These values are then written to the Perf
-  // Graph. (Note: Some of the repeated or skipped frames will probably be due
-  // to the imperfection of JavaScript timers).
-  void PrintFramesCountPerfResults(std::string frame_analyzer_output) {
-    size_t unique_frames_pos =
-        frame_analyzer_output.rfind("Unique_frames_count");
-    EXPECT_NE(unique_frames_pos, std::string::npos)
-        << "Missing Unique_frames_count in frame analyzer output:\n"
-        << frame_analyzer_output;
-
-    std::string unique_frame_counts =
-        frame_analyzer_output.substr(unique_frames_pos);
-    // TODO(phoglund): Fix ESTATS result to not have this silly newline.
-    std::replace(
-        unique_frame_counts.begin(), unique_frame_counts.end(), '\n', ' ');
-
-    std::vector<std::pair<std::string, std::string> > key_values;
-    base::SplitStringIntoKeyValuePairs(
-        unique_frame_counts, ':', ' ', &key_values);
-    std::vector<std::pair<std::string, std::string> >::const_iterator iter;
-    for (iter = key_values.begin(); iter != key_values.end(); ++iter) {
-      const std::pair<std::string, std::string>& key_value = *iter;
-      perf_test::PrintResult(
-          key_value.first, "", "VGA", key_value.second, "", false);
-    }
-  }
-
-  // Processes the |frame_analyzer_output| to extract the PSNR and SSIM values.
-  //
-  // The frame analyzer produces PSNR and SSIM results for every unique frame
-  // that has been captured. This method forms a list of all the psnr and ssim
-  // values and passes it to PrintResultList() for printing on the Perf Graph.
-  void PrintPsnrAndSsimPerfResults(std::string frame_analyzer_output) {
-    size_t stats_start = frame_analyzer_output.find("BSTATS");
-    EXPECT_NE(stats_start, std::string::npos)
-        << "Missing BSTATS in frame analyzer output:\n"
-        << frame_analyzer_output;
-    size_t stats_end = frame_analyzer_output.find("ESTATS");
-    EXPECT_NE(stats_end, std::string::npos)
-        << "Missing ESTATS in frame analyzer output:\n"
-        << frame_analyzer_output;
-
-    stats_start += std::string("BSTATS").size();
-    std::string psnr_ssim_stats =
-        frame_analyzer_output.substr(stats_start, stats_end - stats_start);
-
-    // PSNR and SSIM values aren't really key-value pairs but it is convenient
-    // to parse them as such.
-    // TODO(phoglund): make the format more convenient so we need less
-    // processing here.
-    std::vector<std::pair<std::string, std::string> > psnr_ssim_entries;
-    base::SplitStringIntoKeyValuePairs(
-        psnr_ssim_stats, ' ', ';', &psnr_ssim_entries);
-
-    std::string psnr_value_list;
-    std::string ssim_value_list;
-    std::vector<std::pair<std::string, std::string> >::const_iterator iter;
-    for (iter = psnr_ssim_entries.begin(); iter != psnr_ssim_entries.end();
-         ++iter) {
-      const std::pair<std::string, std::string>& psnr_and_ssim = *iter;
-      psnr_value_list.append(psnr_and_ssim.first).append(",");
-      ssim_value_list.append(psnr_and_ssim.second).append(",");
-    }
-
-    // Nuke last comma.
-    ASSERT_GT(psnr_value_list.size(), 0u) << "Received no valid PSNR values.";
-    ASSERT_GT(ssim_value_list.size(), 0u) << "Received no valid SSIM values.";
-    psnr_value_list.erase(psnr_value_list.size() - 1);
-    ssim_value_list.erase(ssim_value_list.size() - 1);
-
-    perf_test::PrintResultList("PSNR", "", "VGA", psnr_value_list, "dB", false);
-    perf_test::PrintResultList("SSIM", "", "VGA", ssim_value_list, "", false);
-  }
-
   base::FilePath GetWorkingDir() {
     std::string home_dir;
     environment_->GetVar(kHomeEnvName, &home_dir);
     base::FilePath::StringType native_home_dir(home_dir.begin(),
                                                home_dir.end());
     return base::FilePath(native_home_dir).Append(kWorkingDirName);
   }
 
   PeerConnectionServerRunner peerconnection_server_;
 
@@ skipping 56 matching lines @@
 
   AssertNoAsynchronousErrors(left_tab);
   AssertNoAsynchronousErrors(right_tab);
 
   EXPECT_TRUE(PollingWaitUntil(
       "haveMoreFramesToSend()", "no-more-frames", right_tab,
       polling_interval_msec));
 
   RunARGBtoI420Converter(
       kVgaWidth, kVgaHeight, GetWorkingDir().Append(kCapturedYuvFileName));
-  std::string output;
   ASSERT_TRUE(
       CompareVideos(kVgaWidth,
                     kVgaHeight,
                     GetWorkingDir().Append(kCapturedYuvFileName),
                     GetWorkingDir().Append(kReferenceYuvFileName),
-                    GetWorkingDir().Append(kStatsFileName),
-                    &output));
-
-  PrintFramesCountPerfResults(output);
-  PrintPsnrAndSsimPerfResults(output);
+                    GetWorkingDir().Append(kStatsFileName)));
 
   ASSERT_TRUE(peerconnection_server_.Stop());
   ASSERT_TRUE(ShutdownPyWebSocketServer());
 }