955901 - Video Player doesn't shows subtitles

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Video, defect)

Description

[ro2342]

Attachment: File Maneger with mp4 and srt file on folder

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131206152524

Steps to reproduce:

I open a video in Video Player. In its folder, I uploaded the .srt file too. 


Actual results:

Video Player doesn't show the subtitles in video. It doesn't have any way to show it. It doesn't have any way to enable it.


Expected results:

Video Player should shows subtitle .srt when it is on folder. Video Player need a settings page to enable or desable subtitle.

Comment 1

[ro2342]

My device: LG Fireweb

Version: 1.1.0.1
Firmware: LGD300fAR-00-V10d-724-06-SEP-30-2013
Hardware: REV_1.0
18.1
Compilation: 20131005124448
Update channel: release-lg
Git: 2013-10-05 01:22:53

Comment 2

[Marco Chen [:mchen]]

Hi,

There is a W3C standard for video caption called WebVTT (http://dev.w3.org/html5/webvtt).

Do you need this?

Comment 3

[ro2342]

(In reply to Marco Chen [:mchen] [PTO from 2013/12/24 ~ 2014/01/02] from comment #2)
> Hi,
> 
> There is a W3C standard for video caption called WebVTT
> (http://dev.w3.org/html5/webvtt).
> 
> Do you need this?

Yes.

Comment 4

[Marco Chen [:mchen]]

If this is what you want then it already be enabled on our master branch but not on FxOS V1.1.0 (Gecko 18).

Comment 5

[ro2342]

(In reply to Marco Chen [:mchen] [PTO from 2013/12/24 ~ 2014/01/02] from comment #4)
> If this is what you want then it already be enabled on our master branch but
> not on FxOS V1.1.0 (Gecko 18).

Thank you so much.

Ok. It will be featured in FxOS 1.2 or 1.3?

Comment 6

[Greg Weng [:snowmantw][:gweng][:λ]]

Subtitle won't show up in this case (with the video at the same folder).
I've tried this on Gecko 29.0a1, with Gaia at 1/06 (my own branch, but rebase on the new master).

Veck want to take this bug and I've discussed this with John Hu, he said this bug is OK to be taken. And we can implement the function first, then to invite UX to correct the designation. 

(Should I change this bug to NEW?)

Comment 7

[Marco Chen [:mchen]]

(In reply to Greg Weng [:snowmantw] from comment #6)

Hi Greg,

Why do we encourage people to use WebVTT but intead of using ".srt"?
Web VTT is the web standard.

Comment 8

[Greg Weng [:snowmantw][:gweng][:λ]]

Huh? Following your discussion, the requirement seems followed such transformation:

1. I need SRT (Summary)-> 2. Are you asking WebVTT (Comment 2)? -> 3. Yes, I need WebVTT (Comment 3)...

(Where is SRT now???)

So I now got confused: what's the requirement now?
I followed the original requirement.

And as I asked, some popular video player on Android would do this. And I think it's a convenient way to allow user to put subtitles, which is the same behavior they would do on desktop.

However if this need more discussions, we can clarify it here.

Comment 9

[Marco Chen [:mchen]]

Hi Greg,

That is my mistake for mis-typing the sentence. 

I mean "according to WebVTT is the web standard, we should encourage people to use WebVTT but not .srt".
So the job here may be

  1. Let video tag in video app include webvtt file (automatically choose the same name or an UI for user to select).
  2. If it is possible, a tool can be made for transforming .srt to webvtt format.

Comment 10

[Greg Weng [:snowmantw][:gweng][:λ]]

Ok, Veck and I would check the features you mentioned. If they need to be implemented, there would be some patches.

Comment 11

[Veck Hsiao]

Hi, all!
I start studying this problem.

Comment 12

[ro2342]

I think it should be srt and webvtt.

Comment 13

[Veck Hsiao]

Hi, all!
I've tried to reproduce this bug with Nightly.
I can't temporarily load .mp4 but .webm work, so I reproduced this bug by playing a webm file and load SRT (.srt) and WebVTT (.vtt). 
As ro2342 said, the subtitle did not display, both srt and vtt file.
I'm now studying the problem and solution.

Comment 14

[Veck Hsiao]

Greg Wang found that something wrong about webvtt support.

This is a bug of Gecko that seems haven't yet patched: 
https://bugzilla.mozilla.org/show_bug.cgi?id=629350

Is WebVTT being supported now?

Comment 15

[Rick Eyre (:reyre)]

(In reply to Veck Hsiao from comment #14)
> Greg Wang found that something wrong about webvtt support.
> 
> This is a bug of Gecko that seems haven't yet patched: 
> https://bugzilla.mozilla.org/show_bug.cgi?id=629350
> 
> Is WebVTT being supported now?

WebVTT is partially supported on non-stable releases of Firefox. Nightly has it, as well as Aurora and Beta I believe. We're still in the process of adding support for it.

We're only working on support for WebVTT now and not SRT.

Comment 16

[ro2342]

It's sad because almost anyone uses webvtt and srt is The filetype everyone knows to use in subtitles 

I think it should support srt and webvtt.

Comment 17

[Greg Weng [:snowmantw][:gweng][:λ]]

One solution John and I has discussed is to translate srt to webvtt on the phone, but this would cost runtime resource and maybe just a temporary solution if Gecko can support srt in the future.

And as Rick said, it seems that we already supported WebVTT, so Veck "should" be successful while test WebVTT with webm (but it apparently not)? Maybe we need to do more investigation.

Comment 18

[Veck Hsiao]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #17)
> One solution John and I has discussed is to translate srt to webvtt on the
> phone, but this would cost runtime resource and maybe just a temporary
> solution if Gecko can support srt in the future.
> 
> And as Rick said, it seems that we already supported WebVTT, so Veck
> "should" be successful while test WebVTT with webm (but it apparently not)?
> Maybe we need to do more investigation.

I test is again. It still not show the subtitle with both .srt and .vtt.

My Nightly might be the latest version because I just download it two days ago.

I'll study why WebVTT could not display.

Comment 19

[Veck Hsiao]

I fixed the problem that my Nightly could not play video with WebVTT.
And I play the webm video with both .srt and .vtt again.

SRT could not display whereas WebVTT could.

I think I'm gonna study whether there is a method to transform .srt to .vtt.

The best solution might be expectation of support of SRT. :)

Comment 20

[ro2342]

Will it be working on Firefox OS?

Comment 21

[Veck Hsiao]

Yes! 
I launched it on Firefox OS simulator on Nightly.

Comment 22

[Veck Hsiao]

I test this on a device and it works, too.

Comment 23

[Rick Eyre (:reyre)]

Did you get WebVTT subtitles to work on Firefox OS Veck? If so, what did you have to do to get them to show? I'm curious whether this is a bug or not.

Comment 24

[Ralph Giles (:rillian)]

We enabled WebVTT subtitle support in firefox 28, so they should display by default in Firefox OS 1.3 and later.

I've just confirmed this: https://people.xiph.org/~giles/2013/vtt-01.html displays a subtitle in the 1.3 simulator and Desktop Nightly, but not in the 1.2 simulator or on my Keon running 1.2. It is possible to get playback on earlier versions by setting the 'media.webvtt.enabled' pref to true. I tested this on Firefox OS at some point, but don't remember if it was 1.0 or 1.1.

> The best solution might be expectation of support of SRT. :)

The problem with SRT is that it does not define a character set encoding, so one must use heuristics to guess. This caused many problems for all web browsers with HTML pages, so we do not want to support SRT. Unfortunate because it is otherwise simple and very popular.

WebVTT has similar syntax, so converting with a server script or javascript on the phone is not expensive. Three steps:

1. Determine SRT file character encoding and convert to UTF-8.
2. Convert comma (,) radix to period (.) radix in timestamps.
3. Prepend 'WEBVTT\n\n' to the file.

Comment 25

[Veck Hsiao]

(In reply to Rick Eyre (:reyre) from comment #23)
> Did you get WebVTT subtitles to work on Firefox OS Veck? If so, what did you
> have to do to get them to show? I'm curious whether this is a bug or not.

Hi! Rick.
I can't display both WebVTT and SRT with app "Video" on Firefox OS on Nightly initially.
These are my platform information:
* Operating System: OS X 10.9.1 (Maverick)
* Nightly version: 30.0a1 (2014-02-11)

I discussed with Greg and found a way to display WebVTT subtitle.
This is what I did:
1. implement an app by modifying gaia/test_apps/template/index.html
2. add following snippet in <body>..</body>:
   <video controls>
     <source src="bunny.webm" type="video/webm">
     <track kind="subtitles" src="bunny.vtt" srclang="en">
   </video>
3. put *.webm and *.vtt into template folder.

After doing so, I launched this apps on Firefox OS on Nightly and got subtitle displayed.


Thanks you Ralph!
I'll try it!

Comment 26

[Rick Eyre (:reyre)]

(In reply to Veck Hsiao from comment #25)
 
Okay sounds good. Thanks for the info Veck.

Comment 27

[Greg Weng [:snowmantw][:gweng][:λ]]

So, do covert at Gaia seems a way to support that. I'm not sure how much cost we would pay at runtime, but I think taking a profiling can provide good information.

Comment 28

[Veck Hsiao]

Attachment: WIP patch

Comment 29

[Veck Hsiao]

Sorry for lack of describe the patch doing:

On line 82 to 94 of gaia/apps/video/js/db.js
I added a snippet using navigator.getDeviceStorage('sdcard'); to get the video and subtitle files in sdcard on device. But there is a problem that if files got successfully, program does not get into onsuccess function on line 88. While onerror function on line 92 is fine when request gets FileNotFound.

Comment 30

[John Hu [:johnhu][:johu][:醬糊小弟]]

Attachment: application.zip

Veck,

Thanks for contributing this bug. With this WIP, I found an issue that we need to put sdcard permission into manifest. Please find the attachment which loads file correctly.

Comment 31

[Veck Hsiao]

Thanks John,

I tried your attachment. But it still has the same problem.
First, I put "device-storage:sdcard":{ "access": "readonly" } into the manifest.webapp of video app, but it didn't change anything.(That is, onsuccess still cannot be gotten in)

Then I tried to move your attachment app into device with copying your code to replace test-app/template.
It always gave me "Unable to write the file: [object DOMError]".

Comment 32

[John Hu [:johnhu][:johu][:醬糊小弟]]

Please tell us more about your environment 
1. Which device do you use?
2. Which version/branch do you use?

I had showed the attachment to Greg face by face. It doesn't make sense you cannot do it.

Comment 33

[Veck Hsiao]

Hi John!
With help of Greg, we figured out the problem is because I didn't reset gaia before.
I did the following flows to successfully access to sdcard onsuccess:
1. Put permission of sdcard as you provided in attachment into manifest.
2. Use command "DEVICE_DEBUG=1 NOFTU=1 make reset-gaia" reseting gaia.
3. Launch video or template.

Finally, the message in request.onsuccess function is shown up.

Thanks for your help!

Comment 34

[Veck Hsiao]

Attachment: 2014-04-22-09-17-21.png

I found a problem that when I implement test-app/template/index.html with:
      <video controls>
        <source src="./sample/sample.webm" type="video/webm">
        <track kind="subtitles" src="./sample/sample.vtt" srclang="en">
      </video>
It worked well when I use firefox browser or device to launch this app.
But when I implemented it with sdcard like this:
      <video controls>
        <source src="/sdcard/sample/sample.webm" type="video/webm">
        <track kind="subtitles" src="/sdcard/sample/sample.vtt" srclang="en">
      </video>
and installed into device, the app was always loading and never play.

The log message from adb logcat is:
E/GeckoConsole( 2009): [JavaScript Warning: "All candidate resources failed to load. Media load paused." {file: "chrome://global/content/bindings/videocontrols.xml" line: 1015}]

On the other hand, in test.js of template apps, it could successfully get the file from sdcard as I mentioned above.

Comment 35

[John Hu [:johnhu][:johu][:醬糊小弟]]

I believe you won't have the authority to do so. And that definitely won't work :)

We may need to use device storage to retrieve the files in sdcard and use URL.createObjectURL to build the url for "src" attribute.

Please note, sdcard is protected by permission. You may need to request device-storage:sdcard permission to do so.

Comment 36

[Veck Hsiao]

Additionally, I want to convert *.srt into *.vtt on device. 
I think I should read the content of *.srt and create a new file named *.vtt to write correct content in.

I used the same way to access to sdcard with "navigator.getDeviceStorage('sdcard');".
Then I built a blob file and call "sdcard.addName(..)" to write file on sdcard.
But it didn't work, instead shown "Unable to write the file: [Object DOMError]".
I even modified the permission in manifest.webapp of sdcard from "readonly" to "readwrite", but it still performed read only.

Comment 37

[John Hu [:johnhu][:johu][:醬糊小弟]]

Have you tried "make reset-gaia"?

Every modification on manifest need to reset-gaia to let gecko rebuild the cache of app. But we don't need it if we distribute an app through firefox marketplace.

Comment 38

[Veck Hsiao]

OK! Thanks for reminding me. It successfully wrote.

Comment 39

[John Hu [:johnhu][:johu][:醬糊小弟]]

Veck,

To save to sdcard is not the only choice of persisting the converted srt file. We may also save it as a blob in a field of video metadata to mediadb. So that, we may delete the cached srt file when we delete the video file as usual. That's more convenient in the case that user deletes video files in UMS mode because mediadb does the synchronization between file system and DB.

Please note all of them may need to take care about the synchronization between original vtt and cached srt file.

Comment 40

[Veck Hsiao]

OK! I would try this better solution. Thanks!

Comment 41

[Veck Hsiao]

Hi John!
I successfully made srt file translated into vtt so that it can display video with subtitle in WebVTT format. No matter user provides only srt or vtt, the both cases can display video with subtitle. But I have a problem when I was working.

If I put a video file without subtitle into sdcard and launch video, the program will call getMetadata() to check if there is a vtt file or srt file. If a valid subtitle file exist, getMetadata() translates it into vtt if it is an srt format whereas vtt file will be loaded directly.

The problem is that if initially there does not have any subtitle file exist, getMetadata() will do nothing about subtitle issue. After that I put a subtitle file into sdcard, and launch video app again, it seems that the program doesn't work to check existence of subtitle again. So the subtitle will not be converted or loaded. Until I remove the video file from sdcard and put it back to sdcard again, the check procedure will operate again.

Comment 42

[Veck Hsiao]

Attachment: WIP: display subtitle and check file update

Comment 43

[Veck Hsiao]

Attachment: display vtt.png

The patch "display vtt.png" shows the screen of video playing with its subtitle file.
The height of subtitle bar vary from different video. In this video, the height is short while some video perform higher bar.

Comment 44

[Veck Hsiao]

In attachment 8418169 [details] (WIP)

I made subtitle file converted into vtt if its format is srt, and if the format is vtt, it could be loaded directly.

In this patch I also made a check for updating of subtitle file.
I modified the code in apps/video/js/db.js to make it checks for file update.
That is, every time when subtitle file updated (modified contents, rename, ...etc.), program will also get metadata again. This makes enumerateDB() called each time when video app is launched. 
I think this workflow is correct, to check if the video has to load with subtitle file every time the app is launched.

Comment 45

[Greg Weng [:snowmantw][:gweng][:λ]]

Some opinions from Jho

Comment 46

[Greg Weng [:snowmantw][:gweng][:λ]]

Sorry, adding CC while I'm typing.

Some opinions from John:

1. Those hard-coded path of subtitles can be replaced by the file info just got at the moment the video info fetched.

2. Remove those temporary files and comments

3. Re-organize code to make your new features as separated files and then include them to call functions

4. The size of the subtitle element may need Gecko supports

5. You can send the patch for review after you refine it

Comment 47

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8418169 [details]
WIP: display subtitle and check file update

Please find the comments at comment 48.

This is a new feature. So, I put media team in CC list. Once you finish the patch, please also set review flag to David.

Comment 48

[Veck Hsiao]

Attachment: Patch for video app subtitle display implemetation

The files I modified in this commit:
gaia/apps/video/manifest.webapp
gaia/apps/video/js/metadata.js
gaia/apps/video/js/db.js
gaia/apps/video/js/video.js

I created gaia/apps/video/js/subtitle.js to implement subtitle display feature.

After I rebased my commit to master branch which I had update to the latest version, it came up extra 140 files changed. Those were not done by me. All my work on this patch was listed above.

Comment 49

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8421499 [details]
Patch for video app subtitle display implemetation

Veck,

The "rebase" shouldn't introduce unrelated file changes. Please do it again and check if anything wrong with the procedure of rebase. Maybe, Greg may help you about that.

BTW, the patch gets travis red on linter:
apps/video/js/subtitle.js: line 4, col 21, 'getSubtitle' is defined but never used. (ERROR)

apps/video/js/subtitle.js: line 47, col 24, 'updateSubtitle' is defined but never used. (ERROR)

So, I remove the review flag. Please put that back when you fix above things.

Comment 50

[Veck Hsiao]

Hi John, 

https://travis-ci.org/mozilla-b2g/gaia/builds/25529075

I fixed the problem of my commit. 
And this link shows the result of testing by TravisCI.

It seems that testing was failed due to the error in unit-test-in-firefox.
But when I traced the report, I can't find where did the error occurred.
Could you please help me to figure out the error with the report if there exist an error indeed.

Thanks!

Comment 51

[John Hu [:johnhu][:johu][:醬糊小弟]]

I had restarted your unit test in travis. It looks like some tests take too long to run which makes travis times out. The limitation is 50 mins. Let's check it again.

Comment 52

[John Hu [:johnhu][:johu][:醬糊小弟]]

Add Russ to cc list.

Comment 53

[Veck Hsiao]

I do some modification and add some comments.
The result of testing still staying timeout on unit-test-in-firefox.
Please help me to set up review.
Thank you very much!

Comment 54

[John Hu [:johnhu][:johu][:醬糊小弟]]

You can set the review flag to russ, since he is a peer of video app now.

I had checked the unit tests. There is a failure in video app. Please also checked that.

Comment 55

[Veck Hsiao]

Comment on attachment 8421499 [details]
Patch for video app subtitle display implemetation

>https://github.com/mozilla-b2g/gaia/pull/19183/

Comment 56

[Russ Nicoletti [:russn]]

(In reply to Veck Hsiao from comment #55)
> Comment on attachment 8421499 [details]
> Patch for video app subtitle display implemetation
> 
> >https://github.com/mozilla-b2g/gaia/pull/19183/

I have begun reading the bug and understanding the changes. I will continue to review tomorrow.

Comment 57

[Russ Nicoletti [:russn]]

Hi Veck,

I have updated the PR with some comments. I tried to test the patch but I couldn't get it working. I'm getting a SecurityError when setSubtitle is attempting to read the vtt file. Not sure why. I can see the video manifest.web app has the new sdcard permission.

I'm leaving the review as r?. I'll review again when you update the PR.

Comment 58

[Russ Nicoletti [:russn]]

Comment on attachment 8421499 [details]
Patch for video app subtitle display implemetation

Because changes break unit tests, giving r-. Change to r? when changes are made (or needinfo me if you have any questions)

Comment 59

[sangyeol]

Hi Veck,

It doesn't seem to be vides.js supports any subtitles yet.
Don't you have any plans to complete this 'video app subtitle display implemetation' ?
And if you already done this, please let me know which branch it has been applied.

Thanks.

Comment 60

[BMO Automation]

Firefox OS is not being worked on

Comment 61

[David Lawrence [:dkl]]

Attachment: 1335362_1.patch

this is a test - please disregard