0

Quick background to this project: Have a list of 4 videos play on a kiosk-type machine that is running the chrome web browser. Allow users to select a video to play, and then continue looping through the list, allowing a user to select a video to play at will.

So, I have the JavaScript all written to kick off this process, and it occurred to me that the recursive nature of the way I have things written may be a bad way to go ... even though it seems like the most logical way to do it.

Is this thing going to overflow the stack at some point and throw an error if I leave it going for long stretches of time? The basic idea is that it should loop on it's own until some user intervention.

Here's my JavaScript code for playing videos (in my html I have four buttons that are set to call the function with the corresponding video numbers):

function playVideo(video_number) {

    //remove all previous event listeners for the video element node
    var el = document.getElementById('video'),
        elClone = el.cloneNode(true);
    el.parentNode.replaceChild(elClone, el);

    //define the array that has information about our videos.
    //array defines the following:
    //[0]=path to video, [1]=video title, [2]=color code for visual, 
    //[3]=next video to play in list
    var video_array = [
        ['./video1.mp4', 'VIDEO 1 TITLE', 'red', 1], //
        ['./video2.mp4', 'VIDEO 2 TITLE', 'green', 2], //
        ['./video3.mp4', 'VIDEO 3 TITLE', 'blue', 3], //
        ['./video4.mp4', 'VIDEO 4 TITLE', 'orange',  0], //
    ];

    //get all the elements we're going to be using
    var t = document.getElementById('title');
    var v = document.getElementById('video');
    var s = document.getElementById('source');
    var p = document.getElementById('progress');            

    t.innerHTML = video_array[video_number][1]; //set the title of the video
    t.style.color = video_array[video_number][2];

    p.className = 'progress-bar ' + 'btn' + (video_number + 1);

    v.src = video_array[video_number][0];
    v.load();
    v.play();

    //add event listener for the progress bar update
    v.addEventListener('timeupdate', function() {
        p.style.width = ( (v.currentTime / v.duration) * 100 ).toFixed(4) + '%';
    }, false);

    //do some stuff when the video ends
    v.addEventListener('ended', function() {

        //remove the listener for the progress bar
        v.removeEventListener('timeupdate', function() { 
            console.log('removed the progress bar listener')
        }, false);

        console.log('video ' + video_number + ' ended');

        //play the next video in the list ...
        playVideo(video_array[video_number][3]);

    }, false);

} //end function playVideo()
ray_voelker
  • 495
  • 3
  • 12
  • 3
    There's no recursion there. The event handler will be called after the "playVideo" function call has finished. – Pointy Feb 04 '15 at 20:35
  • 2
    I have been working in web development for a few years, and have yet to see a browser actually explode. – ssube Feb 04 '15 at 20:38
  • 2
    Agreed, this is not recursion in the pure stack sense. However, there is a recursion-like pattern here that definitely could cause a problem. You keep adding event handlers to the same element without ever removing old ones. And they are closures. Closures with references to DOM nodes. I forsee memory issues here. Please performance test this code carefully. – Brandon Feb 04 '15 at 20:38
  • 1
    your code to "remove all previous event listeners for the video element node" only replaces a node with a new deep cloned version. you need to actually detach the event listeners before replacing the node to avoid memory leaks. – mothmonsterman Feb 04 '15 at 20:39

0 Answers0