Comparison of videos with reference frame not starting from zero

webrtc/tools/compare_videos.py

Index: webrtc/tools/compare_videos.py
diff --git a/webrtc/tools/compare_videos.py b/webrtc/tools/compare_videos.py
index 6aa659be362e4828e809979e4b54d6ffea3aedfe..ead5acd305d4d264021b0b3c6644a302b4a81b6e 100755
--- a/webrtc/tools/compare_videos.py
+++ b/webrtc/tools/compare_videos.py
@@ -48,9 +48,17 @@ def _ParseArgs():
                     help=('The path to where the zxing executable is located. '
                           'If omitted, it will be assumed to be present in the '
                           'PATH with the name zxing[.exe].'))
-  parser.add_option('--stats_file', type='string', default='stats.txt',
+  parser.add_option('--stats_file_ref', type='string', default='stats_ref.txt',
                     help=('Path to the temporary stats file to be created and '
-                          'used. Default: %default'))
+                          'used for the reference video file. '
+                          'Default: %default'))
+  parser.add_option('--stats_file_test', type='string',
+                    default='stats_test.txt',
+                    help=('Path to the temporary stats file to be created and '
+                          'used for the test video file. Default: %default'))
+  parser.add_option('--stats_file', type='string',
+                    default='stats_deprecated.txt',
+                    help=('DEPRECATED and ignored'))

[kjellander_webrtc, 2017/01/04 14:47:07]

I suggest having --stats_file_test read from this value if it's specified +
print a warning during execution that --stats_file is DEPRECATED but that its
value is being used.

If not specified, --stats_file_test should just use its own value.

If we just ignore its value, the test in Chrome will break when we land this CL
and it gets rolled in. 

[kjellander_webrtc, 2017/01/09 12:12:19]

You raised the point off-review that the test won't fail if the value of the
flag is ignored. You're absolutely right about that, the test don't use the
stats file for anything (but it's contents might be interesting for debugging a
failed test etc, so it's still interesting). As long as the flag exists until
the call to compare_video.py in
https://cs.chromium.org/chromium/src/chrome/browser/media/webrtc/webrtc_video...
is updated, this is fine.

I think we should just remove the passing of the flag entirely in that test,
since it will then just use the default value.

   parser.add_option('--yuv_frame_width', type='int', default=640,
                     help='Width of the YUV file\'s frames. Default: %default')
   parser.add_option('--yuv_frame_height', type='int', default=480,
@@ -74,6 +82,40 @@ def _ParseArgs():
                  options.frame_analyzer)
   return options
 
+def _DevNull():
+  """On Windows, sometimes the inherited stdin handle from the parent process
+  fails. Workaround this by passing null to stdin to the subprocesses commands.
+  This function can be used to create the null file handler.
+  """
+  return open(os.devnull, 'r')
+
+def DecodeBarcodesInVideo(options, path_to_decoder, video, stat_file):
+  # Run barcode decoder on the test video to identify frame numbers.
+  png_working_directory = tempfile.mkdtemp()
+  cmd = [
+    sys.executable,
+    path_to_decoder,
+    '--yuv_file=%s' % video,
+    '--yuv_frame_width=%d' % options.yuv_frame_width,
+    '--yuv_frame_height=%d' % options.yuv_frame_height,
+    '--stats_file=%s' % stat_file,
+    '--png_working_dir=%s' % png_working_directory,
+  ]
+  if options.zxing_path:
+    cmd.append('--zxing_path=%s' % options.zxing_path)
+  if options.ffmpeg_path:
+    cmd.append('--ffmpeg_path=%s' % options.ffmpeg_path)
+
+
+  barcode_decoder = subprocess.Popen(cmd, stdin=_DevNull(),
+                                     stdout=sys.stdout, stderr=sys.stderr)
+  barcode_decoder.wait()
+
+  shutil.rmtree(png_working_directory)
+  if barcode_decoder.returncode != 0:
+    print 'Failed to run barcode decoder script.'
+    return 1
+  return 0
 
 def main():
   """The main function.
@@ -97,32 +139,11 @@ def main():
     path_to_decoder = os.path.join(SCRIPT_DIR, 'barcode_tools',
                                    'barcode_decoder.py')
 
-  # On Windows, sometimes the inherited stdin handle from the parent process
-  # fails. Work around this by passing null to stdin to the subprocesses.
-  null_filehandle = open(os.devnull, 'r')
-
-  # Run barcode decoder on the test video to identify frame numbers.
-  png_working_directory = tempfile.mkdtemp()
-  cmd = [
-    sys.executable,
-    path_to_decoder,
-    '--yuv_file=%s' % options.test_video,
-    '--yuv_frame_width=%d' % options.yuv_frame_width,
-    '--yuv_frame_height=%d' % options.yuv_frame_height,
-    '--stats_file=%s' % options.stats_file,
-    '--png_working_dir=%s' % png_working_directory,
-  ]
-  if options.zxing_path:
-    cmd.append('--zxing_path=%s' % options.zxing_path)
-  if options.ffmpeg_path:
-    cmd.append('--ffmpeg_path=%s' % options.ffmpeg_path)
-  barcode_decoder = subprocess.Popen(cmd, stdin=null_filehandle,
-                                     stdout=sys.stdout, stderr=sys.stderr)
-  barcode_decoder.wait()
-
-  shutil.rmtree(png_working_directory)
-  if barcode_decoder.returncode != 0:
-    print 'Failed to run barcode decoder script.'
+  if DecodeBarcodesInVideo(options, path_to_decoder,
+                           options.ref_video, options.stats_file_ref) != 0:
+    return 1
+  if DecodeBarcodesInVideo(options, path_to_decoder,
+                           options.test_video, options.stats_file_test) != 0:
     return 1
 
   # Run frame analyzer to compare the videos and print output.
@@ -131,11 +152,12 @@ def main():
     '--label=%s' % options.label,
     '--reference_file=%s' % options.ref_video,
     '--test_file=%s' % options.test_video,
-    '--stats_file=%s' % options.stats_file,
+    '--stats_file_ref=%s' % options.stats_file_ref,
+    '--stats_file_test=%s' % options.stats_file_test,
     '--width=%d' % options.yuv_frame_width,
     '--height=%d' % options.yuv_frame_height,
   ]
-  frame_analyzer = subprocess.Popen(cmd, stdin=null_filehandle,
+  frame_analyzer = subprocess.Popen(cmd, stdin=_DevNull(),
                                     stdout=sys.stdout, stderr=sys.stderr)
   frame_analyzer.wait()
   if frame_analyzer.returncode != 0: