trjhtr

I've written a JavaScript prototype which extends the video tag with a new function.

myVideo.subtitle('/path/to/subtitle')

The HTML video tag only supports WebVTT as subtitles. So I thought, let's create support for .srt files as well with a custom written script.

It doesn't add support throughout the <track> element, however, I might add that in a future version. At the moment only adding a subtitle programmatically works.

You can even add a subtitle with a .txt extension or whatever. As long as it's supported by the browser and the content of the file is formatted like below:

1
hour:minute:second,milliseconds --> hour:minute:second,milliseconds
this is the text

2
hour:minute:second,milliseconds --> hour:minute:second,milliseconds
this is more text

I'm planning to add more subtitle formats in future versions.

What can I do better and what can I do more efficiently? Thanks for taking your time, I really appreciate it.

Below is lots of code. Perhaps its easier to read / review here: https://codepen.io/richardmauritz/project/editor/DOzmNL#

window.addEventListener("DOMContentLoaded", function ()

 var video = document.getElementById('video');

 video.subtitle('/subs/sub.txt');
)

HTMLMediaElement.prototype.subtitle = function (file)

 /**
 * Without a file no need to execute script
 */
 if (!file)
 
 return;
 

 var HTMLMediaElement = this;

 /**
 * Confirm element is a video
 */
 if (HTMLMediaElement.tagName !== 'VIDEO')
 
 return;
 

 var subtitle = 
 data: 
 subtitles: ,
 paragraphs: null,
 element: null,
 index: 0,
 current: null,
 next: null,
 ,
 /**
 * Sets innerHTML of the <sub> element
 */
 setTextContent: function (text)
 
 subtitle.data.element.innerHTML = text ? text : '';
 ,
 /**
 * Returns innerHTML of the <sub> element
 */
 getTextContent: function ()
 
 return subtitle.data.element.innerHTML;
 ,
 /**
 * Creates a subtitle element for the current video
 */
 createElement: function ()
 
 /**
 * Check if subtitle element doensn't exists yet
 */
 if (HTMLMediaElement.nextSibling.tagName !== 'undefined' && HTMLMediaElement.nextSibling.tagName !== 'SUB')
 
 /**
 * Insert nice subtitle font
 */
 var font = document.createElement('link');
 font.rel = 'stylesheet';
 font.href = 'https://fonts.googleapis.com/css?family=Sunflower:300';
 /**
 * Append font
 */
 document.head.appendChild(font);
 /**
 * Create new sub element
 */
 var element = document.createElement('sub');
 /**
 * Store element into current subtitle object
 */
 subtitle.data.element = element;
 /**
 * Append node to document
 */
 HTMLMediaElement.parentNode.insertBefore(element, HTMLMediaElement.nextSibling);
 
 ,
 /**
 * Loads subtitle file over HTTP(S)
 * Calls subtitle.parse(content)
 *
 * @param string - Path / URL to subtitle
 */
 load: function (file)
 
 /**
 * XMLHttpRequest
 */
 var request = new XMLHttpRequest();
 request.open('GET', file);
 request.onreadystatechange = function ()
 
 /**
 * Resolve promise, return subtitle contents
 */
 if (request.responseText !== '')
 
 subtitle.parse(request.responseText);
 
 ;
 /**
 * Send XMLHttpRequest
 */
 request.send();
 ,
 /**
 * Parses subtitle file
 *
 * @param string - SRT text content
 * @returns object - Object containing subtitles
 */
 parse: function (content)
 
 /**
 * First split all paragraphs into chunks
 */
 subtitle.data.paragraphs = content.split(/ns*n/g);

 for (var i = 0; i < subtitle.data.paragraphs.length; i++)
 
 /**
 * Temporary array
 */
 var arr = subtitle.data.paragraphs[i].split('n');

 /**
 * Store paragraph information
 */
 subtitle.data.subtitles.push(
 "$index": arr.slice(0, 1).join(),
 "$timing": subtitle.stringToArray(arr.slice(1, 2).join().split(" --> ")),
 "$textContent": arr.slice(2, arr.length).join()
 );
 ;

 /**
 * Set defaults
 */
 subtitle.data.current = subtitle.data.subtitles[subtitle.data.index];
 subtitle.data.next = subtitle.data.subtitles[subtitle.data.index + 1];
 subtitle.createElement();
 ,
 /**
 * Starts displaying the subtitles when video is started
 * Gets called using the video.timeupdate event listener
 */
 play: function ()
 
 /**
 * Set subtitle when video's currentTime matches the subtitle time
 */
 if (subtitle.stringToArray(video.getCurrentTime.toString()).join('') > subtitle.data.current.$timing[0].join(''))
 
 if (subtitle.getTextContent() === '')
 
 subtitle.setTextContent(subtitle.data.current.$textContent);
 
 ;
 /**
 * Unset current and set next subtitle when video's currentTime is greater than subtitles end time
 */
 if (subtitle.stringToArray(video.getCurrentTime.toString()).join('') > subtitle.data.current.$timing[1].join(''))
 
 subtitle.setTextContent('');
 subtitle.data.index++;
 subtitle.data.current = subtitle.data.next;
 subtitle.data.next = subtitle.data.subtitles[subtitle.data.index];
 
 ,
 /**
 * Converts SRT timing string (00:00:00,000) to array ['00', '00', '00', '000']
 *
 * @param string - SRT timing string Eg. 01:44:03,732 (hours:minutes:seconds:milliseconds)
 * @returns array - Array ['hour', 'minute', 'seconds', 'milliseconds']
 */
 stringToArray: function (string)
 
 var response = ;

 if (typeof string === 'object')
 
 for (var i = 0; i < string.length; i++)
 
 response.push(string[i].split(/[:,]+/));

 

 return response;
 
 else
 
 response.push(string.split(/[:,]+/));
 return response[0];
 
 ,
 /**
 * Gets the current active subtitle
 *
 * @returns object - Current subtitle
 */
 getCurrentSubtitle: function ()
 
 return subtitle.data.current;
 ,
 getNextSubtitle: function ()
 
 return subtitle.data.next;
 ,
 setNextSubtitle: function ()
 
 subtitle.data.index++;
 subtitle.data.current = subtitle.data.next;
 subtitle.data.next = subtitle.data.subtitles[subtitle.data.index];
 ,
 /**
 * Recalculates which subtitle is current and next
 */
 recalculate: function ()
 
 for (var i = 0; i < subtitle.data.subtitles.length; i++)
 
 /**
 * Find next subtitle based on video's currentTime
 */
 if (subtitle.stringToArray(video.getCurrentTime.toString()).join('') < subtitle.data.subtitles[i].$timing[0].join(''))
 
 /**
 * Update subtitle data
 */
 subtitle.data.index = i;
 subtitle.data.current = subtitle.data.subtitles[i];
 subtitle.data.next = subtitle.data.subtitles[i + 1];
 /**
 * Break for loop when matched
 */
 break;
 
 
 
 

 var video = 
 /**
 * Returns the current playback position in the video (in seconds)
 *
 * @returns number - Playback position in seconds
 */
 getCurrentTime: 
 /**
 * Returns the video durantion hours
 * 
 * @returns string
 */
 hours: function ()
 
 return Math.floor(HTMLMediaElement.currentTime / 3600) < 10 ? '0' + Math.floor(HTMLMediaElement.currentTime / 3600) : Math.floor(HTMLMediaElement.currentTime / 3600);
 ,
 /**
 * Returns the video durantion minutes
 * 
 * @returns string
 */
 minutes: function ()
 
 return Math.floor(HTMLMediaElement.currentTime / 60) < 10 ? '0' + Math.floor(HTMLMediaElement.currentTime / 60) : Math.floor(HTMLMediaElement.currentTime / 60);
 ,
 /**
 * Returns the video durantion seconds
 * 
 * @returns string
 */
 seconds: function ()
 
 return Math.floor(HTMLMediaElement.currentTime % 60) < 10 ? '0' + Math.floor(HTMLMediaElement.currentTime % 60) : Math.floor(HTMLMediaElement.currentTime % 60);
 ,
 /**
 * Returns the video durantion milliseconds
 * 
 * @returns string
 */
 milliseconds: function ()
 
 return (HTMLMediaElement.currentTime % 60).toString().replace('.', '').substring(2, 5);
 ,
 /**
 * Returns the full duration in the same format as the subtitle
 * 
 * @returns string
 */
 toString: function ()
 
 return video.getCurrentTime.hours() + ':' + video.getCurrentTime.minutes() + ':' + video.getCurrentTime.seconds() + ',' + video.getCurrentTime.milliseconds();
 
 ,
 /**
 * Fires when video starts playing or unpaused
 */
 playing: function ()
 
 subtitle.play();
 ,
 /**
 * Fires when video is set forwards or backwards
 */
 seeking: function ()
 
 subtitle.recalculate();
 
 

 HTMLMediaElement.addEventListener('timeupdate', video.playing);
 HTMLMediaElement.addEventListener('seeking', video.seeking);

 /**
 * Initialize the subtitle
 */
 subtitle.load(file);

