Video: allow multiple transcripts

[auraz]

Ticket: BLD-642
XML should look like:

<video youtube="1.00:gnqth8uWzO8" display_name="Biology Demonstration" >
     <transcript language="de" src="my_german_translation.srt" />
     <transcript language="zh" src="my_mandarin_translation.srt" />
     <transcript language="ru" src="my_russian_translation.srt" />
</video>

When the student hovers over the 'cc' button then a menu appears over the 'cc' button with languages.
The show/hide transcript functionality remains present upon clicking the 'cc' button
The "download transcript" button should download whichever transcript is currently selected by the student.

@nedbat, @singingwolfboy, @valera-rozuvan, @frrrances please review.

Current coverage is 84% and it is different from jenkins results because coverage.py configured such that tests in LMS/CMS don't count coverage for modules in common/lib.
common/lib/xmodule/xmodule/video_module/video_module.py (96.0%)
common/lib/xmodule/xmodule/video_module/transcripts_utils.py (49.7%)
common/lib/xmodule/xmodule/video_module/video_utils.py (100%)
Transcript_utils is covered by acceptance tests.

[frrrances]

@talbs can you review this in my place?

[talbs]

Yep, I'm on it. @auraz, it looks like some tests are failing currently as well.

[frrrances]

@auraz I ended up having more time than @talbs so I did the review. Looks good to me. 👍

[auraz]

'uk', not 'ua'

[nedbat]

(Missing the opening quote on de') What are the values here? The complete text of the translated transcript? Or a filename?

[polesye]

Fixed.

[nedbat]

@auraz I've reviewed all the code I understand. I've asked a few questions on the ticket also. Some things need to be fixed.

[cahrens]

@auraz I'm just starting to look at the PR as @singingwolfboy is out for a few days. Please fix the quality issues and also look at your code coverage. It looks like a lot of the video module Python code is not covered.

[cahrens]

When I created some new transcript entries, but the transcript files I referred to haven't been uploaded. When I save the video module, an error bar flashes briefly at the bottom. I think this is the error it is showing me:

[cahrens]

2014-02-12 16:58:19,756 INFO 12647 [xmodule.video_module.video_module] video_module.py:494 - Can't find uploaded transcripts: OEoXaMPEzfM
[12/Feb/2014 16:58:19] "POST /preview/xblock/i4x:;_;_fff;_101;_video;_7bae39df4c97496083e1dca7a8252cb1/handler/xmodule_handler/save_user_state HTTP/1.1" 500 32114

[auraz]

@cahrens error that you've seen, was unrelated to this PR and has already been fixed today and merged to master from https://github.com/edx/edx-platform/pull/2586

[cahrens]

@auraz Yes, that is the bug I was seeing. I created a story for a custom error handler for xblock ajax callbacks. In general, we need to handle errors thrown by xblocks in a better manner.

https://edx-wiki.atlassian.net/browse/LMS-2248

[cahrens]

I am having trouble determining if this feature is working. Frances gave me the test course that has transcripts for 3 languages. However, they actually all appear to be the same. I tried modifying the German one and re-uploading it to my course, but I still don't see the modified transcript when I click on German on the transcript popup menu.

When I try it in the LMS, I don't see the modified transcript either, and the popup menu does not bold the currently selected language like it does in Studio. Furthermore, I don't see a link to download the transcript (which perhaps would tell me which one I am looking at).

Please tell me what I am doing wrong!

[valera-rozuvan]

@polesye I am getting the following error:

WE'RE HAVING TROUBLE RENDERING YOUR COMPONENT

Students will not be able to access this component. Re-edit your
component to fix the error.

Error: u'EN'

I have uploaded a new transcript file, and entered "EN", and "OEoXaMPEzfM-russian.srt" into the two Transcript Translations fields.

[valera-rozuvan]

@polesye This is the transcript file OEoXaMPEzfM-russian.srt https://gist.github.com/valera-rozuvan/44e3a139967289b00276.

[cahrens]

This is in the beforeEach.

[polesye]

This is in the beforeEach.

Removed.

[auraz]

@cahrens > You will also need to make it clear to the user that simply modifying an existing srt file (to fix a translation) does not work. It is necessary to delete the old assignment and add a new one.

This is fixed as you've suggested. @polesye would you like to add acc.test for this?

[auraz]

@cahrens >
You should define how you expect the video component to act if someone refers to a transcript file that does not exist. I ran into 2 different scenarios, neither of which was nice.

1. I was able to save the video component, but no transcripts displayed at all (even though transcripts for 2 of the languages were correct).

2. In another case, I got a "Studio is having trouble saving your work" error when I tried saving the component. The unit page did not render properly, and I had to refresh it.

Answer:
Exception should be generated in Studio.
No exception should be generated when importing from xml (just no subtitles for given language).

Studio:
I was able to reproduce # 2): Studio's having trouble saving your work TranscriptException at /xblock/test.1.2/branch/draft/block/video40b : Can't find uploaded transcripts: zzzz.srt That's fine.

I wasn't able to reproduce # 1) on save.

But I was able to reproduce error, when SJSON and SRT files where deleted while video player was using them. We've added handler for this case.

[polesye]

I am still not seeing any indication of which language is selected in the LMS. In Studio, the selected language is bolded, but it is not in the LMS.

Fixed styles.

[auraz]

cache_toolbox code was removed, as we now getting transcripts via Xblock handler right from contentstore, not via assets urls.

[auraz]

Removed import of modulestore inside functions, as modulestore can be accessed from xmodule.django

[auraz]

Update: from xmodule.modulestlre.django import modulestore is not importable at module level in video_module due to https://gist.github.com/auraz/9120459

[auraz]

@nedbat, @cahrens, all your comments were addressed. Please continue review.

[cahrens]

👍 I would recommend that you squash commits before submitting.

It is still the case that if you modify a transcript (download, change, re-upload), the new version is not picked up automatically. Now it is slightly better in that the new transcript will be processed when you save the video component (without having to delete and re-add the filename). In the future, it would be nice to figure out a way to avoid the need to edit the video component.

[auraz]

@cahrens Sure, commits will be squashed.

It is possible to always regenerate subtitles, but it will slow down rendering of video player in LMS, that's why we intentionally disabled this functionality. If you think that we still need to do it, please let me know your opinion.

[cahrens]

I'd say wait until people complain before slowing down the rendering. Documentation around what is required to regenerate transcripts may be enough @mhoeber.

[auraz]

@cahrens I've got you point. We will think about it.

[nedbat]

I have no further comments. :)

[auraz]

@nedbat but you've not given thumbs up )

[nedbat]

👍