Add SubStation Alpha (SSA) support

[avelad]

Add SubStation Alpha (SSA) support

Format info: https://en.wikipedia.org/wiki/SubStation_Alpha

Note: SSA and ASS are supported

[avelad]

Before merging this, it is necessary to previously accept #3036 so that I can add a part that is the same for the conversion to WebVTT in src=.

[avelad]

Merge ready, it is now possible to review this PR.

[avelad]

@joeyparrish , do you think you could find a time this week to review this?

[TheModMaker]

I think it would be easier to understand if you split the raw parsing and the name handling into separate parts. For example, have a parse method that produces a nested object map:

{
  "Script Info": {
    "Title": "Foo",
    "Original Script": ""
  },
  "V4+ Styles": {
    "Format": "..."
  }
}

[avelad]

I'm going to think of a way to make it easier, and hope to come back tomorrow with a different implementation.

[avelad]

Done!

[TheModMaker]

I was suggesting a more generic parser that didn't check for tag names. For example, something like:

const ret = {};
let section = null;
for (const line of lines) {
  const match = /\[([^\]]+)\]/.exec(line);
  if (match) {
    section = match[1];
    ret[section] = {};
  } else if (/\s*;/.test(line)) {
    // Skip comments.
  } else {
    const parts = line.split(':');
    ret[section][parts[0].trim()] = parts.slice(1).join(':').trim();
    // You'll need to special-case Dialogue to support multiple cues.
  }
}

I would also suggest putting this in another function to convert the data buffer to the object. Then you can just use data['V4 Styles'] or data['Events'] later.

[avelad]

Given that it is a popular format, what is the advantage of doing what you propose? At this time, what is supported are known tags and those that we add to sections. Adding unknown sections I don't think has any advantage ... Sorry if I didn't understand you.

[TheModMaker]

Your code "adds" unknown sections too by ignoring them in the if statements below. I was suggesting we separate the text parsing from the handling of the sections. Like you would split XML parsing from the reading of the elements. By splitting them up, you can focus on reading the tag names without seeing all the regex and string parsing.

[avelad]

Sorry, I still don't see it :(. Right now with the last changes the events and styles are already separated, and also the parsing of them is made independent. Adding a generic parse here does not add much since it is a known format.In your example above, script information is added to the object that is not really useful.
On the other hand, even if I do a generic parse, I still have to know the tags.
The execution time of my proposal and yours should be similar.

[avelad]

@TheModMaker I have applied all your comments. Can you review again?

[avelad]

Ready for review!

[TheModMaker]

Since this just forwards, I suggest just using parseMedia directly.

[avelad]

We cannot do this because getCues is static and is necessary in the player.

[TheModMaker]

Part of my generic suggestion was to avoid checking if (mimeType == 'foo') and instead use the parser interface directly. Doing something like this:

const factory = TextEngine.findParser(mimeType);
if (factory) {
    const obj = factory();
    cues = obj.parseMedia(data);
}

But you don't have to do that now. You could do it in another PR or I could do it instead.

[avelad]

I'm going to address it in another PR, I keep it on my radar. Thanks!

[TheModMaker]

I was suggesting a more generic parser that didn't check for tag names. For example, something like:

const ret = {};
let section = null;
for (const line of lines) {
  const match = /\[([^\]]+)\]/.exec(line);
  if (match) {
    section = match[1];
    ret[section] = {};
  } else if (/\s*;/.test(line)) {
    // Skip comments.
  } else {
    const parts = line.split(':');
    ret[section][parts[0].trim()] = parts.slice(1).join(':').trim();
    // You'll need to special-case Dialogue to support multiple cues.
  }
}

I would also suggest putting this in another function to convert the data buffer to the object. Then you can just use data['V4 Styles'] or data['Events'] later.

[TheModMaker]

Your code "adds" unknown sections too by ignoring them in the if statements below. I was suggesting we separate the text parsing from the handling of the sections. Like you would split XML parsing from the reading of the elements. By splitting them up, you can focus on reading the tag names without seeing all the regex and string parsing.

[TheModMaker]

Part of my generic suggestion was to avoid checking if (mimeType == 'foo') and instead use the parser interface directly. Doing something like this:

const factory = TextEngine.findParser(mimeType);
if (factory) {
    const obj = factory();
    cues = obj.parseMedia(data);
}

But you don't have to do that now. You could do it in another PR or I could do it instead.

[shaka-bot]

Test Failure:

Generating Closure dependencies...
Linting JavaScript...

/var/lib/jenkins/workspace/Manual PR Test (local-tests)/lib/text/ssa_text_parser.js
272:11  error  'alpha' is never reassigned. Use 'const' instead  prefer-const

✖ 1 problem (1 error, 0 warnings)
1 error and 0 warnings potentially fixable with the `--fix` option.

END-BUILD: FAILURE
Build step 'Execute shell' marked build as failure

[avelad]

Fixed lint error. Sorry....

[shaka-bot]

All tests passed!