Remarks

• Functions that require an argument to work should rather than silently fail, throw TypeError().

/**
 * Without a file no need to execute script
 */
if (!file)

 return;

could become

if (!file) throw new TypeError(`$file is not a valid path name`); 

There's also no need for comment here — it's in the error's message.

• You shouldn't touch the prototype of HTMLMediaElement. Don't modify objects you don't own.

• If you didn't extend the prototype of HTMLMediaElement but of HTMLVideoElement instead, the following check wouldn't be necessary:

if (HTMLMediaElement.tagName !== 'VIDEO')

 return;

• In setTextContent() and getTextContent() instead of .innerHTML you should use .textContent. It's more performant since it's not parsing HTML, which also prevents potential XSS attacks.

• In setTextContent() this ternary: text ? text : '' could become text || ''.

• 
  

  In createElement() instead of enclosing everything in one big if you could check for the opposite condition and return if it matches.

  
  
  

  Also, to make the code more concise and not to repeat HTMLMediaElement.nextSibling.tagName twice, you could put both strings that you test for in an array, like so:

  
  

if (!['undefined', 'SUB'].includes(HTMLVideoElement.nextSibling.tagName))

• There are some comments that are not telling more than the code itself, but their presence makes reading far harder, e.g.:

/**
 * Create new sub element
 */
