Making webrtc video quality test page generic

chrome/test/data/webrtc/video_extraction.js

Index: chrome/test/data/webrtc/video_extraction.js
diff --git a/chrome/test/data/webrtc/video_extraction.js b/chrome/test/data/webrtc/video_extraction.js
index 93cf9e7fc49312342d6aab5981f2dd9bf666a273..ced2f925ecd5148e36d3eb73da08a229403b6cf5 100644
--- a/chrome/test/data/webrtc/video_extraction.js
+++ b/chrome/test/data/webrtc/video_extraction.js
@@ -11,6 +11,30 @@
 var gVideoId = 'remote-view';
 
 /**
+ * The ID of the canvas tag from which the frames are captured.
+ * @private
+ */
+var gCanvasId = 'remote-canvas';
+
+/**
+ * The height of the video elements.
+ * @private
+ */
+var gVideoHeight = 0;
+
+/**
+ * The width of the video elements.
+ * @private
+ */
+var gVideoWidth = 0;
+
+/**
+ * The height of the canvas.
+ * @private
+ */
+var gCanvasHeight = 0;
+
+/**
  * Counts the number of frames that have been captured. Used in timeout
  * adjustments.
  * @private
@@ -78,25 +102,62 @@ window.onload = function() {
 /**
  * Starts the frame capturing.
  *
- * @param {Number} The width of the video/canvas area to be captured.
- * @param {Number} The height of the video area to be captured.
- * @param {Number} The height of the canvas where we put the video frames.
+ * @param {String} The ID of the video tag from which the height and width
+ *                 parameters are to be extracted

[phoglund_chromium, 2014/04/28 08:51:43]

Nit: End with .

[amogh.bihani, 2014/04/28 09:47:24]

Done.

  * @param {Number} The frame rate at which we would like to capture frames.
  * @param {Number} The duration of the frame capture in seconds.
  */
