JavascriptSubtitlesOctopus
JavascriptSubtitlesOctopus copied to clipboard
WASM brotli broke some compressed subs
While 128be61fe30ec9039192c6dda91c714ef03051e7 didn't affect the “Brotli Compressed Subtitles” test, as it turns out it causes issues with more complex brotli-compressed subs like the Railgun S or SnK openings. The former only renders the first karaoke line and nothing afterwards, the latter only shows the colour blobs of the first karaoke line and nothing else. Sorry for not noticing earlier. As a – hopefully temporary – hotfix I reverted the change.
@WeebDataHoarder: do you want to look into this and try to find a fix so it can be reapplied?
I believe I have a patch ready for this, but it involves bringing along custom TextDecoder polyfill (which is what brotli.js did on their own).
Great! Needing one more polyfill for old engines seems unproblematic. considering we basically already have this and a license-compatible polyfill is available. Can you explain though what the problem is and why we need to deal with text encodings for brotli subs? The compressed version is opaque binary data and given libass never receives an encoding parameter from JSO, the uncompressed version must always be UTF-8 to be understood.
@WeebDataHoarder: Looking at the wip(?) commit you pushed to the “lazy font” PR, there's a comment about “future emscripten versions” adding the required polyfill on their own. Did you notice emscripten was updated to 3.1.15 (back then the most recent version, now it’s 3.1.17) before this issue was opened or is this about intended/post-3.1.15 emscripten changes?
This was an update done mainly on my side to see how the new versions behaved, haven't checked recent updates lately. ------- Original Message ------- On Friday, July 29th, 2022 at 11:23 PM, TheOneric @.***> wrote:
@.***(https://github.com/WeebDataHoarder): Looking at the wip(?) commit you pushed to the “lazy font” PR, there's a not about “future emscripten versions” adding the required polyfill on their own. Did you notice emscripten was updated to 3.1.15 (back then the most recent version, now it’s 3.1.17) before this issue was opened or is this about intended/post-3.1.15 emscripten changes?
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>
fyi, I just tested master + 128be61fe30ec9039192c6dda91c714ef03051e7 + https://github.com/WeebDataHoarder/JavascriptSubtitlesOctopus/commit/35a92168cd1acb30ca1fc9330f6fa81934305265 and the same issue as before still persists. It appears TextDecoder wasn't the only problem.
I tentatively deprecated in-house brotli decompression in e459c8e6ffa83f2244f9745f54c39a19edd9684f recommending transparent (de)compression via Content-Encoding to be used instead. If no one complains that Content-Encoding was unusable on some platform, it'll eventually be removed completely and the issues with moving to WASM-brotli will be obsolete.
After the release, manual decompression was now dropped in c975a29e69655a05e3e8f1de05d3725a00e61d33.
If this should prove problematic this can be reverted, otherwise the manual decompression headaches and confusions are now gone for good and it won't interfere with the more flexible Content-Encoding: anymore.
For those who came here in search what to use instead, here’s the relevant bit from 4.1.0’s README:
Brotli Compressed Subtitles (DEPRECATED)
Manual support for brotli-compressed subtitles is tentatively deprecated and may be removed with the next release.
Instead use HTTP's Content-Encoding: header to transmit files compressed and
let the browser handle decompression before it reaches JSO. This supports more
compression algorithms and is likely faster.
Do not use a .br file extension if you use Content-Ecoding: as this will
conflict with the still existing manual support which tries to decompress any data
with a .br extension.