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

Support Safari 12.1, fix canvas leak, and improve recognition #214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oskarpearson
Copy link

https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/srcObject

On Safari 12 (and indeed later verisons of other browsers), the video.src = URL.createObjectURL(mediaStream); syntax is deprecated. Instead, it's
been replaced by video.srcObject.

However, using the srcObject syntax also lead to hundreds of individual canvases
being created in Safari 12.1. This used lots of memory, slowing the app, and
Safari then stopped the website from creating further canvasses and scanning
failed to work. The longer the video preview was open the worse things became.

This version of the code now uses a single canvas for rendering, rather than
relying on garbage collection to clean them up over time.

Additionally, Safari requires a new playsinline attribute for the display
element in the html.

Additionally, I found that on Safari 12.1 with an iPhone 6s plus, the generated
image was vertically squished, and the bar-code wouldn't be recognised as it
was not square. This changes the code to use the rendered window size for the
canvas, rather than what the camera has been detected as. This seems to
dramatically improve QR code recognition.

@oskarpearson
Copy link
Author

Based on work done in #164 - thanks!

@oskarpearson
Copy link
Author

Example where the image is squished

screen shot 2018-12-06 at 17 32 17

https://developer.mozilla.org/en-US/docs/Web/API/HTMLMediaElement/srcObject

On Safari 12 (and indeed later verisons of other browsers), the `video.src =
URL.createObjectURL(mediaStream);` syntax is deprecated. Instead, it's
been replaced by `video.srcObject`.

However, using the srcObject syntax also lead to hundreds of individual canvases
being created in Safari 12.1. This used lots of memory, slowing the app, and
Safari then stopped the website from creating further canvasses and scanning
failed to work. The longer the video preview was open the worse things became.

This version of the code now uses a single canvas for rendering, rather than
relying on garbage collection to clean them up over time.

Additionally, Safari requires a new `playsinline` attribute for the display
element in the html.

Additionally, I found that on Safari 12.1 with an iPhone 6s plus, the generated
image was vertically `squished`, and the bar-code wouldn't be recognised as it
was not square. This changes the code to use the rendered window size for the
canvas, rather than what the camera has been detected as. This seems to
dramatically improve QR code recognition.
@oskarpearson
Copy link
Author

Heya @bitjson

Not sure if you've seen this yet. Would you let me know your thoughts?

Thanks!

@1001daysofcode
Copy link

Any update on this? @bitjson

@@ -408,6 +438,7 @@ module.exports = function(){
var video = getVideoPreview();
if(video){
video.src = '';
video.srcObject = '';
Copy link

@fujiwaraizuho fujiwaraizuho Mar 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'' is not an Object
You should substitute null here.

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.

3 participants