-function startFrameCapture(width, height, canvas_height, frame_rate, duration){
+function startFrameCapture(videoTagName, frame_rate, duration) {
   gFrameCaptureInterval = 1000/frame_rate;
   gCaptureDuration = 1000 * duration;
+  extractParameters(videoTagName);

[phoglund_chromium, 2014/04/28 08:51:43]

Adding globals for height and width is a step backwards. Keep the parameters to
shoot() but pass in video.videoWidth as the width, etc.

What about something like this:

var video = document.getElementById(videoTagName);
if (!video)
  failTest('Did not find element ' + videoTagName + '.');
if (video.videoWidth == 0 || video.videoHeight == 0)
  failTest('Trying to capture from ' + videoTagName + ' but it is not playing
video');

...

setTimeout(function() { shoot(video.videoWidth, video.videoHeight); },
           gFrameCaptureInterval);

You can get rid of the whole canvas height thing. I haven't seen us capture
anything else than the whole feed, so keeping it seems like unnecessary
flexibility.

[amogh.bihani, 2014/04/28 09:47:24]

Done.

 
-  console.log('Received width is: ' + width + ', received height is: ' + height
-              + ', capture interval is: ' + gFrameCaptureInterval +
-              ', duration is: ' + gCaptureDuration);
+  console.log('Capture interval is: ' + gFrameCaptureInterval +
+              ', Duration is: ' + gCaptureDuration);
   gStartOfTime = new Date().getTime();
-  setTimeout(function() { shoot(width, height, canvas_height); },
+  setTimeout(function() { shoot(); },
              gFrameCaptureInterval);
 }
 
 /**
+ * Extracts the height and the width of the video tag streaming the video
+ * and sets those parameters to other tags.
+ *
+ * @param {String} The ID of the video tag from which the height and width
+ *                 parameters are to be extracted
+ */
+function extractParameters(videoTagName) {
+  var localView = document.getElementById(videoTagName);
+  gVideoHeight = localView.videoHeight;
+  gVideoWidth = localView.videoWidth;
+  // TODO(amogh.bihani): Change this when we have testcase where we capture only
+  // the bar code and not the whole frame.
+  gCanvasHeight = gVideoHeight;
+
+  if (gVideoHeight == undefined || gVideoWidth == undefined ||

[phoglund_chromium, 2014/04/28 08:51:43]

This error checking for undefined will not really do anything, as
localView.videoHeight will throw if localView is null. Write the error checks
like I suggested above instead.

[amogh.bihani, 2014/04/28 09:47:24]

Done.

+      gVideoHeight == 0 || gVideoWidth == 0) {
+    throw failTest('Could not extract video stream size');
+  }
+
+  console.log('Received height is: ' + gVideoHeight + ', received width is: ' +
+              gVideoWidth);
+
+  var remoteView = document.getElementById(gVideoId);

[phoglund_chromium, 2014/04/28 08:51:43]

This and 149-157 really seems to have no function but make the web page look
nicer. This code doesn't belong in video_extraction.js and it certainly doesn't
belong in a function called extractParameters. Just try deleting the code. If
you still feel you need it, put it somewhere else like in the video quality test
HTML page (for instance, when you invoke startFrameCapture(...), also invoke
makeWebpageLookPretty(...) and put the prettifying code in there).

[amogh.bihani, 2014/04/28 09:47:24]

Done.

+  remoteView.height = gVideoHeight;
+  remoteView.width = gVideoWidth;
+
+  var remoteCanvas = document.getElementById(gCanvasId);

[phoglund_chromium, 2014/04/28 08:51:43]

I'm pretty sure you don't need to set the dimensions of the canvas; I think it
will figure out how big it needs to be by itself.

[amogh.bihani, 2014/04/28 09:47:24]

Done.

+  remoteCanvas.height = gCanvasHeight;
+  remoteCanvas.width = gVideoWidth;
+
+  var outputDiv = document.getElementById("output");
+  outputDiv.style.display = "inline-block";
+  outputDiv.style.position = "relative";
+  outputDiv.style.height = gVideoHeight;
+  outputDiv.style.width = gVideoWidth;
+}
+
+/**
  * Captures an image frame from the provided video element.
  *
  * @param {Video} video HTML5 video element from where the image frame will
@@ -107,7 +168,7 @@ function startFrameCapture(width, height, canvas_height, frame_rate, duration){
  * @return {Canvas}
  */
 function capture(video, width, height) {
-  var canvas = document.getElementById('remote-canvas');
+  var canvas = document.getElementById(gCanvasId);

[phoglund_chromium, 2014/04/28 08:51:43]

This is a good idea (we should get rid of the hard-coded canvas name though and
create the canvas on the fly, but you don't have to do that this patch).

   var ctx = canvas.getContext('2d');
   ctx.drawImage(video, 0, 0, width, height);
   return canvas;
@@ -118,12 +179,8 @@ function capture(video, width, height) {
  * the current frame from the video and extracts the data from it. Then it saves
  * it in the frames array and adjusts the capture interval (timers in JavaScript
  * aren't precise).
- *
- * @param {Number} The width of the video/canvas area to be captured.
- * @param {Number} The height of the video area to be captured.
- * @param {Number} The height of the canvas where we put the video frames.
  */
-function shoot(width, height, canvas_height){
+function shoot() {

[phoglund_chromium, 2014/04/28 08:51:43]

Like I said above, keep the parameters for width and height, throw away
canvas_height and delete 205-208 (which isn't used anyway).

[amogh.bihani, 2014/04/28 09:47:24]

Done.

   // The first two captured frames have big difference between the ideal time
   // interval between two frames and the real one. As a consequence this affects
   // enormously the interval adjustment for subsequent frames. That's why we
@@ -137,17 +194,17 @@ function shoot(width, height, canvas_height){
     gFrames.pop();
   }
   var video  = document.getElementById(gVideoId);
-  var canvas = capture(video, width, height);
+  var canvas = capture(video, gVideoWidth, gVideoHeight);
 
   // Extract the data from the canvas.
   var ctx = canvas.getContext('2d');
   var img;
-  if (height == canvas_height) {
+  if (gVideoHeight == gCanvasHeight) {
     // We capture the whole video frame.
-    img = ctx.getImageData(0, 0, width, height);
+    img = ctx.getImageData(0, 0, gVideoWidth, gVideoHeight);
   } else {
-    // We capture only the barcode (canvas_height is the height of the barcode).
-    img = ctx.getImageData(0, 0, width, canvas_height);
+    // We capture only the barcode (gCanvasHeight is the height of the barcode).
+    img = ctx.getImageData(0, 0, gVideoWidth, gCanvasHeight);
   }
   gFrames.push(img.data.buffer);
   gFrameCounter++;
@@ -160,7 +217,7 @@ function shoot(width, height, canvas_height){
 
   if (real_time_elapsed < gCaptureDuration) {
     // If duration isn't over shoot again
-    setTimeout(function() { shoot(width, height, canvas_height); },
+    setTimeout(function() { shoot(); },
                gFrameCaptureInterval - diff);
   } else {  // Else reset gFrameCounter and send the frames
     dDoneFrameCapturing = true;