rewrite: "v2"

[ThaUnknown]

This is a full rewrite of the JS of JSSO, meaning it includes breaking changes, hence the 'v2'.

I have used ES2020, but that can easily be compiled down by the user with the likes of babel or other, I however didn't force any reliance on new API's, nor have I removed any backwards compatibility, which means this [hopefully, pls check @dmitrylyzo ] should still run on any legacy browsers [after compiling using babel]. Maybe we could additionally provide a version of JSSO which is already babel'ed so the user doesn't have to do it? That's another topic.

I changed a lot of things, which means more can be changed. If you think something should be further changed, please say so.

The README still needs to be updated, but that always comes last.

The reason it's a single commit is because it was a full rewrite, rather than bit by bit patching, and since there's a "no broken commits" policy here, this is the only way I could imagine of doing it.

In general this is focused on performance, performance, performance [where it matters] and bugfixes.

Overview:

Structure:

• now using classes, so the structure is a lot less chaotic
• now extending EventTarget
  • this allows the user to run so.addEventListener and so.removeEventListener rather than using so.oncustomeventname
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L477
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L543
• using standardjs as a linter, because seeing 2 different code styles, sometimes even in the same commit was painful
• using const, let, destructing, anon functions, null coales, and other
• things the user shouldnt use are prefixed with _ so they show up at the end of stuff like console inspect etc.
• using nodejs standards for things like private attribute naming _, callback styles etc
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L444
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L511
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L501

Features:

• removed "render mode" replaced by "blending mode: wasm/js"
  • this allows the user to "mix and match" rendering "modifiers" of which there are now a few, like for example: wasm, async,offscreen and onDemand, or js, sync, main, event
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L165
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L178
  • the default is now offscreen, ondemand, async, wasm rendering, which turns off parts of it if the browser lacks support
• made asyncRender a modifier, which means you can now use it with blendrender
• created offscreenRender, a modifier which allows you to handle all the rendering on the worker using OffscreenCanvases [chrome only, firefox very soon]
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L58
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L86-L87
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L253
• created onDemandRender, a modifier which renders subtitles as the video player decodes video frames, rather than relying on events, this makes it insanely reliable to paint subtitles [currently chrome only, firefox shows interest but isnt working on this yet, for browsers which don't support it, it's polyfilled]
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L444-L448
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L238
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L577

Performance/bugs:

• async/offscreen/ondemand implementation/changes
• JS blend decode is now a LOT faster than before, thanks to some uint32array and byteshift magic
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L213-L226
  • I however don't know how this will behave on devices whos endianness is different from x86, can some1 test this?
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L222
  • the whole decode needs to be done inside WASM, rather than on JS, this code is now a lot faster, but it still can be a lot faster if it was done inside WASM
• the browser will no longer freeze when a CSS animation resizes the canvas
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L170
  • for instant dimension changes [fullscreen change, or flat resize] canvas will update instantly
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L177-L182
  • for any dynamic dimension changes [resizing window, css animation, etc] JSSO will wait for the animation to finish before asking for a canvas re-draw
    https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L169-L175
• the canvas will no longer randomly disappear [removed resize with timeout which was a very lazy hack]
• the canvas will no longer loose content when changing position without changing dimensions
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L165
• when resizing, the content of the canvas doesn't disappear, just gets rescaled until the next frame is painted
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L177-L179
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L109-L112
• a few idiot checks for people making dumb mistakes
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L246
• attempt to fix high GPU VRAM usage caused by asyncRender's ImageBitmaps, by closing them, but this doesn't reduce it by all that much
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/subtitles-octopus.js#L456
  https://github.com/ThaUnknown/JavascriptSubtitlesOctopus/blob/v2/src/post-worker.js#L259

Test link: https://thaunknown.github.io/JavascriptSubtitlesOctopus/ts

[TFSThiagoBR98]

I have used ES2020, but that can easily be compiled down by the user with the likes of babel or other, I however didn't force any reliance on new API's, nor have I removed any backwards compatibility, which means this [hopefully, pls check @dmitrylyzo ] should still run on any legacy browsers [after compiling using babel]. Maybe we could additionally provide a version of JSSO which is already babel'ed so the user doesn't have to do it? That's another topic.