var element = document.createElement('sub');

• Entire 22-line synchronous load() function can be replaced with a neat asynchronous one-liner using Fetch API:

file => fetch(file).then(response => response.text()).then(subtitle.parse)

• In parse() function you've got arr.slice(0, 1).join() and arr.slice(1, 2).join(). They are simply equal to arr[0] and arr[1] respectively.

• There are 3 instances (in play() and recalculate()) of

subtitle.stringToArray(video.getCurrentTime.toString()).join('')

which is equal to

video.getCurrentTime.toString().replace(/[:,]+/g, '')

It's more concise and has one less function call. Replacing one with the other has also that advantage, that it removes all the cases where you pass string to stringToArray() fucntion. Speaking of…

• 
  

  stringToArray() function was especially confusing to me, until I went down the stream to see how it's actually used. Both documentation and parameter name suggested you expect a string, but then there was a check if string is actually an object and if so it seemed you… iterate over it. time instanceof Array would be more clear. Also, a comment that you test for an array, different parameter name and mention in documentation that both string as well as an array may be passed would be really helpful.

  
  
  

  All together with the previous bullet point, the entire function could be:

  
  

splitTimes: time => time.map(str => str.split(/[:,]+/))

• In recalculate() function instead of for and if to find particular index, you could use function… well, .findIndex(). ES2015 goodies (｡◕‿‿◕｡).

• In video.getCurrentTime you've got a lot of long statements with ternary operators to pad time to 2 digits with zero. In ES2015 it can be done like this:

`$…`.padStart(2, '0'),

Note: the two back-ticks are template literals to convert math expression in them to string.

What's more, splitting the whole thing into so many functions harms performance and readability. It could all be written as:

getCurrentTime: () => 
 const time = HTMLVideoElement.currentTime,
 hours = `$Math.floor(time / 3600)`.padStart(2, '0'),
 minutes = `$Math.floor(time / 60)`.padStart(2, '0'),
 seconds = `$Math.floor(time % 60)`.padStart(2, '0'),
 milliseconds = `$time % 60`.replace('.', '').substring(2, 5);

 return `$hours:$minutes:$seconds,$milliseconds`;

• Other little things: there are a few unnecessary semicolons, few are missing, sub is not the right HTML element in this context and many comments are invalid and hence confusing.

Rewrite

Below code has less than 45% of lines and around 50% of characters of the original one. I hope I didn't miss anything ;).

HTMLVideoElement.prototype.subtitle = function(file) 

 if (!file) throw new TypeError(`$file is not a valid path name`); 

 const HTMLVideoElement = this;

 const subtitle = 
 data: 
 subtitles: ,
 paragraphs: null,
 element: null,
 index: 0,
 current: null,
 next: null,
 ,
 // Sets textContent of the <sub> element
 setTextContent: text => subtitle.data.element.textContent = text ;

 const video = 
 /**
 * Returns the current playback position in format HH:MM:SS,fff
 *
 * @returns string - Playback position in seconds
 */
 getCurrentTime: () => 
 const time = HTMLVideoElement.currentTime,
 hours = `$Math.floor(time / 3600)`.padStart(2, '0'),
 minutes = `$Math.floor(time / 60)`.padStart(2, '0'),
 seconds = `$Math.floor(time % 60)`.padStart(2, '0'),
 milliseconds = `$time % 60`.replace('.', '').substring(2, 5);

 return `$hours:$minutes:$seconds,$milliseconds`;
 ,
 playing: () => subtitle.play(), // When video starts playing or gets unpaused
 seeking: () => subtitle.recalculate() // When video is set forwards or backwards
 ;

 HTMLVideoElement.addEventListener('timeupdate', video.playing);
 HTMLVideoElement.addEventListener('seeking', video.seeking);

 // Initialize the subtitle
 subtitle.load(file);
;

window.addEventListener('DOMContentLoaded', () => document.getElementById('video').subtitle('/subs/sub.txt'));

• You've inserted some safety checks across your code, which is nice, but you are silently swallowing these errors. You should log something into the console when a check failed. For instance, you can console.error(new Error("[...your message here...]")) when the file parameter is not passed. Creating a new Error object instead of just logging a string gives you a convenient stack trace to examine when something weird happens.


• You should use a different name for the video element. Shadowing the HTMLMediaElement makes the code somewhat confusing to read.


• The method name says setTextContent, but you are setting innerHTML! You should avoid using innerHTML when textContent can be used instead because setting innerHTML unnecessarily can open you to a wide range of XSS attacks. If you absolutely have to set innerHTML, at least try to sanitize the HTML first.


• I don't think you should be monkey-patching the HTMLMediaElement prototype. It's bad practice to modify objects you do not own.


• 
  "Resolve promise, return subtitle contents" Well, technically you're not resolving any Promises, but developers keep inventing new technical terms so I don't really know whether the word "promise" refers to anything other than "[an] object represents the eventual completion (or failure) of an asynchronous operation, and its resulting value".


• The sub element is for subscripts, not for subtitles. Using a div element is better in this case. It's better to say nothing than to lie.
  


• Maybe it's time to write ES6 and beyond code?