#543 ✓ staged
Dan Barron

TTML Subtitles Seemingly broken

Reported by Dan Barron | May 31st, 2011 @ 10:41 AM | in 1.0 Release (closed)

Hey guys, it's most likely I'm doing something wrong, but I'm trying to get TTML subtitles working with the popcorn.parserTTML.js parser for Popcorn. On my own, the subtitles don't work. I'm using a valid TTML file, but with an .xml extension. This file is a copy of a DFXP file from Brightcove, which, as it would seem, is just a synonym for TTML, as the XML adheres to the TTML standard. So that's not working when I do it.

However, I went into the qunit tests in the parsers directory. The file popcorn.parserTTML.unit.html shows failures. I've included a screencap to show these. The actual popcorn.parserTTML.html file does not show subtitles either.

Other subtitles work fine, but I have a requirement to work with TTML so I have to get this working. Could anyone let me know if this is a bug, or if I'm just doing this wrong? I wrote a document to test simple subtitles with TTML. My actual players are more complex as they have to load in videos from Brightcove, but I'd rather tackle that once I can be assured that my subtitles are working without all that extra heft.

My document is as follows:

<!DOCTYPE html>
<html>
<head>
  <title>TTML Demo</title>

  <script src="popcorn-complete.min.js"></script>
  <script src="popcorn.subtitle.js"></script>
  <script src="parserTTML/popcorn.parserTTML.js"></script>

  <script>
    document.addEventListener( 'DOMContentLoaded', function () {
      var p = Popcorn( '#video' )
      .volume( 0 )
      .play();
    }, false);
  </script>
</head>
<body>
  <div>
    <video id='video'
      controls
      data-timeline-sources="parserTTML/data/data.ttml">

      <source id='mp4'
        src="test.mp4"
        type='video/mp4; codecs="avc1, mp4a"'>

      <p>Your user agent does not support the HTML5 Video element.</p>

    </video>
  </div>
</body>
</html>

As you can see, I'm using the TTMl file bundled with the Popcorn parsers to test this, and not my DFXP file. Neither work. If I've failed to provide any additional information to help solve this bug, please let me know. I'm tearing my hair out over this. I need a hug.

Update: For some reason my screen cap didn't upload. The short of it says that the tests timed out.

Popcorn 0.3 TTML Parser Plugin (3, 0, 3)
Parsed all subtitles, expected: 0 result: 10, diff: 0 10
Test timed out
Expected 41 assertions, but 2 were run

