Skip to content

Commit

Permalink
initial rework of update function
Browse files Browse the repository at this point in the history
  • Loading branch information
@ScottDowne
ScottDowne committed Oct 22, 2010
1 parent a79c916 commit ac3753c
Showing 1 changed file with 97 additions and 38 deletions.
135 changes: 97 additions & 38 deletions popcorn.js
View file
Expand Up @@ -24,18 +24,46 @@
// The video manager manages a single video element, and all it's commands.
popcorn.VideoManager = function(videoElement) {
this.commandObjects = {};
this.commandsByIn = [];
this.inIndex = 0;
this.commandsByOut = [];
this.outIndex = 0;
this.manifestObjects = {};
this.previousTime = 0;

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

Probably actually best as previousUpdateTime, if the variable is not associated with the update() function in some structural way (which would be better).
this.videoElement = videoElement;
videoElement.videoManager = this;
popcorn.addInstance(this);
videoElement.setAttribute("ontimeupdate", "popcorn.update(this);");
};

// commands are stored in two arrays, one sorted by out time, and the other sorted by in time
popcorn.VideoManager.prototype.addCommand = function(command) {
this.commandObjects[command.id] = command;
this.commandsByIn.push(command);
this.commandsByIn.sort(function(a, b){
return (a.params["in"] - b.params["in"]);
});
this.commandsByOut.push(command);
this.commandsByOut.sort(function(a, b){
return (a.params["out"] - b.params["out"]);
});

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

The above construction will lead to O(n^2 log n) performance for a sequence of adding commands, if you assume that the sort is O(n log n). Instead, you could either a) traverse the array looking for the correct insertion point, and use splice (which would be O(n^2)) or, even better, just store all the added commands unsorted, and have a "sorted" boolean. It is initially unset, and is unset every time a command is added. Then, check the sorted boolean at the start of the update() function and sort the arrays then if necessary. This construction would be O(n log n), because you sort only once. In the common case, this saves lots of time in addCommand() and removeCommand() at the cost of a small use of time in the first call to update(), which I think is an OK trade-off.
};

popcorn.VideoManager.prototype.removeCommand = function(command) {
// loop through in and out commands, check id, and remove accordingly
for (var i = 0, il = this.commandsByIn.length; i < il; i++) {
if (command.id === this.commandsByIn[i].id) {
this.commandsByIn.splice(i, 1);
break;
}
}
for (var o = 0, ol = this.commandsByOut.length; o < ol; o++) {
if (command.id === this.commandsByOut[o].id) {
this.commandsByOut.splice(o, 1);
break;
}
}

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

Removal is also O(n^2 log n). You need to search the array and splice().
delete this.commandObjects[command.id];
};

Expand Down Expand Up @@ -63,52 +91,83 @@
popcorn.VideoManager.prototype.loaded = function() {};

var inactiveTarget = {};

// Update is called on the video every time it's time changes.
var callOut = function(command) {
if (inactiveTarget[command.params.target] <= 0 || --inactiveTarget[command.params.target] <= 0) {
$("#" + command.params.target + " .inactive").fadeIn(500);
}
command.onOut();
command.extension.onOut();
};
var callIn = function(command) {
$("#" + command.params.target + " .inactive").hide();
inactiveTarget[command.params.target]++;

if (command.params.target) {
$("#" + command.params.target + " .overlay").show().fadeOut(500);
}
if (typeof command.flash === "undefined") {
var section = $(command.target).parents('section');
if (!section.hasClass('hover')) {
section.addClass('hover');
section.attr('hoveron', $('video')[0].currentTime);
}
}
command.onIn();
command.extension.onIn();
};

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

Should all this stuff be encapsulated in the command object? So you call command.onIn() and it does all of this?