Isn't it better to include babel and eslint as devDependencies and compile it along with the worker, instead of include the compiled version.

[ThaUnknown]

I have used ES2020, but that can easily be compiled down by the user with the likes of babel or other, I however didn't force any reliance on new API's, nor have I removed any backwards compatibility, which means this [hopefully, pls check @dmitrylyzo ] should still run on any legacy browsers [after compiling using babel]. Maybe we could additionally provide a version of JSSO which is already babel'ed so the user doesn't have to do it? That's another topic.

Isn't it better to include babel and eslint as devDependencies and compile it along with the worker, instead of include the compiled version.

Most likely, yes. But that's outside of the scope of this PR, I'd rather focus on the functionality/code

[ThaUnknown]

I put up a "small" test case for this, ~300-500 bitmaps per frame, on firefox this is likely to crash, runs fine on chrome tho
https://thaunknown.github.io/JavascriptSubtitlesOctopus/ts

[heyaco]

@ThaUnknown subs do not show in fullscreen mode

[ThaUnknown]

@ThaUnknown subs do not show in fullscreen mode

use f11, not video controls

[ThaUnknown]

updated example video with some CSS animation showcases

[ThaUnknown]

@TFSThiagoBR98 @dmitrylyzo @TheOneric does anything need to be changed for this PR for it to be reviewed? At most I could add URLs to code snippets of the fixes/features if thats required

[TFSThiagoBR98]

If possible, can you add JSDoc comments to subtitle-octopus.js? I'll use that to generate the definition files for #95.

At most I could add URLs to code snippets of the fixes/features if thats required.

That may help

[ThaUnknown]

If possible, can you add JSDoc comments to subtitle-octopus.js? I'll use that to generate the definition files for #95.

this is some of the least fun stuff I've ever done

[ThaUnknown]

That may help

added code snippets to wherever it was possible

[ThaUnknown]

@TFSThiagoBR98 @TheOneric I've pulled upstream, resolved conflicts

is there anything to be done to get this PR to move, or is it simply a case of "this PR is trying to do too much" and noone is willing to review it?

[ThaUnknown]

additionally this PR is a fix to #46 [as the renderer won't request more subtitles under load], partial fix to #72 [perf optimisations], full fix to #97 [as onDemandRender doesn't use event listeners, and destory() removes listeners], partial fix to 1001432683[as you can use _demandRender, omitting JSO's render loop], and partial fix to #8 [only for ondemand render]

[TheOneric]

The patch requirements have been explained before and serve to ensure long-term maintainability and reducing the chance of new bugs slipping in. Reducing them to "no broken commits" is leaving out several key points. Most notably this fails at not bundling logically unrelated changes and proper documentation. Some of the listed changes also appear to have API implications. The list in OP with pointers to individual code section strongly indicates, that a separation is very much possible.

[ThaUnknown]

The patch requirements have been explained before and serve to ensure long-term maintainability and reducing the chance of new bugs slipping in. Reducing them to "no broken commits" is leaving out several key points. Most notably this fails at not bundling logically unrelated changes and proper documentation. Some of the listed changes also appear to have API implications. The list in OP with pointers to individual code section strongly indicates, that a separation is very much possible.

while separation is possible, this means loosing functionality in the initial PRs as i didn't modify JSO, but deleted all of it and started from 0, for my own sanity

are you okay with PRs like that?

[TheOneric]

There must be “no temporary user-visible behaviour changes” (and permanent changes need to be justified). Start from the existing code and change it one step to be review- and understandable.

[ThaUnknown]

There must be “no temporary user-visible behaviour changes” (and permanent changes need to be justified). Start from the existing code and change it one step to be review- and understandable.

I don't see how that can happen with a full rewrite of the JS part of the library

to put it simply the current js is close to unmaintainable, be because it was made from a fork of a fork of a library, you even have some of the original libassjs code in here, taking the code apart like that to make massive changes will take weeks