Comments and changes to this ticket

  • cadecairos

    cadecairos June 1st, 2011 @ 09:33 AM

    Hey Dan, Thanks for filing this bug report!

    Could you please let us know what Operating system and Web Browser you are experiencing this trouble in?

    Thanks!

    -Chris

  • Dan Barron

    Dan Barron June 1st, 2011 @ 09:38 AM

    Oh, duh!

    This was tested on Mac OS 10.6.7 and it happens in Chrome 12 beta, Firefox 4, and Safari 5

  • Rick

    Rick June 1st, 2011 @ 10:54 AM

    • State changed from “new” to “open”

    My first thoughts would be that you need to remove these lines:

    
      <script src="popcorn.subtitle.js"></script>
      <script src="parserTTML/popcorn.parserTTML.js"></script>
    

    popcorn.complete.min.js provides both of those extensions.

    Note: There is no "return to OP" feature, so I'm not sure how to re-assign to Dan Barron

  • cadecairos

    cadecairos June 1st, 2011 @ 10:56 AM

    • State changed from “open” to “new”

    I'm able to reproduce the test failures on Fedora 14 using Firefox 4 and Chrome 12 Beta. looks like a problem with script loading in popcorn-complete-min.js

    Could be related related to the open map plug-in loading OpenLayer.js from http://openlayers.org/ which seems to hang sometimes. This hang looks to be causing the tests to time out. A new bug is being filed for this.

    When I run the TTML demo using popcorn-complete-min-js, The parser still works and subtitles appear.

    Have you double checked that the relative paths to the resources (video file, TTML file, etc) are all correct?

  • cadecairos

    cadecairos June 1st, 2011 @ 10:56 AM

    • State changed from “new” to “open”
  • Dan Barron

    Dan Barron June 1st, 2011 @ 11:14 AM

    Rick,

    I removed those two script tags and substituted in popcorn-complete.min.js and subtitles were still not working. I've double checked my references to the TTML files and those are all in order. My debugger throws an error if it can't locate the TTML file I'm trying to load, and I don't see any of those errors.

  • David Humphrey

    David Humphrey June 1st, 2011 @ 04:41 PM

    • State changed from “open” to “bugs”
  • David Seifried

    David Seifried June 2nd, 2011 @ 03:54 PM

    • State changed from “bugs” to “peer-review-requested”

    my bad, meant to update butter

  • David Seifried

    David Seifried June 2nd, 2011 @ 03:55 PM

    • State changed from “peer-review-requested” to “bugs”
  • David Seifried

    David Seifried September 14th, 2011 @ 06:59 PM

    • Tag changed from parsers, subtitles, ttml to parsers, starters, subtitles, ttml
    • Milestone set to 1.0 Release
    • Milestone order changed from “62” to “0”
  • David Seifried

    David Seifried October 18th, 2011 @ 04:41 PM

    • Assigned user set to “Steven W”

    Steven it would be awesome if you could take a look at this

  • Steven W

    Steven W October 20th, 2011 @ 11:55 PM

    • State changed from “bugs” to “peer-review-requested”
    • Assigned user changed from “Steven W” to “David Seifried”

    Registering the mime type on the server works, but it doesn't really address the larger issue for all our users.

    This patch changes:

    • On xhr, popcorn will try to automatically parse XML from the responseText, just like it does with JSON. This will help ensure other XML-based parsers (like TTXT) will also continue to work. If it can't be parsed, data.xml is null, just like always.
    • Also, a subtitle parse fix for Opera in the parser itself. Calling element.getAttribute() to parse the id wasn't working with a namespace-qualified attribute in Opera

    Plugin unit tests pass on:
    Firefox 7.0.1
    Firefox 3.5.19
    Chrome 15
    Opera 11.51
    Safari 5.0.2
    IE 9

    Core Unit tests written to test new functionality, they pass on:
    Firefox 7.0.1
    Chrome 15
    Opera 11.51
    Safari 5.0.2

    I get some issues running other core tests, notably the getScript() hangs in FF 7. I get this behaviour even before I made these changes, though. I don't think it's related. The TTML parser still has a lot of whitespace/refactoring issues, I'll fix those in #752

    Branch: https://github.com/stevenaw/popcorn-js/tree/t543

  • David Seifried

    David Seifried October 24th, 2011 @ 03:17 PM

    • State changed from “peer-review-requested” to “super-review-requested”
    • Assigned user changed from “David Seifried” to “Rick”

    This looks good to me, and seems like something we are going to need to document to let users know to set a mime-type on their server ( if they can ) and explain why they could possibly be getting a fail.

    New core unit tests pass for me, and im also not getting the getScript error you are seeing.

    Passing to Rick for SR.

  • cadecairos

    cadecairos October 24th, 2011 @ 04:14 PM

    setting mimetypes server-side shouldn't be a big issue now. If response xml is ever null because of a bad mimetype, it will try to parse it with DomParser (this is a parser wide change ). this is good.

  • Rick

    Rick October 26th, 2011 @ 06:45 PM

    • Assigned user changed from “Rick” to “Steven W”
    • State changed from “super-review-requested” to “review-needs-work”

    There is no reason for the function textToXML to be defined everytime Popcorn.xhr.httpData` is called. I would yank that out and at the line that currently looks like...

    if ( !data.xml || !data.xml.documentElement ) {
      // Check for null documentElement as well so IE will cooperate
      data.xml = textToXML( settings.ajax.responseText );
    }
    

    Change to...

    // add this line to the existing var declarations. Shown here for illustration only
    var parser, xml, errors;
    
    if ( !data.xml || !data.xml.documentElement ) {
      // Check for null documentElement as well so IE will cooperate
      try {
        parser = new DOMParser();
        xml = parser.parseFromString( settings.ajax.responseText, "text/xml" );
        errors = xml.getElementsByTagName( "parsererror" );
    
        if ( !errors ) {
          data.xml = xml;
        }
      } catch ( e ) {
        // suppress
      }
    }
    
  • Steven W

    Steven W October 26th, 2011 @ 11:18 PM

    • Assigned user changed from “Steven W” to “David Seifried”
    • State changed from “review-needs-work” to “peer-review-requested”

    Fixed and re-pushed.

    Tested in browsers mentioned above, as well as:
    Opera 12
    FF 8
    FF 9a2
    FF 10a1
    IE9 (No cross-browser unit tests because IE's DOMParser.parseFromString output lacks ".toString()", but I've verified in-debugger that we're getting back what we should in the unit tests.)


    All runs well. Same branch: https://github.com/stevenaw/popcorn-js/tree/t543

    Commit: https://github.com/stevenaw/popcorn-js/commit/99e012d654ac53ecbc05b...

  • Rick

    Rick October 28th, 2011 @ 02:20 PM

    • Assigned user changed from “David Seifried” to “Steven W”
    • State changed from “peer-review-requested” to “review-needs-work”
  • Steven W

    Steven W October 29th, 2011 @ 01:02 AM

    The "if ( data.xml ) { data.xml = null; }" line is in there as an IE 9 compatibility fix, because IE9 doesn't seem to have a null responseXML when it can't parse the response by the browser: it has a XmlDocument object with a null documentElement. So that condition attempts to normalize this and set the value to null.

    You make a good point though about it not being very intuitive, so I've made a change. Setting it to null when an exception is caught gives the same result because IE is also the only browser that throws an exception in DOM Parser, so it still normalizes. My concern is just that if IE changes their DOMParser implementation to the standard way of returning a parsererror element, then we open up the possibility of data.xml not being null in some browsers when invalid xml is parsed in IE.

    I'm proposing setting data.xml to null no matter what just after "if ( !data.xml || !data.xml.documentElement )". It reads clear, accomplishes the desired end result, and is potentially a little more future proof. Let me know your thoughts, I've pushed here: https://github.com/stevenaw/popcorn-js/commit/aa2b59aed99f1f7077325...

  • Rick

    Rick October 29th, 2011 @ 09:59 AM

    "It reads clear, accomplishes the desired end result, and is potentially a little more future proof. " These are my favorite things. Great fix btw. Can you actually add the above explanation as a comment right in the code? Take a look at this: http://www.1729.com/blog/ExtremeNegativeCodeDocumentation.html for an idea of why I ask for that :)

  • Steven W

    Steven W October 29th, 2011 @ 12:18 PM

    • Assigned user changed from “Steven W” to “Rick”
    • State changed from “review-needs-work” to “peer-review-requested”

    Certainly, and thanks for the link. I'm actively trying to learn the right balance between too much unnecessary comments, too little necessary, and using program structure to bridge the two.

    I've re-pushed the ticket, could you please PR this one Rick?

    Last Commit: https://github.com/stevenaw/popcorn-js/commit/6120d9b2d084577575413...
    Branch Changes: https://github.com/stevenaw/popcorn-js/compare/develop...t543
    Whole Branch: https://github.com/stevenaw/popcorn-js/tree/t543

  • cadecairos

    cadecairos October 31st, 2011 @ 11:54 AM

    • Assigned user changed from “Rick” to “cadecairos”
  • cadecairos

    cadecairos October 31st, 2011 @ 01:50 PM

    • Assigned user changed from “cadecairos” to “David Seifried”
    • State changed from “peer-review-requested” to “super-review-requested”

    Cool fix, this should stop all that nonsense with bad mime types.

    Tested: Core Unit tests, TTML unit tests

    Passing in: Firefox 7, Chrome 15, Safari 5.1, Opera 11.52

    Core and TTML pass lint.

    Style looks good.

  • Scott Downe

    Scott Downe November 1st, 2011 @ 09:56 AM

    • Assigned user changed from “David Seifried” to “Scott Downe”

    Taking a few reviews off Dave's shoulders.

  • Scott Downe

    Scott Downe November 1st, 2011 @ 10:24 AM

    • Assigned user changed from “Scott Downe” to “cadecairos”
    • State changed from “super-review-requested” to “review-looks-good”

    Full core test pass in chrome and FF. Known failures in Opera's language stuff, rest is pass.

    Full linting pass on parsers and core.

    Parser tests passing in FF Chrome and Opera.

  • cadecairos
  • Rick
  • Rick

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Pages