kacpergalka
kacpergalka

Reputation: 263

Bad performance of videos loop

I am working on a video slider, in which following video starts just before the previous ends - in a loop. At the same time i need to update a progress bar of each one. The code below works, but Chrome is burning my macbook. Am I doing sth wrong? How can I improve the performance? There are free slides, and videos has no loop="true" attribute. I just start the slider by playing the first video.

videos.on('timeupdate', function () {
    var progress = this.currentTime / this.duration * 100 + '%';
    $(this).closest('.slide').find('.js-video-progress').width(progress);
    if ((this.currentTime * 1000 + 1000) > this.duration * 1000) {
        if ($(this).closest('.slide').next('.slide').length) {
            $(this).closest('.slide').next('.slide').addClass('is-swiped').find('video')[0].play();
        } else {
            $('.slide').eq(0).addClass('is-swiped').find('video')[0].play();
        }
    }
});

Upvotes: 0

Views: 126

Answers (1)

Kaiido
Kaiido

Reputation: 136698

timeupdate may fire at an high rate (possibly even faster than your screen refresh rate), so whatever you do in there must be as light as possible.

jQuery's methods do a lot of things every time you call them, and always return new objects which will soon pollute all your memory, forcing the GarbageCollector to come in every time. So don't call twice the same function and instead store the result in a variable.

Here is a slightly better approach storing each reused object in the handler itself:

videos.on('timeupdate', function () {
    var time = this.currentTime, // it's a getter function, store
    var dur = this.duration;
    var progress = time / duration * 100 + '%';
    var $this = $(this); // store
    var $slide = $this.closest('.slide');
    var $progress = $slide.find('.js-video-progress');
    var $slide_next;
    $progress.width(progress);
    if ((time * 1000 + 1000) > dur * 1000) {
        $slide_next = $slide.next('.slide');
        if ($slide_next.length) {
           $slide_next
              .addClass('is-swiped')
              .find('video')[0]
              .play();
        } else { 
            $slide.eq(0)
              .addClass('is-swiped')
              .find('video')[0]
              .play();
        }
    }
});

But personally, I think would even go deeper, at the risk of being verbose, and store all the needed objects in a static object that I would just lookup in the handlers:

videos.each(function attach_data() {
  var $this = $(this);
  var $slide = $this.closest('.slide');
  var $progress = $slide.find('.js-video-progress');
  var $video = $slide.eq(0).find('video');
  var $slide_next = $slide.next('.slide');
  var $next_video = $slide_next.find('video');
  $this.data('$tree', {
    slide: $slide.eq(0),
    progress: $progress,
    video: $video,
    slide_next: $slide_next,
    video_next: $video_next
  })
})
.on('timeupdate', function () {
    var time = this.currentTime;
    var dur = this.duration;
    var progress = time / duration * 100 + '%';
    // retrieve our stored object
    var $tree = $(this).data('$tree');
    $tree.progress.width(progress);
    if ((time * 1000 + 1000) > dur * 1000) {
        if ($tree.slide_next.length) {
           $tree.slide_next.addClass('is-swiped');
           $tree.next_video[0].play(); // you may want to add a check the <video> really exists
        } else { 
           $tree.slide.addClass('is-swiped');
           $tree.video[0].play();
        }
    }
});

Upvotes: 1

Related Questions