Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Question] Supported browsers #14

Closed
nickfujita opened this issue Oct 20, 2017 · 5 comments
Closed

[Question] Supported browsers #14

nickfujita opened this issue Oct 20, 2017 · 5 comments

Comments

@nickfujita
Copy link
Contributor

Moving the discussion from libjass issue#48

Is there a list of supported browsers for this library? I've already started some testing, but seems there are some tweaks here and there to get some of the things working.

I am looking to support the following:
Desktop

  • IE
  • Edge
  • Firefox (tested and confirmed)
  • Chrome (tested and confirmed)
  • Safari (tested and confirmed)

Mobile web iOS

  • Safari (tested and fails with error "no native wasm support detected")
  • Chrome (tested and appears to fail, not able to attach debugger to chrome on iOS, but assuming it's due to no native wasm support?)

Mobile web Android

  • Chrome (tested and seems when attaching to video element, styling of canvas is set to display: none, and styles for positioning are missing?)
  • Android browser

Native iOS

  • WKWebView (tested and fails with error "no native wasm support detected")

Native Android

  • webkit.WebView (tested and confirmed)

The 2 major issues i'm seeing are:

  • no native wasm support on iOS. Is there a backup solution for this case?
  • canvas display and positioning on Android Chrome when attaching to video element via playerjs
@nickfujita
Copy link
Contributor Author

After further investigation, I did find the fallbacks for the error "no native wasm support detected", and they are in fact working in older browsers. The reason I thought it wasn't working was that there were no subtitles being displayed, and the error was in the console. However, it looks like it is the same issue that I was seeing on Chrome in mobile web for Android, where the CSS display property was just set to display: none, and the positioning related styles were not applied. After resizing the window, it seems that the internal "resize" method is called, and the styling for the canvas is set, and all is well with the rendering. I am not seeing this issue with the demo page in this repo, but only in my project, where there is a lot more code running at the same time, and it is in a Iframe/webview. I suspect that there is a race condition that is not being handled, where the worker is taking longer than expected, and the "resize" function in "setVideo" is being called before it's ready. So ensuring that we call resize once the worker is ready should fix this.

As a test, I added a call to resize if the css display property of the canvas was none on the canvas targeted postmessage to confirm the theory, and it seems to "fix" the issue:
https://github.com/Dador/JavascriptSubtitlesOctopus/blob/master/build/subtitles-octopus/subtitles-octopus.js#L241

if (self.canvas.style.display == 'none') {
  self.resize();
}

However, the effect is that there is a animation of the subtitles to their new coordinates after the resize method is called. So I would imagine that this call needs to be made a bit earlier, before the first subtitle is rendered to canvas, but after the worker is ready with the canvas.

@Dador
Copy link
Collaborator

Dador commented Oct 25, 2017

Both width&height of <video> element and width&height of video itself used in calucation inside resize(). If some of them are unavailable, then it doesn't show canvas.
There is check "is video loaded" and script is waiting for it (loadedmetadata).
But there is no check like "is video element shown and has width&height > 0", maybe that's the cause of the bug you describing. That's not that easy to resolve because there is no event "on element resize" or "on element shown" (but we can add setTimeout and periodically check is element shown), so first we should make sure this is the reason of your problem.

My guess is that you creating video element, init subtitles and only after you're showing video element (or other JS code showing it after subtitles already inited).

You could try init subtitles only after video is elemnt displayed.
Also you could try call console.log('video dimensions', video.offsetWidth, video.offsetHeight, video.videoWidth, video.videoHeight) right after subtitles init. If first 2 numbers zeroes then it confirms the guess.

P.S. As of general browser support, I'm aiming to support all browers that worth supporting (i.e. subtitles are playable with adequate performance and browser supports HTML5 video with H264).

@nickfujita
Copy link
Contributor Author

You were right! I was instantiating both the video player and the subtitles at the same time, and the video element was not ready in time for the initial sizing in the subtitle lib. I tried to delay initializing until the video element emits a 'loadeddata' event to indicate that the content is loaded, and video is sized. However, initializing the subtitles does take some time, so instead I am continuing to init them both at the same time, but will call for a resize on the video 'loadeddata' event instead. This seems to resolve the issue, and doesn't cause any delays or strange animations for the subs in the first frame.

That issue aside, while testing in Firefox, I am able to see subs rendering properly in the demo page, but when using the lib in my app, there seems to be an error emitted from the worker when posting 'worker-init' to the worker.

Worker error:  error {
  target: Worker,
  isTrusted: true,
  message: "RuntimeError: index out of bounds",
  filename: "<my_domain>/subtitles-octopus-worker.js",
  lineno: 245549,
  colno: 0,
  currentTarget: Worker,
  eventPhase: 2,
  bubbles: false,
  cancelable: true,
  defaultPrevented: false
}

I was not able to successfully track down the exact location of the error by lineno since the worker file is minified. However, I did trace it back to the call to removeRunDependency('worker-init'); in the post-worker.js file. Simply commenting out this line stops the worker error from occurring, but the result is that nothing is rendered to the canvas. I did confirm that the canvas is created and sized appropriately to the video; so it is not the same issue as before. It just seems that there is nothing being rendered to the canvas at this point. I also tried to delay the call to init the worker just incase the worker was still loading due to high resource consumption on my app, but am seeing the same result.

Any thoughts? I really appreciate your help on this!

@Dador
Copy link
Collaborator

Dador commented Oct 26, 2017

Emscripten runs "program" after all dependecies are resolved, so removing removeRunDependency('worker-init'); basically stops program from starting. Probably problem lays down deeper than error when calling removeRunDependency('worker-init');.

Line number looks strange (too large). You could try to wrap worker code in try catch. Even if you're not ready to build it by yourself, you could just wrap already generated js. Also make sure that you use latest version of all files.

@nickfujita
Copy link
Contributor Author

Seems that a fresh copy of the lib files from latest master did the trick! So far looking good on all browsers after tweaking my integration a bit!

Thanks again for the help!

JustAMan added a commit to JustAMan/JavascriptSubtitlesOctopus that referenced this issue Apr 27, 2020
Use patched emscripten so Octopus would work in Cordova
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants