Addressing review comments.

google · Jan 7, 2015 · 0920c19 · 0920c19
1 parent b2a8afa
commit 0920c19
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 25 deletions.
diff --git a/bin/compare_json b/bin/compare_json
@@ -17,7 +17,7 @@
 #
 # This generates a JSON structure containing the information about scores.
 # The structure is as follows:
-# - A list of codecs that are being compared, with longnames
+# - A list of codecs that are being compared, with long names
 # - A summary list of files being compared and their relative performance
 # - A dictionary with key=codec shortname containing
 #   - A list of entries, each consisting of
@@ -36,6 +36,7 @@ import pick_codec
 import score_tools
 import visual_metrics
 
+
 def main():
   parser = argparse.ArgumentParser()
   parser.add_argument('--score', action='store_true', default=False)

diff --git a/lib/encoder.py b/lib/encoder.py
@@ -447,7 +447,7 @@ def Execute(self, bitrate, videofile, workdir):
 
   def EncodeCommandLine(self, bitrate, videofile, workdir):
     return self.context.codec.EncodeCommandLine(
-      self.parameters, bitrate, videofile, workdir)
+        self.parameters, bitrate, videofile, workdir)
 
   def VerifyEncode(self, bitrate, videofile, workdir):
     """Returns true if a new encode of the file gives exactly the same file."""

diff --git a/lib/score_tools_unittest.py b/lib/score_tools_unittest.py
@@ -46,7 +46,7 @@ def test_ScoreCpuPsnr(self):
               'encode_cputime': 0.7}
     self.assertEqual(10.0, score_tools.ScoreCpuPsnr(100, result))
     # Test 1.01 percent overrun in bitrate.
-    self.assertAlmostEqual(10.0 - 10*(100.0/99 - 1),
+    self.assertAlmostEqual(10.0 - 10 * (100.0 / 99 - 1),
                            score_tools.ScoreCpuPsnr(99, result))
     result['encode_cputime'] = 1.1
     self.assertAlmostEqual(0.0, score_tools.ScoreCpuPsnr(100, result))

diff --git a/lib/visual_metrics.py b/lib/visual_metrics.py
@@ -76,6 +76,7 @@ def bdsnr(metric_set1, metric_set2):
     avg_diff = 0.0
   return avg_diff
 
+
 def bdrate(metric_set1, metric_set2):
   """
   BJONTEGAARD    Bjontegaard metric calculation
@@ -214,6 +215,7 @@ def GraphBetter(metric_set1_sorted, metric_set2_sorted, use_set2_as_base):
 
   return avg
 
+
 def DataSetBetter(metric_set1, metric_set2, method):
   """
   Compares two data sets and determines which is better and by how
@@ -289,13 +291,20 @@ def ListOneTarget(codecs, rate, videofile, do_score, datatable,
     if my_optimizer.Score(bestsofar) < 0.0:
       return
     if full_results:
+      # Datatable is a dictionary of codec name -> result sets.
+      # Each result set is an array containing result info.
+      # Each result info is a dictionary containing the
+      # target bitrate, the command line, the score and the result.
       (datatable.setdefault(codec_name, {})
           .setdefault(videofile.basename, [])
           .append({'target_bitrate': rate,
                    'encode_command': bestsofar.EncodeCommandLine(),
                    'score': my_optimizer.Score(bestsofar),
                    'result': bestsofar.ResultWithoutFrameData()}))
     else:
+      # Datatable is a dictionary of codec name -> result sets.
+      # Each result set is an array containing result info.
+      # Each result info contains the achieved bitrate and the PSNR.
       (datatable.setdefault(codec_name, {})
           .setdefault(videofile.basename, [])
           .append((bestsofar.result['bitrate'], bestsofar.result['psnr'])))
@@ -315,7 +324,7 @@ def ListMpegResults(codecs, do_score, datatable, score_function=None,
 def ExtractBitrateAndPsnr(datatable, codec, filename, full_results=False):
   if full_results:
     dataset = [(r['result']['bitrate'], r['result']['psnr'])
-                          for r in datatable[codec][filename]]
+               for r in datatable[codec][filename]]
   else:
     dataset = datatable[codec][filename]
   return dataset
@@ -348,9 +357,8 @@ def BuildComparisonTable(datatable, metric, baseline_codec, other_codecs,
     # Read the metric file from each of the directories in our list.
     for this_codec in other_codecs:
 
-      # If there is a metric in this_codec,
-      # calculate the overall difference between it and the baseline
-      # codec's metric
+      # If there is a metric in this_codec, calculate the overall difference
+      # between it and the baseline codec's metric.
       if (this_codec in datatable and filename in datatable[this_codec]
           and filename in datatable[baseline_codec]):
         this_dataset = ExtractBitrateAndPsnr(datatable,
@@ -374,8 +382,9 @@ def BuildComparisonTable(datatable, metric, baseline_codec, other_codecs,
   data.append(row)
   return data
 
+
 def BuildGvizDataTable(datatable, metric, baseline_codec, other_codecs):
-  """Builds a Gviz DataTable giving this metric for the files and codecs"""
+  """Builds a Gviz DataTable giving this metric for the files and codecs."""
 
   description = {"file": ("string", "File")}
   data = BuildComparisonTable(datatable, metric, baseline_codec, other_codecs)
@@ -386,6 +395,7 @@ def BuildGvizDataTable(datatable, metric, baseline_codec, other_codecs):
   gviz_data_table.LoadData(data)
   return gviz_data_table
 
+
 def CrossPerformanceGvizTable(datatable, metric, codecs, criterion,
                               new_style_links=False):
   """Build a square table of codecs and relative performance."""

diff --git a/website/assets/js/codecfuncs.js b/website/assets/js/codecfuncs.js
@@ -14,23 +14,23 @@ function FillInOneTable(url, id) {
     dataType: 'json',
   }).done(function(responseText) {
     var tableStyles = { allowHtml: true,
-                      alternatingRowStyle: false,
-                      showRowNumber: false,
-                      sort: 'disable',
-                      // Width is problematic, cross-device.
-                      width: '50%',
-                      cssClassNames: {
-                        // These map to .rslts-* classes
-                        headerRow: 'rslts-headerRow',
-                        tableRow: 'rslts-tableRow',
-                        oddTableRow: 'rslts-oddTableRow',
-                        selectedTableRow: 'rslts-selectedTableRow',
-                        hoverTableRow: 'rslts-hoverTableRow',
-                        headerCell: 'rslts-headerCell',
-                        tableCell: 'rslts-tableCell',
-                        rowNumberCell: 'rslts-rowNumberCell',
-                      }
-                    };
+			alternatingRowStyle: false,
+			showRowNumber: false,
+			sort: 'disable',
+			// Width is problematic, cross-device.
+			width: '50%',
+			cssClassNames: {
+                            // These map to .rslts-* classes
+                            headerRow: 'rslts-headerRow',
+                            tableRow: 'rslts-tableRow',
+                            oddTableRow: 'rslts-oddTableRow',
+                            selectedTableRow: 'rslts-selectedTableRow',
+                            hoverTableRow: 'rslts-hoverTableRow',
+                            headerCell: 'rslts-headerCell',
+                            tableCell: 'rslts-tableCell',
+                            rowNumberCell: 'rslts-rowNumberCell',
+			}
+                      };
     var data = new google.visualization.DataTable(responseText);
     var table = new google.visualization.Table(
     document.getElementById(id));