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..2542de3a8463fbee8c0e529a6bdf278841c814c7 100755
--- a/webrtc/tools/compare_videos.py
+++ b/webrtc/tools/compare_videos.py
@@ -48,7 +48,11 @@ 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'))
+  parser.add_option('--stats_file_test', type='string',
+                    default='stats_test.txt',
                     help=('Path to the temporary stats file to be created and '
                           'used. Default: %default'))
   parser.add_option('--yuv_frame_width', type='int', default=640,
@@ -74,10 +78,36 @@ def _ParseArgs():
                  options.frame_analyzer)
   return options
 
+def make_stat_file(options, path_to_decoder, null_filehandle, video, stat_file):

[phoglund, 2016/12/08 09:55:43]

The convention in these files is MakeStatFile (main is always lowercase by
convention though).

[phoglund, 2016/12/08 09:55:43]

Passing null_filehandle seems silly. This function doesn't care if it's the null
filehandle or something else; I'd just call it stdin_handle.

[phoglund, 2016/12/08 09:55:43]

Sure, this function makes a stat file, but it's better to allude to what this
function does and why in the naming. What actually happens is that we take a
file in and decode the barcodes in it, so call it DecodeBarcodesInVideo or
something instead.

[mandermo, 2016/12/08 18:08:30]

From the comment for null_filehandle, I understand that it very much matters
that it is a null file handle and not any handle. I have made a copy of the
declaration of the null_filehandle variable and put it to
make_stat_file/DecodeBarcodesInVideo and removed it as a parameter. That caused
some more duplication, not sure what is best.

[mandermo, 2016/12/08 18:08:30]

Renamed to DecodeBarcodesInVideo

[mandermo, 2016/12/08 18:08:30]

Done.

+  # 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=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.'
+    return 1
+  return 0
 
 def main():
   """The main function.
 
+  TODO(mandermo) update
   A simple invocation is:
   ./webrtc/tools/barcode_tools/compare_videos.py
   --ref_video=<path_and_name_of_reference_video>
@@ -101,28 +131,11 @@ def main():
   # 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 make_stat_file(options, path_to_decoder, null_filehandle,
+      options.ref_video, options.stats_file_ref) != 0:
+    return 1
+  if make_stat_file(options, path_to_decoder, null_filehandle,
+      options.test_video, options.stats_file_test) != 0:
     return 1
 
   # Run frame analyzer to compare the videos and print output.
@@ -131,7 +144,8 @@ 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,
   ]