Closed Bug 955901 Opened 11 years ago Closed 7 years ago

Video Player doesn't shows subtitles

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::Video
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WONTFIX

People

(Reporter: ro2342, Assigned: fbukevin, NeedInfo)

Details

Attachments

(8 files)

File Maneger with mp4 and srt file on folder
28.71 KB, image/png
Details
WIP patch
327 bytes, text/plain
Details
application.zip
7.15 KB, application/zip
Details
2014-04-22-09-17-21.png
5.97 KB, image/png
Details
WIP: display subtitle and check file update
315 bytes, text/plain
Details
display vtt.png
47.38 KB, image/png
Details
Patch for video app subtitle display implemetation
54 bytes, text/plain
rnicoletti
: review-
Details
1335362_1.patch
570 bytes, patch
Details | Diff | Splinter Review
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.
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
Hi, There is a W3C standard for video caption called WebVTT (http://dev.w3.org/html5/webvtt). Do you need this?
(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.
If this is what you want then it already be enabled on our master branch but not on FxOS V1.1.0 (Gecko 18).
(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?
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?)
Assignee: nobody → fbukevin
Status: UNCONFIRMED → NEW
Ever confirmed: true
(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.
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.
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.
Ok, Veck and I would check the features you mentioned. If they need to be implemented, there would be some patches.
Hi, all! I start studying this problem.
I think it should be srt and webvtt.
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.
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?
Flags: needinfo?(rick.eyre)
(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.
Flags: needinfo?(rick.eyre)
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.
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.
Flags: needinfo?(fbukevin)
(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.
Flags: needinfo?(fbukevin)
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. :)
Will it be working on Firefox OS?
Yes! I launched it on Firefox OS simulator on Nightly.
I test this on a device and it works, too.
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.
Flags: needinfo?(fbukevin)
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.
(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!
Flags: needinfo?(fbukevin) → needinfo?(gweng)
(In reply to Veck Hsiao from comment #25) Okay sounds good. Thanks for the info Veck.
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.
Flags: needinfo?(gweng)
Attached file WIP patch —
Flags: needinfo?(johu)
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.
Attached file 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.
Flags: needinfo?(johu)
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]".
Flags: needinfo?(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.
Flags: needinfo?(johu)
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!
Attached image 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.
Flags: needinfo?(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.
Flags: needinfo?(johu)
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.
Flags: needinfo?(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.
Flags: needinfo?(johu)
OK! Thanks for reminding me. It successfully wrote.
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.
OK! I would try this better solution. Thanks!
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.
Flags: needinfo?(johu)
Attachment #8418169 - Flags: feedback?(johu)
Attached image 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.
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.
Some opinions from Jho
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 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.
Attachment #8418169 - Flags: feedback?(johu)
Flags: needinfo?(johu)
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.
Attachment #8421499 - Flags: review?(dflanagan)
Flags: needinfo?(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.
Attachment #8421499 - Flags: review?(dflanagan)
Flags: needinfo?(johu)
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!
Flags: needinfo?(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.
Flags: needinfo?(johu)
Add Russ to cc list.
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!
Flags: needinfo?(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.
Flags: needinfo?(johu)
Comment on attachment 8421499 [details] Patch for video app subtitle display implemetation >https://github.com/mozilla-b2g/gaia/pull/19183/
Attachment #8421499 - Flags: review?(rnicoletti)
(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.
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 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)
Attachment #8421499 - Flags: review?(rnicoletti) → review-
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.
Flags: needinfo?(fbukevin)
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Attached patch 1335362_1.patch — — Splinter Review

this is a test - please disregard

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: