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

Fix canvas.toDataURL yielding blank image. #106

Merged
merged 4 commits into from
Jan 9, 2017

Conversation

rasmusvhansen
Copy link
Contributor

Added preserveDrawingBuffer as option to getContext for webgl.
Tested in Chrome and IE11

Can be tested as follows:

  1. Go to http://phoboslab.org/log/2013/05/mpeg1-video-decoder-in-javascript
  2. Run the following snippet in the console
  3. Scroll to the bottom and see the blank image.
var img = document.createElement('img');
img.src = player.canvas.toDataURL('image/webp', 0.7)
img.style.border = '1px solid red';
document.body.appendChild(img);

Added `preserveDrawingBuffer` as option to getContext for webgl.
Tested in Chrome and IE11
@phoboslab
Copy link
Owner

phoboslab commented Jan 8, 2017

Interesting. Thanks for this! However, preserveDrawingBuffer has some performance implications. So I think this should only be enabled through a flag in the options when creating the player, or better yet, through a new method on the player.

I.e. calling player.toDataURL('image/webp', 0.7) would enable preserveDrawingBuffer (or re-create the context!?) and then call player.canvas.toDataURL() internally.

Also, StackOverflow suggests that you don't even need to have preserveDrawingBuffer enabled; the data URL would just need to be captured directly after rendering to the canvas, before the canvas is presented in the browser. Of course this would complicate things when we want to call it on a paused player etc.

@rasmusvhansen
Copy link
Contributor Author

rasmusvhansen commented Jan 9, 2017

On a desktop machine, it does not seem to affect performance noticably. I did some rudimentary testing with 8 simultaneous 640x480 streams in Chrome (which is about the limit for smooth streaming on my machine it seems) and the performance seemed to stay the same.
However, the StackOverflow page you linked to also hints at that it will be more of an issue with mobile OS browsers.

I don't know the code well enough to implement enabling/disabling preserveDrawingBuffer dynamically when calling toDataUrl plus I also need it to work for canvas.toBlob.

I have added it as an option to the constructor, so users of JSMPEG can choose whether or not to enable it.
Let me know if that is an acceptable solution.

@phoboslab phoboslab merged commit 6a32d86 into phoboslab:master Jan 9, 2017
@phoboslab
Copy link
Owner

Yep. Since toDataURL() is not the common use-case for this lib, this is a perfectly fine solution. Thanks!

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

Successfully merging this pull request may close these issues.

2 participants