// Update is called on the video every time its time changes.
// commands are stored in sorted in and out arrays
// inIndex and outIndex are values for the last know array positions
// These indexes travel up and down the sorted in and out arrays
popcorn.update = function(vid) {
var t = vid.currentTime,
commandObject = {}; // Loops through all commands in the manager, preloading data, and calling onIn() or onOut().

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

The code below would be a lot clearer if the variable "t" were named "currentTime".
for (var i in vid.videoManager.commandObjects) {
if (vid.videoManager.commandObjects.hasOwnProperty(i)) {
commandObject = vid.videoManager.commandObjects[i];
if (commandObject.running && (commandObject.params["in"] > t || commandObject.params["out"] < t)) {
commandObject.running = false;
if (inactiveTarget[commandObject.params.target] <= 0 || --inactiveTarget[commandObject.params.target] <= 0) {
$("#" + commandObject.params.target + " .inactive").fadeIn(500);
}
commandObject.onOut();
commandObject.extension.onOut();
commandsByOut = vid.videoManager.commandsByOut,
commandsByIn = vid.videoManager.commandsByIn;
if (vid.videoManager.previousTime <= t) { // seeking forwards
while (commandsByOut[vid.videoManager.outIndex].params["out"] < t) {
callOut(commandsByOut[vid.videoManager.outIndex]);
// check if falling off end of array
if (vid.videoManager.outIndex < commandsByOut.length-1) {
vid.videoManager.outIndex++;
} else {
return; // exits update();

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

It's bad practice to return() in multiple places, and in the middle of functions. Break out of the loop instead.

However, this raises the question of function invariants (things which are always supposed to be true, and which you use to check correctness). I thought of the invariant as being "when the function ends, each pointer points at the first item in the array whose out time is strictly greater than the current time." However, this is not necessarily true if the pointer is pointing at the last item.

This consideration of invariants reveals a bug: once the pointer has reached the last command, onOut() will be called on it every time update() is called. And onIn() will be called on it every time update() is called until the out time has passed.

One solution to this would be to have a dummy "terminating" command on the end of the list, with an in and out time of 999 hours or whatever, and whose onIn() and onOut() functions did nothing (and would never be called anyway). The pointers can then point at that when the time of the video has advanced past all the commands. This is a bit like putting an elephant in Cairo:
http://en.wikipedia.org/wiki/Elephant_in_Cairo

This means you don't have to check for falling off the end of the array, because you could never go past the last command. So it also simplifies the code.
}
}
}
for (var j in vid.videoManager.commandObjects) {
if (vid.videoManager.commandObjects.hasOwnProperty(j)) {
commandObject = vid.videoManager.commandObjects[j];
if (!commandObject.loaded && (commandObject.params["in"] - 5) < t && commandObject.params["out"] > t) {
commandObject.loaded = true;
commandObject.preload();
while (commandsByIn[vid.videoManager.inIndex].params["in"] < t) {

This comment has been minimized.

Sign in to view

Copy link
@gerv

gerv Oct 25, 2010

Surely <= rather than < ?
if (commandsByIn[vid.videoManager.inIndex].params["out"] > t) {
callIn(commandsByIn[vid.videoManager.inIndex]);
}
if (!commandObject.running && commandObject.params["in"] < t && commandObject.params["out"] > t) {
commandObject.running = true;

$("#" + commandObject.params.target + " .inactive").hide();
inactiveTarget[commandObject.params.target]++;

if (commandObject.params.target) {
$("#" + commandObject.params.target + " .overlay").show().fadeOut(500);
}
if (typeof commandObject.flash === "undefined") {
var section = $(commandObject.target).parents('section');
if (!section.hasClass('hover')) {
section.addClass('hover');
section.attr('hoveron', $('video')[0].currentTime);
}
}
commandObject.onIn();
commandObject.extension.onIn();
// check if falling off end of array
if (vid.videoManager.inIndex < commandsByIn.length-1) {
vid.videoManager.inIndex++;
} else {
return; // exits update();
}
}
} else { // seeking backwards
while (commandsByIn[vid.videoManager.inIndex].params["in"] > t) {
callOut(commandsByIn[vid.videoManager.inIndex]);
// check if falling off front of array
if (vid.videoManager.inIndex > 0) {
vid.videoManager.inIndex--;
} else {
return; // exits update();
}
}
while (commandsByOut[vid.videoManager.outIndex].params["out"] > t) {
if (commandsByOut[vid.videoManager.outIndex].params["in"] < t) {
callIn(commandsByOut[vid.videoManager.outIndex]);
}
// check if falling off front of array
if (vid.videoManager.outIndex > 0) {
vid.videoManager.outIndex--;
} else {
return; // exits update();
}
}
}
vid.videoManager.previousTime = t;
};

// Store VideoManager instances
Expand Down

0 comments on commit ac3753c

Please sign in to comment.