From 48edd4a2bf5c260d0a71b395c0d1712dbb308fa2 Mon Sep 17 00:00:00 2001 From: Valera Rozuvan Date: Tue, 4 Jun 2013 18:08:46 +0300 Subject: [PATCH 1/2] Experimenting with optimal event attachment strategy. --- .../xmodule/js/src/videoalpha/display.coffee | 22 +++++++++++++++++++ .../videoalpha/display/video_player.coffee | 3 +-- .../display/video_progress_slider.coffee | 6 ++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee index a27362b094..0e52180ce6 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee @@ -83,6 +83,19 @@ class @VideoAlpha embed: -> @player = new VideoPlayerAlpha video: this + @attachEventDispatchToFunctions + onPlay: 'play_video' + onPause: 'pause_video' + + attachEventDispatchToFunctions: (funcList) -> + $.each funcList, (funcName, eventName) => + @player[funcName] = @attachEventDispatch(@player[funcName], eventName) if @player.hasOwnProperty(funcName) + + attachEventDispatch: (func, eventName) -> + => + @log eventName + func.apply this, arguments + fetchMetadata: (url) -> @metadata = {} $.each @videos, (speed, url) => @@ -92,6 +105,15 @@ class @VideoAlpha @metadata[@youtubeId()].duration log: (eventName)-> + console.log 'log' + console.log 'this = ', this + console.log + id: @id + code: @youtubeId() + currentTime: @player.currentTime + speed: @speed + console.log '' + logInfo = id: @id code: @youtubeId() diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee index 1b761594de..53249c73da 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee @@ -184,7 +184,6 @@ class @VideoPlayerAlpha extends SubviewAlpha @caption.pause() onPlay: => - @video.log 'play_video' unless @player.interval @player.interval = setInterval(@update, 200) if @video.show_captions is true @@ -193,7 +192,6 @@ class @VideoPlayerAlpha extends SubviewAlpha @progressSlider.play() onPause: => - @video.log 'pause_video' clearInterval(@player.interval) @player.interval = null if @video.show_captions is true @@ -206,6 +204,7 @@ class @VideoPlayerAlpha extends SubviewAlpha @caption.pause() onSeek: (event, time) => + console.log 'old time = ' + @currentTime + ', new time = ' + time @player.seekTo(time, true) if @isPlaying() clearInterval(@player.interval) diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee index 19eff226fb..b12041d9c6 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee @@ -6,7 +6,11 @@ class @VideoProgressSliderAlpha extends SubviewAlpha @slider = @el.slider range: 'min' change: @onChange - slide: @onSlide + + # We don't want to attach to 'slide' event because we already have 'change' event. + # If we have two events, then callback will be triggered twice, sending misinformation + # to the server. + # slide: @onSlide stop: @onStop @buildHandle() From 1956020f5c7ca4d2e7e1c90f4cb7dfa68343fe65 Mon Sep 17 00:00:00 2001 From: Valera Rozuvan Date: Wed, 5 Jun 2013 16:50:11 +0300 Subject: [PATCH 2/2] Overhaul of event log of load, play, pause, seek, speed-change events. --- .../xmodule/js/src/videoalpha/display.coffee | 33 +++++-------------- .../videoalpha/display/video_caption.coffee | 2 +- .../videoalpha/display/video_player.coffee | 20 +++++++++-- .../display/video_progress_slider.coffee | 7 ++-- 4 files changed, 28 insertions(+), 34 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee index 0e52180ce6..5f68e63c98 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display.coffee @@ -83,19 +83,6 @@ class @VideoAlpha embed: -> @player = new VideoPlayerAlpha video: this - @attachEventDispatchToFunctions - onPlay: 'play_video' - onPause: 'pause_video' - - attachEventDispatchToFunctions: (funcList) -> - $.each funcList, (funcName, eventName) => - @player[funcName] = @attachEventDispatch(@player[funcName], eventName) if @player.hasOwnProperty(funcName) - - attachEventDispatch: (func, eventName) -> - => - @log eventName - func.apply this, arguments - fetchMetadata: (url) -> @metadata = {} $.each @videos, (speed, url) => @@ -104,21 +91,17 @@ class @VideoAlpha getDuration: -> @metadata[@youtubeId()].duration - log: (eventName)-> - console.log 'log' - console.log 'this = ', this - console.log - id: @id - code: @youtubeId() - currentTime: @player.currentTime - speed: @speed - console.log '' - + log: (eventName, data)-> + # Default parameters that always get logged. logInfo = id: @id code: @youtubeId() - currentTime: @player.currentTime - speed: @speed + + # If extra parameters were passed to the log. + if data + $.each data, (paramName, value) -> + logInfo[paramName] = value + if @videoType is "youtube" logInfo.code = @youtubeId() else logInfo.code = "html5" if @videoType is "html5" diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.coffee index b0adfbfd81..ae64194c55 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.coffee @@ -120,7 +120,7 @@ class @VideoCaptionAlpha extends SubviewAlpha seekPlayer: (event) => event.preventDefault() time = Math.round(Time.convert($(event.target).data('start'), '1.0', @currentSpeed) / 1000) - $(@).trigger('seek', time) + $(@).trigger('caption_seek', time) calculateOffset: (element) -> @captionHeight() / 2 - element.height() / 2 diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee index 53249c73da..8d251cc1f8 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.coffee @@ -24,9 +24,9 @@ class @VideoPlayerAlpha extends SubviewAlpha if @video.videoType is 'youtube' $(@qualityControl).bind('changeQuality', @handlePlaybackQualityChange) if @video.show_captions is true - $(@caption).bind('seek', @onSeek) + $(@caption).bind('caption_seek', @onSeek) $(@speedControl).bind('speedChange', @onSpeedChange) - $(@progressSlider).bind('seek', @onSeek) + $(@progressSlider).bind('slide_seek', @onSeek) if @volumeControl $(@volumeControl).bind('volumeChange', @onVolumeChange) $(document).keyup @bindExitFullScreen @@ -96,6 +96,7 @@ class @VideoPlayerAlpha extends SubviewAlpha at: 'top center' onReady: (event) => + @video.log 'load_video' if @video.videoType is 'html5' @player.setPlaybackRate @video.speed unless onTouchBasedDevice() @@ -184,6 +185,8 @@ class @VideoPlayerAlpha extends SubviewAlpha @caption.pause() onPlay: => + @video.log 'play_video', + currentTime: @currentTime unless @player.interval @player.interval = setInterval(@update, 200) if @video.show_captions is true @@ -192,6 +195,8 @@ class @VideoPlayerAlpha extends SubviewAlpha @progressSlider.play() onPause: => + @video.log 'pause_video', + currentTime: @currentTime clearInterval(@player.interval) @player.interval = null if @video.show_captions is true @@ -204,7 +209,10 @@ class @VideoPlayerAlpha extends SubviewAlpha @caption.pause() onSeek: (event, time) => - console.log 'old time = ' + @currentTime + ', new time = ' + time + @video.log 'seek_video', + old_time: @currentTime + new_time: time + type: event.type @player.seekTo(time, true) if @isPlaying() clearInterval(@player.interval) @@ -217,6 +225,12 @@ class @VideoPlayerAlpha extends SubviewAlpha if @video.videoType is 'youtube' @currentTime = Time.convert(@currentTime, parseFloat(@currentSpeed()), newSpeed) newSpeed = parseFloat(newSpeed).toFixed(2).replace /\.00$/, '.0' + + @video.log 'speed_change_video', + currentTime: @currentTime + old_speed: @currentSpeed() + new_speed: newSpeed + @video.setSpeed newSpeed, updateCookie if @video.videoType is 'youtube' if @video.show_captions is true diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee index b12041d9c6..e9ed9923b0 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.coffee @@ -7,10 +7,7 @@ class @VideoProgressSliderAlpha extends SubviewAlpha range: 'min' change: @onChange - # We don't want to attach to 'slide' event because we already have 'change' event. - # If we have two events, then callback will be triggered twice, sending misinformation - # to the server. - # slide: @onSlide + slide: @onSlide stop: @onStop @buildHandle() @@ -39,7 +36,7 @@ class @VideoProgressSliderAlpha extends SubviewAlpha onSlide: (event, ui) => @frozen = true @updateTooltip(ui.value) - $(@).trigger('seek', ui.value) + $(@).trigger('slide_seek', ui.value) onChange: (event, ui) => @updateTooltip(ui.value)