Better comparison for frame analyzer

webrtc/tools/frame_analyzer/video_quality_analysis.cc

 /*
  *  Copyright (c) 2012 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 310 matching lines @@
 }
 
 void PrintMaxRepeatedAndSkippedFrames(const std::string& label,
                                       const std::string& stats_file_ref_name,
                                       const std::string& stats_file_test_name) {
   PrintMaxRepeatedAndSkippedFrames(stdout, label, stats_file_ref_name,
                                    stats_file_test_name);
 }
 
 namespace {
+// The barcode number that means that the barcode could not be decoded.
+const int DECODE_ERROR = -1;
+
 // Clusters the frames in the file. First in the pair is the frame number and
 // second is the number
 // of frames in that cluster. So if first frame in video has number 100 and it
 // is repeated 3 after
 // each other, then the first entry in the returned vector has first set to 100
 // and second set
 // to 3.
 std::vector<std::pair<int, int> > CalculateFrameClusters(FILE* file) {
   std::vector<std::pair<int, int> > frame_cnt;
   char line[STATS_LINE_LENGTH];
   while (GetNextStatsLine(file, line)) {
-    int decoded_frame_number = ExtractDecodedFrameNumber(line);
-    if (decoded_frame_number == -1) {
-      continue;
+    int decoded_frame_number;
+    if (IsThereBarcodeError(line)) {
+      decoded_frame_number = DECODE_ERROR;

[kjellander_webrtc, 2017/02/06 12:30:38]

Can we increment a counter for this so we can log this as a perf metric?

[mandermo, 2017/02/07 14:25:27]

I have now added a separate counter for reference and test and print them in the
end.

+    } else {
+      decoded_frame_number = ExtractDecodedFrameNumber(line);
     }
-    if (frame_cnt.empty() || frame_cnt.back().first != decoded_frame_number) {
+    if (frame_cnt.size() >= 2 && decoded_frame_number != DECODE_ERROR &&
+        frame_cnt.back().first == DECODE_ERROR &&
+        frame_cnt[frame_cnt.size() - 2].first == decoded_frame_number) {
+      // Handle when there is a decoding error inside a cluster of frames.
+      frame_cnt[frame_cnt.size() - 2].second += frame_cnt.back().second;
+      frame_cnt.pop_back();
+    } else if (frame_cnt.empty() ||
+               frame_cnt.back().first != decoded_frame_number) {
       frame_cnt.push_back(std::make_pair(decoded_frame_number, 1));
     } else {
       ++frame_cnt.back().second;
     }
   }
   return frame_cnt;
 }
 }  // namespace
 
 void PrintMaxRepeatedAndSkippedFrames(FILE* output,
                                       const std::string& label,
                                       const std::string& stats_file_ref_name,
                                       const std::string& stats_file_test_name) {
   FILE* stats_file_ref = fopen(stats_file_ref_name.c_str(), "r");
   FILE* stats_file_test = fopen(stats_file_test_name.c_str(), "r");
   if (stats_file_ref == NULL) {
     fprintf(stderr, "Couldn't open reference stats file for reading: %s\n",
             stats_file_ref_name.c_str());
     return;
   }
   if (stats_file_test == NULL) {
     fprintf(stderr, "Couldn't open test stats file for reading: %s\n",
             stats_file_test_name.c_str());
     fclose(stats_file_ref);
     return;
   }
 
   int max_repeated_frames = 1;
-  int max_skipped_frames = 1;
+  int max_skipped_frames = 0;
 
   std::vector<std::pair<int, int> > frame_cnt_ref =
       CalculateFrameClusters(stats_file_ref);
 
   std::vector<std::pair<int, int> > frame_cnt_test =
       CalculateFrameClusters(stats_file_test);
 
   fclose(stats_file_ref);
   fclose(stats_file_test);
 
   auto it_ref = frame_cnt_ref.begin();
   auto it_test = frame_cnt_test.begin();
   auto end_ref = frame_cnt_ref.end();
   auto end_test = frame_cnt_test.end();
 
   if (it_test == end_test || it_ref == end_ref) {
     fprintf(stderr, "Either test or ref file is empty, nothing to print\n");
     return;
   }
 
+  while (it_test != end_test && it_test->first == DECODE_ERROR) {
+    ++it_test;
+  }
+
+  if (it_test == end_test) {
+    fprintf(stderr, "Test video only has barcode decode errors\n");
+    return;

[kjellander_webrtc, 2017/02/06 12:30:38]

It seems we should start returning an exit code from the application instead of
just printing something and returning void.

Then
https://cs.chromium.org/chromium/src/third_party/webrtc/tools/frame_analyzer/...
could just be updated to return the int and we would get test failures for cases
like this assuming the calling script for frame analyzer is looking at the exit
code.

With that, it would make sense to start writing a few unit tests for
PrintMaxRepeatedAndSkippedFrames that verifies the code for the different cases.
With this CL there's now so much more error handling logic so I think it makes a
lot of sense to have such tests now.

[mandermo, 2017/02/07 14:25:27]

Have added test cases covering different scenarios. The cases that return early
are less interesting and complicated than the different combinations of decode
errors I cover.

+  }
+
   // Find the first frame in the reference video that match the first frame in
   // the test video.
-  while (it_ref != end_ref && it_ref->first != it_test->first) {
+  while (it_ref != end_ref &&
+         (it_ref->first == DECODE_ERROR || it_ref->first != it_test->first)) {
     ++it_ref;
   }
   if (it_ref == end_ref) {
     fprintf(stderr,
             "The barcode in the test video's first frame is not in the "
             "reference video.\n");
     return;
   }
 
+  // The test frames that does not have corresponding barcode in the reference
+  // video.
+  std::vector<int> no_match_in_ref;
+
+  int total_skipped = 0;
   for (;;) {
     max_repeated_frames =
         std::max(max_repeated_frames, it_test->second - it_ref->second + 1);
+
+    bool passed_error = false;
+
     ++it_test;
+    while (it_test != end_test && it_test->first == DECODE_ERROR) {
+      ++it_test;
+      passed_error = true;
+    }
     if (it_test == end_test) {
       break;
     }
+
     int skipped_frames = 0;
+    auto old_it_ref = it_ref;
     ++it_ref;
-    while (it_ref != end_ref && it_ref->first != it_test->first) {
-      skipped_frames += it_ref->second;
-      ++it_ref;
+    for (; it_ref != end_ref; ++it_ref) {
+      if (it_ref->first == DECODE_ERROR) {
+        passed_error = true;
+        continue;
+      }
+      if (it_ref->first >= it_test->first) {
+        break;
+      }
+      ++skipped_frames;
     }
+    if (passed_error) {
+      skipped_frames = 0;
+    }
+    if (it_ref != end_ref && it_ref->first == it_test->first) {
+      total_skipped += skipped_frames;
+      if (skipped_frames > max_skipped_frames) {
+        max_skipped_frames = skipped_frames;
+      }
+      continue;
+    }
+    no_match_in_ref.push_back(it_test->first);
     if (it_ref == end_ref) {
-      fprintf(stderr,
-              "The barcode in the test video is not in the reference video.\n");
-      return;
-    }
-    if (skipped_frames > max_skipped_frames) {
-      max_skipped_frames = skipped_frames;
+      break;
+    } else {
+      it_ref = old_it_ref;
     }
   }
 
   fprintf(output, "RESULT Max_repeated: %s= %d\n", label.c_str(),
           max_repeated_frames);
   fprintf(output, "RESULT Max_skipped: %s= %d\n", label.c_str(),
           max_skipped_frames);
+  fprintf(output, "RESULT Total_skipped: %s= %d\n", label.c_str(),
+          total_skipped);
+  fprintf(output, "RESULT No_matched: [");
+  for (std::size_t i = 0; i < no_match_in_ref.size(); ++i) {
+    if (i > 0) {
+      fprintf(output, ", ");
+    }
+    fprintf(output, "%d", no_match_in_ref[i]);
+  }
+  fprintf(output, "]\n");
 }
 
 void PrintAnalysisResults(const std::string& label, ResultsContainer* results) {
   PrintAnalysisResults(stdout, label, results);
 }
 
 void PrintAnalysisResults(FILE* output, const std::string& label,
                           ResultsContainer* results) {
   std::vector<AnalysisResult>::iterator iter;
 
@@ skipping 12 matching lines @@
     for (iter = results->frames.begin(); iter != results->frames.end() - 1;
          ++iter) {
       fprintf(output, "%f,", iter->ssim_value);
     }
     fprintf(output, "%f] score\n", iter->ssim_value);
   }
 }
 
 }  // namespace test
 }  // namespace webrtc