Skip to content

E
edx-platform
Commit 43c16bd0 by polesye
Options

BLD-915: Fix issues with different videoIDs.

parent cc76690d
Showing with 257 additions and 122 deletions
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
def start_stubs():
@before.each_scenario
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,25 +233,42 @@ function (Initialize) {
'1.0': 'testId',
'1.50': 'videoId'
},
youtubeId: Initialize.prototype.youtubeId
youtubeId: Initialize.prototype.youtubeId,
isFlashMode: jasmine.createSpy().andReturn(false)
};
});
it('returns duration for current video', function () {
var 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 if speed is not 1.0';
it(msg, function () {
var expected;
state.speed = '2.0';
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;
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 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 () {
......@@ -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 true;
}
return false;
return state.videoCaption.loaded;
}, '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 true;
}
return false;
return state.videoCaption.loaded;
}, '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');
}
youTubeId = state.youtubeId();
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);
// 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);
});
}
_updateVcrAndRegion(state, true);
});
}
......@@ -185,36 +167,53 @@ function (HTML5Video, Resizer) {
dfd.resolve();
}
}
function _updateVcrAndRegion(state, isYoutube) {
var update = function (state) {
var duration = state.videoPlayer.duration(),
time;
function _updateVcrAndRegion(state) {
var duration = state.videoPlayer.duration(),
time;
time = state.videoPlayer.figureOutStartingTime(duration);
time = state.videoPlayer.figureOutStartingTime(duration);
// Update the VCR.
state.trigger(
'videoControl.updateVcrVidTime',
{
time: time,
duration: duration
}
);
// Update the VCR.
state.trigger(
'videoControl.updateVcrVidTime',
{
time: time,
duration: duration
}
);
// Update the time slider.
state.trigger(
'videoProgressSlider.updateStartEndTimeRegion',
{
duration: duration
}
);
state.trigger(
'videoProgressSlider.updatePlayTime',
{
time: time,
duration: duration
}
);
};
// Update the time slider.
state.trigger(
'videoProgressSlider.updateStartEndTimeRegion',
{
duration: duration
}
);
state.trigger(
'videoProgressSlider.updatePlayTime',
{
time: time,
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,
isFlash = this.currentPlayerMode === 'flash';
var time = this.videoPlayer.currentTime;
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');
}
youTubeId = this.youtubeId();
// 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',
left, width, start, end, duration, rangeParams;
var 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 → 100644
{
"start": [
1000
],
"end": [
2000
],
"text": [
"Equal transcripts"
]
}
\ No newline at end of file
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
......@@ -94,7 +94,7 @@ Feature: LMS Video component
# 11
Scenario: CC button works correctly w/o english transcript in HTML5 mode of Video component
Given the course has a Video component in HTML5 mode:
| transcripts |
| transcripts |
| {"zh": "chinese_transcripts.srt"} |
And I make sure captions are opened
Then I see "好 各位同学" text in the captions
......@@ -112,13 +112,13 @@ Feature: LMS Video component
# 13
Scenario: CC button works correctly w/o english transcript in Youtube mode of Video component
Given the course has a Video component in Youtube mode:
| transcripts |
| transcripts |
| {"zh": "chinese_transcripts.srt"} |
And I make sure captions are opened
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:
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']:
if kwargs['metadata'].get('sub'):
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())
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment