7
\$\begingroup\$

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(/\n\s*\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);
}
\$\endgroup\$
2
\$\begingroup\$

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.



  • 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() 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 || '',
    // Returns textContent of the <sub> element
    getTextContent: () => subtitle.data.element.textContent,
    // Creates a subtitle element for the current video
    createElement: () => {
      // Return immediately if subtitle element doesn't exist yet
      if (!['undefined', 'SUB'].includes(HTMLVideoElement.nextSibling.tagName)) { return; }

      // Insert nice subtitle font
      const font = document.createElement('link');
      [font.rel, font.href] = ['stylesheet', 'https://fonts.googleapis.com/css?family=Sunflower:300'];
      document.head.appendChild(font);

      // Store new 'sub' element into the current subtitle object
      subtitle.data.element = document.createElement('sub');

      // Append node to document
      HTMLVideoElement.parentNode.insertBefore(subtitle.data.element, HTMLVideoElement.nextSibling);
    },
    /**
     * Loads subtitle file over HTTP(S)
     * Calls subtitle.parse(content)
     *
     * @param   {string}    - Path / URL to subtitle file
     */
    load: file => fetch(file).then(response => response.text()).then(subtitle.parse),
    /**
     * Parses subtitle file
     *
     * @param   {string}    - SRT text content
     * @returns {object}    - Object containing subtitles
     */
    parse: content => {
      // First split all paragraphs into chunks
      subtitle.data.paragraphs = content.split(/\n\s*\n/g);

      subtitle.data.paragraphs.forEach(paragraph => {
        const arr = paragraph.split('\n');

        // Store paragraph information
        subtitle.data.subtitles.push({
          '$index': arr[0],
          '$timing': subtitle.splitTimes(arr[1].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: () => {
      const currentTime = video.getCurrentTime().replace(/[:,]+/g, '');

      // Set subtitle when video's currentTime matches the subtitle time
      if (currentTime > subtitle.data.current.$timing[0].join('') && 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 (currentTime > 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];
      }
    },
    /**
     * Splits each of SRT timing strings (HH:MM:SS,fff) into an array like ['00', '00', '00', '000']
     *
     * @param   {time}    - Array of SRT timing strings, eg. 01:44:03,732
     * @returns {array}   - Array of ['hour', 'minute', 'seconds', 'milliseconds']
     */
    splitTimes: time => time.map(str => str.split(/[:,]+/)),
    /**
     * Gets the current active subtitle
     *
     * @returns {object}    - Current subtitle
     */
    getCurrentSubtitle: () => subtitle.data.current,
    getNextSubtitle: () => subtitle.data.next,
    setNextSubtitle: () => {
      subtitle.data.index++;
      subtitle.data.current = subtitle.data.next;
      subtitle.data.next = subtitle.data.subtitles[subtitle.data.index];
    },
    recalculate: () => { // Recalculates which subtitle is current and next
      const i = subtitle.data.subtitles.findIndex(sub =>
        video.getCurrentTime().replace(/[:,]+/g, '') < sub.$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];
    }
  };

  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'));
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Many many thanks. I can really learn from this. I'm going to figure out each line of the rewrite code. So I can fully understand what you did and how it improves it. I should dive into these ES2015 and 2016 things, but for now its a bit to complicated for me to understand yet. But you gave me a nice piece of feedback, chapeau! \$\endgroup\$
    – Red
    Jun 12 '18 at 12:40
  • \$\begingroup\$ @Red: I tried to explain every significant change I made, but if you'll need some further help, don't hesitate to ask — your code looked fine overall, so I'll be glad to explain something better and hear how it all worked ;). \$\endgroup\$
    – Przemek
    Jun 12 '18 at 12:56
  • \$\begingroup\$ Oh, just to add a note: Lots of people say, extending the DOM's prototype is a bad practice. Many people say this only because they hear this from alot of developers. Well, its perfectly safe to extend the DOM prototype aslong as you code well. This Don't extend the DOM is comming from the days of IE <8; github.com/nbubna/mind-hacking/blob/gh-pages/… \$\endgroup\$
    – Red
    Jun 13 '18 at 12:49
  • \$\begingroup\$ @Red: Danger of naming collision is still real. When you look at how they introduced ES2015 and further ones, every now and then some functionality had to be disabled by default not to break some existing applications that were used at big enough scale for it to be a concern. \$\endgroup\$
    – Przemek
    Jun 13 '18 at 17:00
1
\$\begingroup\$
  • 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?
\$\endgroup\$
6
  • \$\begingroup\$ Because I wanted my answer to be purely a code review, I didn't say the things you did well in my answer. But I am really positively impressed by the way you organize your code into many small functions. The comments in your code are helpful (except the "resolve promise" comment, I am not sure what it means). \$\endgroup\$
    – continued-
    Jun 11 '18 at 14:03
  • \$\begingroup\$ Hi, oh .. no problem :). Im happy you toke your time to review it. Ive heavily updated the code and will Paste it here soon. Its a big update, some errors you noted were indeed some wrong comment lines. I used a promise there, but removed it and forgot the comment line. Most of your feedback given is resolved. Will give notify you when the updated code has been added. Again, thanks for taking your time :) \$\endgroup\$
    – Red
    Jun 11 '18 at 14:28
  • \$\begingroup\$ Remember to post your edited code in a new question instead of editing this one! \$\endgroup\$
    – continued-
    Jun 11 '18 at 14:57
  • \$\begingroup\$ Oh, just to add a note: Lots of people say, extending the DOM's prototype is a bad practice. Many people say this only because they hear this from alot of developers. Well, its perfectly safe to extend the DOM prototype aslong as you code well. This Don't extend the DOM is comming from the days of IE <8; github.com/nbubna/mind-hacking/blob/gh-pages/… \$\endgroup\$
    – Red
    Jun 13 '18 at 12:49
  • \$\begingroup\$ @Red I have actually encountered cases where I extended a random object's prototype and when new language features were added, my code suddenly broke. (anecdotal experience, I don't have proof at hand). Have you heard of smoosh and smooshMap? \$\endgroup\$
    – continued-
    Jun 14 '18 at 7:19

Not the answer you're looking for? Browse other questions tagged or ask your own question.