BLD-915: Fix issues with different videoIDs.

CHANGELOG.rst

@@ -5,6 +5,9 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Blades: Fix issues related to videos that have separate YouTube IDs for the
+different video speeds. BLD-915, BLD-901.
+
 Blades: Add .txt and .srt options to the "download transcript" button. BLD-844.
 
 Blades: Fix bug when transcript cutting off view in full view mode. BLD-852.

common/djangoapps/terrain/start_stubs.py

@@ -15,8 +15,8 @@ SERVICES = {
 }
 
 
-@before.all
+@before.each_scenario
-def start_stubs():
+def start_stubs(_):
     """
     Start each stub service running on a local port.
     """
@@ -25,7 +25,7 @@ def start_stubs():
         setattr(world, name, fake_server)
 
 
-@after.all
+@after.each_scenario
 def stop_stubs(_):
     """
     Shut down each stub service.

common/djangoapps/terrain/stubs/youtube.py

@@ -5,7 +5,6 @@ Stub implementation of YouTube for acceptance tests.
 from .http import StubHttpRequestHandler, StubHttpService
 import json
 import time
-import requests
 from urlparse import urlparse
 from collections import OrderedDict
 
@@ -80,7 +79,7 @@ class StubYouTubeHandler(StubHttpRequestHandler):
             'data': OrderedDict({
                 'id': youtube_id,
                 'message': message,
-                'duration': 60,
+                'duration': 60 if youtube_id == 'OEoXaMPEzfM' else 77,
             })
         })
         response = "{cb}({data})".format(cb=callback, data=json.dumps(data))

common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js

@@ -233,26 +233,43 @@ function (Initialize) {
                         '1.0': 'testId',
                         '1.50': 'videoId'
                     },
-                    youtubeId: Initialize.prototype.youtubeId
+                    youtubeId: Initialize.prototype.youtubeId,
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
+            var msg = 'returns duration for the 1.0 speed if speed is not 1.0';
+            it(msg, function () {
+                var expected;
+
+                state.speed = '1.50';
+                expected = Initialize.prototype.getDuration.call(state);
+
+                expect(expected).toEqual(400);
+            });
+
+            describe('Flash mode', function () {
                 it('returns duration for current video', function () {
-                var expected = Initialize.prototype.getDuration.call(state);
+                    var expected;
+
+                    state.isFlashMode.andReturn(true);
+                    expected = Initialize.prototype.getDuration.call(state);
 
                     expect(expected).toEqual(100);
                 });
 
-            var msg = 'returns duration for the 1.0 speed as a fallback';
+                var msg = 'returns duration for the 1.0 speed as a fall-back';
                 it(msg, function () {
                     var expected;
 
+                    state.isFlashMode.andReturn(true);
                     state.speed = '2.0';
                     expected = Initialize.prototype.getDuration.call(state);
 
                     expect(expected).toEqual(400);
                 });
             });
+        });
 
         describe('youtubeId', function () {
             beforeEach(function () {
@@ -262,7 +279,8 @@ function (Initialize) {
                         '0.50': '7tqY6eQzVhE',
                         '1.0': 'cogebirgzzM',
                         '1.50': 'abcdefghijkl'
-                    }
+                    },
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
@@ -278,14 +296,25 @@ function (Initialize) {
                 });
             });
 
-            describe('without speed', function () {
+            describe('without speed for flash mode', function () {
                 it('return the video id for current speed', function () {
-                    var expected = Initialize.prototype.youtubeId.call(state);
+                    var expected;
+
+                    state.isFlashMode.andReturn(true);
+                    expected = Initialize.prototype.youtubeId.call(state);
 
                     expect(expected).toEqual('abcdefghijkl');
                 });
             });
 
+            describe('without speed for youtube html5 mode', function () {
+                it('return the video id for 1.0x speed', function () {
+                    var expected = Initialize.prototype.youtubeId.call(state);
+
+                    expect(expected).toEqual('cogebirgzzM');
+                });
+            });
+
             describe('speed is absent in the list of video speeds', function () {
                 it('return the video id for 1.0x speed', function () {
                     var expected = Initialize.prototype.youtubeId.call(state, '0.0');

common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js

@@ -55,11 +55,7 @@
                     });
 
                     waitsFor(function () {
-                        if (state.videoCaption.loaded === true) {
+                        return state.videoCaption.loaded;
-                            return true;
-                        }
-
-                        return false;
                     }, 'Expect captions to be loaded.', WAIT_TIMEOUT);
 
                     runs(function () {
@@ -77,17 +73,15 @@
                     });
                 });
 
-                it('fetch the caption in Youtube mode', function () {
+                it('fetch the caption in Flash mode', function () {
                     runs(function () {
                         state = jasmine.initializePlayerYouTube();
+                        spyOn(state, 'isFlashMode').andReturn(true);
+                        state.videoCaption.fetchCaption();
                     });
 
                     waitsFor(function () {
-                        if (state.videoCaption.loaded === true) {
+                        return state.videoCaption.loaded;
-                            return true;
-                        }
-
-                        return false;
                     }, 'Expect captions to be loaded.', WAIT_TIMEOUT);
 
                     runs(function () {
@@ -106,6 +100,31 @@
                     });
                 });
 
+                it('fetch the caption in Youtube mode', function () {
+                    runs(function () {
+                        state = jasmine.initializePlayerYouTube();
+                    });
+
+                    waitsFor(function () {
+                        return state.videoCaption.loaded;
+                    }, 'Expect captions to be loaded.', WAIT_TIMEOUT);
+
+                    runs(function () {
+                        expect($.ajaxWithPrefix).toHaveBeenCalledWith({
+                            url: '/transcript/translation',
+                            notifyOnError: false,
+                            data: jasmine.any(Object),
+                            success: jasmine.any(Function),
+                            error: jasmine.any(Function)
+                        });
+                        expect($.ajaxWithPrefix.mostRecentCall.args[0].data)
+                            .toEqual({
+                                language: 'en',
+                                videoId: 'cogebirgzzM'
+                            });
+                    });
+                });
+
                 it('bind the hide caption button', function () {
                     state = jasmine.initializePlayer();
                     expect($('.hide-subtitles')).toHandleWith(

common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js

@@ -105,6 +105,7 @@ function (VideoPlayer) {
             it('create Flash player', function () {
                 var player;
 
+                spyOn($.fn, 'trigger');
                 state = jasmine.initializePlayerYouTube();
                 state.videoEl = state.el.find('video, iframe').width(100);
                 player = state.videoPlayer.player;
@@ -715,7 +716,8 @@ function (VideoPlayer) {
                     },
                     currentPlayerMode: 'html5',
                     trigger: function () {},
-                    browserIsFirefox: false
+                    browserIsFirefox: false,
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
@@ -1015,7 +1017,8 @@ function (VideoPlayer) {
                         updatePlayTime: jasmine.createSpy(),
                         setPlaybackRate: jasmine.createSpy(),
                         player: jasmine.createSpyObj('player', ['setPlaybackRate'])
-                    }
+                    },
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
@@ -1033,7 +1036,7 @@ function (VideoPlayer) {
                 });
 
                 it('convert the current time to the new speed', function () {
-                    state.currentPlayerMode = 'flash';
+                    state.isFlashMode.andReturn(true);
                     VideoPlayer.prototype.onSpeedChange.call(state, '0.75', false);
                     expect(state.videoPlayer.currentTime).toBe('120.000');
                 });

common/lib/xmodule/xmodule/js/src/video/01_initialize.js

@@ -65,6 +65,7 @@ function (VideoPlayer, VideoStorage) {
         getDuration: getDuration,
         getVideoMetadata: getVideoMetadata,
         initialize: initialize,
+        isFlashMode: isFlashMode,
         parseSpeed: parseSpeed,
         parseVideoSources: parseVideoSources,
         parseYoutubeStreams: parseYoutubeStreams,
@@ -550,7 +551,7 @@ function (VideoPlayer, VideoStorage) {
             _this.videos[speed] = video[1];
         });
 
-        return true;
+        return _.isString(this.videos['1.0']);
     }
 
     // function parseVideoSources(, mp4Source, webmSource, oggSource)
@@ -702,7 +703,11 @@ function (VideoPlayer, VideoStorage) {
     }
 
     function youtubeId(speed) {
-        return this.videos[speed || this.speed] || this.videos['1.0'];
+        var currentSpeed = this.isFlashMode() ? this.speed : '1.0';
+
+        return  this.videos[speed] ||
+                this.videos[currentSpeed] ||
+                this.videos['1.0'];
     }
 
     function getDuration() {
@@ -713,6 +718,10 @@ function (VideoPlayer, VideoStorage) {
         }
     }
 
+    function isFlashMode() {
+        return this.currentPlayerMode === 'flash';
+    }
+
     function getCurrentLanguage() {
         var keys = _.keys(this.config.transcriptLanguages);
 

common/lib/xmodule/xmodule/js/src/video/03_video_player.js

@@ -73,7 +73,7 @@ function (HTML5Video, Resizer) {
         state.videoPlayer.ready = _.once(function () {
             $(window).on('unload', state.saveState);
 
-            if (state.currentPlayerMode !== 'flash') {
+            if (!state.isFlashMode()) {
                 state.videoPlayer.setPlaybackRate(state.speed);
             }
             state.videoPlayer.player.setVolume(state.currentVolume);
@@ -100,7 +100,7 @@ function (HTML5Video, Resizer) {
             modestbranding: 1
         };
 
-        if (state.currentPlayerMode !== 'flash') {
+        if (!state.isFlashMode()) {
             state.videoPlayer.playerVars.html5 = 1;
         }
 
@@ -140,11 +140,8 @@ function (HTML5Video, Resizer) {
             }, false);
 
         } else { // if (state.videoType === 'youtube') {
-            if (state.currentPlayerMode === 'flash') {
             youTubeId = state.youtubeId();
-            } else {
+
-                youTubeId = state.youtubeId('1.0');
-            }
             state.videoPlayer.player = new YT.Player(state.id, {
                 playerVars: state.videoPlayer.playerVars,
                 videoId: youTubeId,
@@ -162,22 +159,7 @@ function (HTML5Video, Resizer) {
                     videoHeight = player.attr('height') || player.height();
 
                 _resize(state, videoWidth, videoHeight);
-
+                _updateVcrAndRegion(state, true);
-                // After initialization, update the VCR with total time.
-                // At this point only the metadata duration is available (not
-                // very precise), but it is better than having 00:00:00 for
-                // total time.
-                if (state.youtubeMetadataReceived) {
-                    // Metadata was already received, and is available.
-                    _updateVcrAndRegion(state);
-                } else {
-                    // We wait for metadata to arrive, before we request the update
-                    // of the VCR video time, and of the start-end time region.
-                    // Metadata contains duration of the video.
-                    state.el.on('metadata_received', function () {
-                        _updateVcrAndRegion(state);
-                    });
-                }
             });
         }
 
@@ -185,8 +167,8 @@ function (HTML5Video, Resizer) {
             dfd.resolve();
         }
     }
-
+    function _updateVcrAndRegion(state, isYoutube) {
-    function _updateVcrAndRegion(state) {
+        var update = function (state) {
             var duration = state.videoPlayer.duration(),
                 time;
 
@@ -215,6 +197,23 @@ function (HTML5Video, Resizer) {
                     duration: duration
                 }
             );
+        };
+
+        // After initialization, update the VCR with total time.
+        // At this point only the metadata duration is available (not
+        // very precise), but it is better than having 00:00:00 for
+        // total time.
+        if (state.youtubeMetadataReceived || !isYoutube) {
+            // Metadata was already received, and is available.
+            update(state);
+        } else {
+            // We wait for metadata to arrive, before we request the update
+            // of the VCR video time, and of the start-end time region.
+            // Metadata contains duration of the video.
+            state.el.on('metadata_received', function () {
+                update(state);
+            });
+        }
     }
 
     function _resize(state, videoWidth, videoHeight) {
@@ -271,6 +270,8 @@ function (HTML5Video, Resizer) {
             }
         });
 
+        _updateVcrAndRegion(state, true);
+        state.trigger('videoCaption.fetchCaption', null);
         state.resizer.setElement(state.el.find('iframe')).align();
     }
 
@@ -348,7 +349,7 @@ function (HTML5Video, Resizer) {
             // where in Firefox speed switching to 1.0 in HTML5 player mode is
             // handled incorrectly by YouTube API.
             methodName = 'cueVideoById';
-            youtubeId = this.youtubeId();
+            youtubeId = this.youtubeId(newSpeed);
 
             if (this.videoPlayer.isPlaying()) {
                 methodName = 'loadVideoById';
@@ -360,10 +361,9 @@ function (HTML5Video, Resizer) {
     }
 
     function onSpeedChange(newSpeed) {
-        var time = this.videoPlayer.currentTime,
+        var time = this.videoPlayer.currentTime;
-            isFlash = this.currentPlayerMode === 'flash';
 
-        if (isFlash) {
+        if (this.isFlashMode()) {
             this.videoPlayer.currentTime = Time.convert(
                 time,
                 parseFloat(this.speed),
@@ -605,6 +605,11 @@ function (HTML5Video, Resizer) {
             }
         }
 
+        if (this.isFlashMode()) {
+            this.setSpeed(this.speed);
+            this.trigger('videoSpeedControl.setSpeed', this.speed);
+        }
+
         if (this.currentPlayerMode === 'html5') {
             this.videoPlayer.player.setPlaybackRate(this.speed);
         }
@@ -653,7 +658,7 @@ function (HTML5Video, Resizer) {
         videoPlayer.startTime = this.config.startTime;
         if (videoPlayer.startTime >= duration) {
             videoPlayer.startTime = 0;
-        } else if (this.currentPlayerMode === 'flash') {
+        } else if (this.isFlashMode()) {
             videoPlayer.startTime /= Number(this.speed);
         }
 
@@ -664,7 +669,7 @@ function (HTML5Video, Resizer) {
         ) {
             videoPlayer.stopAtEndTime = false;
             videoPlayer.endTime = null;
-        } else if (this.currentPlayerMode === 'flash') {
+        } else if (this.isFlashMode()) {
             videoPlayer.endTime /= Number(this.speed);
         }
     }
@@ -749,11 +754,7 @@ function (HTML5Video, Resizer) {
                 // HTML5 video sources play fine from start-time in both Chrome
                 // and Firefox.
                 if (this.browserIsFirefox && this.videoType === 'youtube') {
-                    if (this.currentPlayerMode === 'flash') {
                     youTubeId = this.youtubeId();
-                    } else {
-                        youTubeId = this.youtubeId('1.0');
-                    }
 
                     // When we will call cueVideoById() for some strange reason
                     // an ENDED event will be fired. It really does no damage

common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js

@@ -104,8 +104,7 @@ function () {
     // whole slider). Remember that endTime === null means the end-time
     // is set to the end of video by default.
     function updateStartEndTimeRegion(params) {
-        var isFlashMode = this.currentPlayerMode === 'flash',
+        var left, width, start, end, duration, rangeParams;
-            left, width, start, end, duration, rangeParams;
 
         // We must have a duration in order to determine the area of range.
         // It also must be non-zero.
@@ -120,7 +119,7 @@ function () {
 
         if (start > duration) {
             start = 0;
-        } else if (isFlashMode) {
+        } else if (this.isFlashMode()) {
             start /= Number(this.speed);
         }
 
@@ -128,7 +127,7 @@ function () {
         // video, then we set it to the end of the video.
         if (end === null || end > duration) {
             end = duration;
-        } else if (isFlashMode) {
+        } else if (this.isFlashMode()) {
             end /= Number(this.speed);
         }
 

common/lib/xmodule/xmodule/js/src/video/09_video_caption.js

@@ -155,7 +155,7 @@ function () {
         }
 
         this.el.on('speedchange', function () {
-            if (self.currentPlayerMode === 'flash') {
+            if (self.isFlashMode()) {
                 Caption.fetchCaption();
             }
         });
@@ -628,7 +628,7 @@ function () {
         if (this.videoCaption.loaded) {
             // Current mode === 'flash' can only be for YouTube videos. So, we
             // don't have to also check for videoType === 'youtube'.
-            if (this.currentPlayerMode === 'flash') {
+            if (this.isFlashMode()) {
                 // Total play time changes with speed change. Also there is
                 // a 250 ms delay we have to take into account.
                 time = Math.round(
@@ -670,7 +670,7 @@ function () {
 
         // Current mode === 'flash' can only be for YouTube videos. So, we
         // don't have to also check for videoType === 'youtube'.
-        if (this.currentPlayerMode === 'flash') {
+        if (this.isFlashMode()) {
             // Total play time changes with speed change. Also there is
             // a 250 ms delay we have to take into account.
             time = Math.round(

common/test/data/uploads/subs_b7xgknqkQk8.srt.sjson

@@ -0,0 +1,11 @@
+{
+  "start": [
+    1000
+  ],
+  "end": [
+    2000
+  ],
+  "text": [
+    "Equal transcripts"
+  ]
+}

lms/djangoapps/courseware/features/video.feature

@@ -62,12 +62,12 @@ Feature: LMS Video component
     Given I am registered for the course "test_course"
     And it has a video "A" in "Youtube" mode in position "1" of sequential
     And a video "B" in "Youtube" mode in position "2" of sequential
-    And a video "C" in "Youtube" mode in position "3" of sequential
+    And a video "C" in "HTML5" mode in position "3" of sequential
     And I open the section with videos
     And I select the "2.0" speed on video "A"
     And I select the "0.50" speed on video "B"
     When I open video "C"
-    Then video "C" should start playing at speed "0.50"
+    Then video "C" should start playing at speed "0.75"
     When I open video "A"
     Then video "A" should start playing at speed "2.0"
     And I reload the page
@@ -118,7 +118,7 @@ Feature: LMS Video component
     Then I see "好 各位同学" text in the captions
 
   # 14
-  Scenario: CC button works correctly if transcripts and sub fields are empty, but transcript file exists is assets (Youtube mode of Video component)
+  Scenario: CC button works correctly if transcripts and sub fields are empty, but transcript file exists in assets (Youtube mode of Video component)
     Given I am registered for the course "test_course"
     And I have a "subs_OEoXaMPEzfM.srt.sjson" transcript file in assets
     And it has a video in "Youtube" mode
@@ -176,3 +176,15 @@ Feature: LMS Video component
     Then I can download transcript in "txt" format
     When I open video "C"
     Then menu "download_transcript" doesn't exist
+
+  # 20
+  Scenario: Youtube video has correct transcript if fields for other speeds are filled.
+    Given the course has a Video component in Youtube mode:
+      | sub         | youtube_id_1_5 |
+      | OEoXaMPEzfM | b7xgknqkQk8    |
+    And I make sure captions are opened
+    Then I see "Hi, welcome to Edx." text in the captions
+    And I select the "1.50" speed
+    And I reload the page
+    Then I see "Hi, welcome to Edx." text in the captions
+    And I see duration "1:00"

lms/djangoapps/courseware/features/video.py

@@ -5,6 +5,7 @@ from lettuce import world, step
 import json
 import os
 import requests
+import time
 from common import i_am_registered_for_the_course, section_location, visit_scenario_item
 from django.utils.translation import ugettext as _
 from django.conf import settings
@@ -94,6 +95,21 @@ def add_video_to_course(course, player_mode, hashes, display_name='Video'):
         'metadata': {},
     }
 
+    course_location =world.scenario_dict['COURSE'].location
+
+    if hashes:
+        kwargs['metadata'].update(hashes[0])
+
+    conversions = {
+        'transcripts': json.loads,
+        'download_track': json.loads,
+        'download_video': json.loads,
+    }
+
+    for key in kwargs['metadata']:
+        if key in conversions:
+            kwargs['metadata'][key] = conversions[key](kwargs['metadata'][key])
+
     if player_mode == 'html5':
         kwargs['metadata'].update({
             'youtube_id_1_0': '',
@@ -119,28 +135,18 @@ def add_video_to_course(course, player_mode, hashes, display_name='Video'):
             'html5_sources': HTML5_SOURCES_INCORRECT
         })
 
-    if hashes:
+    if kwargs['metadata'].get('sub'):
-        kwargs['metadata'].update(hashes[0])
-    course_location =world.scenario_dict['COURSE'].location
-
-    conversions = {
-        'transcripts': json.loads,
-        'download_track': json.loads,
-        'download_video': json.loads,
-    }
-
-    for key in kwargs['metadata']:
-        if key in conversions:
-            kwargs['metadata'][key] = conversions[key](kwargs['metadata'][key])
-
-    if 'sub' in kwargs['metadata']:
         filename = _get_sjson_filename(kwargs['metadata']['sub'], 'en')
         _upload_file(filename, course_location)
 
-    if 'transcripts' in kwargs['metadata']:
+    if kwargs['metadata'].get('transcripts'):
         for lang, filename in kwargs['metadata']['transcripts'].items():
             _upload_file(filename, course_location)
 
+    if kwargs['metadata'].get('youtube_id_1_5'):
+        filename = _get_sjson_filename(kwargs['metadata']['youtube_id_1_5'], 'en')
+        _upload_file(filename, course_location)
+
     world.scenario_dict['VIDEO'] = world.ItemFactory.create(**kwargs)
 
 
@@ -208,6 +214,36 @@ def _set_window_dimensions(width, height):
     world.wait(0.2)
 
 
+def _duration():
+        """
+        Total duration of the video, in seconds.
+        """
+        elapsed_time, duration = _video_time()
+        return duration
+
+
+def _video_time():
+        """
+        Return a tuple `(elapsed_time, duration)`, each in seconds.
+        """
+        # The full time has the form "0:32 / 3:14"
+        full_time = world.css_text('div.vidtime')
+
+        # Split the time at the " / ", to get ["0:32", "3:14"]
+        elapsed_str, duration_str = full_time.split(' / ')
+
+        # Convert each string to seconds
+        return (_parse_time_str(elapsed_str), _parse_time_str(duration_str))
+
+
+def _parse_time_str(time_str):
+    """
+    Parse a string of the form 1:23 into seconds (int).
+    """
+    time_obj = time.strptime(time_str, '%M:%S')
+    return time_obj.tm_min * 60 + time_obj.tm_sec
+
+
 @step('when I view the (.*) it does not have autoplay enabled$')
 def does_not_autoplay(_step, video_type):
     assert(world.css_find('.%s' % video_type)[0]['data-autoplay'] == 'False')
@@ -237,8 +273,13 @@ def visit_video_section(_step):
     visit_scenario_item('SECTION')
 
 
+@step('I select the "([^"]*)" speed$')
+def change_video_speed(_step, speed):
+      _change_video_speed(speed)
+
+
 @step('I select the "([^"]*)" speed on video "([^"]*)"$')
-def change_video_speed(_step, speed, player_id):
+def change_video_speed_on_video(_step, speed, player_id):
       _navigate_to_an_item_in_a_sequence(sequence[player_id])
       _change_video_speed(speed)
 
@@ -355,6 +396,14 @@ def start_playing_video_from_n_seconds(_step, position):
     )
 
 
+@step('I see duration "([^"]*)"$')
+def i_see_duration(_step, position):
+    world.wait_for(
+        func=lambda _: _duration() == _parse_time_str(position),
+        timeout=5
+    )
+
+
 @step('I seek video to "([^"]*)" seconds$')
 def seek_video_to_n_seconds(_step, seconds):
     time = float(seconds.strip())