Add support for movie captions #907

douzebis · 2021-05-22T15:04:35Z

Generate HTML5 subtitle tracks dynamically based on the contents of the
where the movie is located.

For example a "movie.en.vtt" file in the same folder as the "movie.mkv"
will be used as a track for English subtitles.

douzebis · 2021-05-22T15:06:56Z

This is a feature enhancement corresponding to issue
#239

Generate HTML5 subtitle tracks dynamically based on the contents of the where the movie is located. For example a "movie.en.vtt" file in the same folder as the "movie.mkv" will be used as a track for English subtitles. Signed-off-by: Frederic Ruget <[email protected]>

douzebis · 2021-05-23T02:46:33Z

Hi! I've seen the failure in test Node / node12.x.
Do I need to commit the changes in files js/viewer-main.js and js/viewer-main.js.map? (Apparently automatically generated?)

beardhatcode · 2021-05-23T08:05:25Z

@douzebis, yes the files in js need to be pushed (it's indeed odd)

beardhatcode

Thank you for your contribution 😄. This looks good to me (but I don't decide if this gets in), I added some comments to make the code cleaner.

I am wondering about how this works when publicly sharing files. So, if you share a folder, will these captions work?

beardhatcode · 2021-05-23T08:12:37Z

src/components/Videos.vue

+				const lang = langs[i]
+				let language = languages[lang]
+				if (language.length === 0) language = lang
+				tracks += '<track src="'


It might be better to create these elements with vue (v-for) and not by creating a string here. A bit like this

Thanks a lot @beardhatcode!
I'll have a look at the 'v-for' syntax and try and use it.
I'll be updating my code and re-submitting.

Also, I may add the possibility to use '.movie.en.vtt' for the language tracks -- with a leading dot.
So as not to clutter the contents of the viewer folder with language tracks.

Ok, that could work but in that case you could have two vtt files per language be sure to give precedence to the visible file (without a dot). However, I would first try to just get it working and leave this addition for another PR.

beardhatcode · 2021-05-23T08:16:22Z

src/models/file.js

@@ -21,13 +21,14 @@
 */
 import { getDavPath } from '../utils/fileUtils'

-export default function(fileInfo, mime, component) {
+export default function(fileInfo, mime, component, caplangs) {


Hmmm, I'm not sure that caplangs should go here in file.js it is video specific. This change should not reach into this file. Maybe it is a util or a mixin?

beardhatcode · 2021-05-23T08:18:02Z

src/views/Viewer.vue

@@ -415,6 +418,19 @@ export default {
 					// retrieve folder list
 					const fileList = await folderRequest(dirPath)

+					// retrieve caption languages


I think this should be moved to the Video component as it is only used for videos

Actually i was unable to code the retrieval of the caption files within Videos.vue, because I could not implement it unless I was using an 'async' function and this did not work well with the Videos.vue template -- I got "promises" instead of the the actual list of files.

Please don't hesitate to suggest code snippets... I need guidance :)

"Promises" are values JavaScript returns when it does not have the data yet. So when you are getting a file listing, you need to send some requests, if all of the javascript stoped at that moment, that would kill the user experience. Instead, JavaScript returns a promise: "I don'd have this data now, but I promise I will have it". You can also say to JS, ah, OK once you have that data call this function with it as argument (then). So what you could do (I think) is:

Add a [created] hook to the Video component. In there you do what you did before to get the data (yourCodeThatFindsTheRightThingAndReturnsAPromice) and then use then to specify what should happen once we have that data. So in the snippet below response will hold the data you want, and

created() { yourCodeThatFindsTheRightThingAndReturnsAPromice(). .then(response => { /* do stuff with the respose to get the right things*/ this.captions = ... // ← set a fild to be used in the template on top }); }

I don't really have that much time to explain it well, but it is really worth the effort to learn about promises and async/await see this useful tutorial by Mozilla

beardhatcode · 2021-05-23T08:30:17Z

src/components/Videos.vue

+				let language = languages[lang]
+				if (language.length === 0) language = lang


Suggested change

let language = languages[lang]

if (language.length === 0) language = lang

let language = languages[lang] || lang

beardhatcode · 2021-05-23T08:32:24Z

src/components/Videos.vue

+			for (let i = 0; i < langs.length; ++i) {
+				const lang = langs[i]


Suggested change

for (let i = 0; i < langs.length; ++i) {

const lang = langs[i]

for (let lang of langs) {

But it might be even better to use v-for instead.

douzebis · 2021-05-23T21:26:07Z

OK.
I did it again from scratch and filed a new pull request:
#910

(I did not want to clutter the history of the viewer with back and forth modifications.)

Now there is a single modified file: src/components/Videos.vue

beardhatcode · 2021-05-23T21:33:12Z

@douzebis, that ok we could have just rewritten the history to clean it up. I'll close this PR then in favour of #910.

szaimen requested a review from skjnldsv May 22, 2021 21:38

szaimen added 3. to review Waiting for reviews enhancement New feature or request labels May 22, 2021

beardhatcode reviewed May 23, 2021

View reviewed changes

This was linked to issues May 23, 2021

add subtitles for video feature #239

Open

Support for subtitles in the video player #879

Closed

douzebis marked this pull request as draft May 23, 2021 10:37

beardhatcode closed this May 23, 2021

This was unlinked from issues May 23, 2021

add subtitles for video feature #239

Open

Support for subtitles in the video player #879

Closed

skjnldsv mentioned this pull request Aug 19, 2022

Upgrade existing video player. #1288

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for movie captions #907

Add support for movie captions #907

douzebis commented May 22, 2021

douzebis commented May 22, 2021

douzebis commented May 23, 2021

beardhatcode commented May 23, 2021

beardhatcode left a comment •

edited

Loading

beardhatcode May 23, 2021 •

edited

Loading

douzebis May 23, 2021

beardhatcode May 23, 2021

douzebis May 23, 2021

beardhatcode May 23, 2021 •

edited

Loading

beardhatcode May 23, 2021

douzebis May 23, 2021

beardhatcode May 23, 2021

beardhatcode May 23, 2021

beardhatcode May 23, 2021

douzebis commented May 23, 2021

beardhatcode commented May 23, 2021

		let language = languages[lang]
		if (language.length === 0) language = lang

	let language = languages[lang]
	if (language.length === 0) language = lang
	let language = languages[lang] \|\| lang

		for (let i = 0; i < langs.length; ++i) {
		const lang = langs[i]

	for (let i = 0; i < langs.length; ++i) {
	const lang = langs[i]
	for (let lang of langs) {

Add support for movie captions #907

Add support for movie captions #907

Conversation

douzebis commented May 22, 2021

douzebis commented May 22, 2021

douzebis commented May 23, 2021

beardhatcode commented May 23, 2021

beardhatcode left a comment • edited Loading

Choose a reason for hiding this comment

beardhatcode May 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beardhatcode May 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

douzebis commented May 23, 2021

beardhatcode commented May 23, 2021

beardhatcode left a comment •

edited

Loading

beardhatcode May 23, 2021 •

edited

Loading

beardhatcode May 23, 2021 •

edited

Loading