Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Web Video Text Tracks Format subtitles #2077

Merged
merged 3 commits into from Jan 3, 2018
Merged

Support Web Video Text Tracks Format subtitles #2077

merged 3 commits into from Jan 3, 2018

Conversation

schplurtz
Copy link
Contributor

Hi,
video already have alternative media, and poster. This adds support for Web Video Text Tracks Format subtitles, based on the same principle. When displaying media foo.*, if there are media files named foo.kind.lang.vtt, then they are vtt tracks and added to the video. kind is a 3letter abbreviation of one of the 5 vtt kinds defined by the w3c, lang is a language abbreviation. for example foo.cap.fr.vtt or foo.sub.de.vtt. The idea comes from a similar request in the DW French forum. A user needs subtitle for the hard of hearing visiting his site.

See the patch in action here https://schplurtz.darktech.org/doku-test/start (for a few weeks only)
image

Hoping you'll like the idea too.
/Schplurtz

@mention-bot
Copy link

@schplurtz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @splitbrain, @akate and @Klap-in to be potential reviewers.

@splitbrain
Copy link
Collaborator

I don't like the global language name array. In fact I would like to skip that array completely. Instead I would simply use the capture group from the regular expression and thus show the language code to the end user.

@schplurtz
Copy link
Contributor Author

Just in case you did not notice, current code will show lang code if there is no match in the
global lang array.

Anyway, I suppress this part as you request, and use only the language code.

foreach( $tracks as $trackid => $info ) {
list( $kind, $srclang ) = $info;
$out .= "<track kind=$kind srclang=$srclang ";
$out .= "label=$srclang ";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two more minor things before I merge this:

  • the attributes should be properly enclosed in quotes, eg. kind="sub" instead of kind=sub
  • even though it's probably not exploitable, I'd feel safer if $kind and $srclang would be escaped via hsc() before outputting them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing as you request, of course. I'll add a commit tonight to the PR.

But note that using hsc($kind) seems useless. $kind can only be one of subtitles, captions, descriptions, chapters and metadata : Initial value is validated by the regexp $re='/\\.(sub|cap|des|cha|met)\\.([^.]+)\\.vtt$/'; and then $kinds array is used to map the 3 letters to the word that is used in html.

$kinds=array(
        'sub' => 'subtitles',
        'cap' => 'captions',
        'des' => 'descriptions',
        'cha' => 'chapters',
        'met' => 'metadata'
);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're totally right of course, but that happens in a different method... one of the two methods may change in the future and someone might not be aware of the implications and accidentally introduce a problem. Since it doesn't hurt to escape here, I prefer to be safe rather than sorry later ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that makes sense.

@splitbrain splitbrain merged commit 345058f into dokuwiki:master Jan 3, 2018
@splitbrain
Copy link
Collaborator

@schplurtz can you add infos about this to the wiki? I think it would make sense to add a new page at https://www.dokuwiki.org/video and link it from https://www.dokuwiki.org/images

@schplurtz
Copy link
Contributor Author

schplurtz commented Jan 4, 2018

@splitbrain, I already had some text for this. It's now on https://www.dokuwiki.org/video . I did my best, but please, find someone to review and check for correctness. I'm better at translating English to French than writing English. No link from /images yet.

Also I copied the whole https://www.dokuwiki.org/wiki:syntax#fallback_formats section to https://www.dokuwiki.org/video so that everything that deals with video is on a single page. Just seemed the right thing to do. Eventually, this section will have to be removed from the syntax.txt file.

Unfortunately, I am most probably going to be offline for some weeks now, so to finish this up, you'll have to ask someone else or remind me in, say, 4 or 5 weeks as I'm pretty sure I won't remember.

@schplurtz schplurtz deleted the vtt-tracks branch January 4, 2018 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants