zxing-js / library

Multi-format 1D/2D barcode image processing library, usable in JavaScript ecosystem.
https://zxing-js.github.io/library/
Apache License 2.0
2.45k stars 542 forks source link

Run decoding algorithm in a web worker? #42

Open simonedavico opened 6 years ago

simonedavico commented 6 years ago

First of all, thank you for this nice module!

I used it in my mobile PWA to show the video stream from the camera in fullscreen and decode a QR code. I also overlay some controls (such as a "Cancel" button) on top of the fullscreen video.

I noticed that while the video is streaming/the library is trying to decode the QR code, the UI becomes a little janky. In order to leave the main thread free, I was thinking of running the decoding algorithm in a web worker. Since I don't know how CPU intensive the QR code decoding algorithm really is, I was looking for some feedback on the idea. Would it improve the overall performance of the library to give such a possibility?

odahcam commented 6 years ago

The library core is good, but there's lots of complex tasks that can be a little heavy in some devices. Of course there's lots of otimizations we can do, but we are at the very beginning of the package yet. :x

I'm pretty sure some part of the algorithm can run in a web worker, but none of the browser classes should work in there because they're built on top of DOM features. So if you manage to include some classes that work with pure data and then pass some data collected from DOM to a web worker and read the results through some event listener you should be good to go.

I'm a little newbie with workers, but you ideia seems nice. It could be better if the bundle was more three shakable and that's because I'm no wizzard in webpack too. 😅

As a final thought, I think we should investigate this and grow the ideia as we improve our tooling.

simonedavico commented 6 years ago

Thank you for your response @odahcam! I know that the interaction with the camera needs the DOM since it uses video and canvas. I was thinking of moving the algorithm involved in the decoding of the image. Is that part of the library dependant on the DOM as well? My intuition tells me no, but I didn't explore the codebase thoroughly yet.

odahcam commented 6 years ago

Well, we have two sections/areas/what you wanna call'em: browser and core.

It works, but I'm planning to improve this separations futher ahead.

simonedavico commented 6 years ago

So theoretically if we improve the browser related interfaces/classes to run the core inside a worker we would block the main thread less, right?

I'll try to think about what would be the best way to evolve the browser part to support this in an opt-in, backwards compatible way. Suggestions/ideas are welcome :)

odahcam commented 6 years ago

I left some ideas on Gitter about sepparating the core and the browser layer. I don't like to mantain a browser layer here (in this repo) while the ZXing original repo doesn't have that layer, this breaks the port concept in my opinion.

I would like to improve the core itself here and separate the browser wrappers in their own package/repository/thing like we do with other readers. There would be some advantages, like:

Also, I would like to include @werthdavid in the discussion.

werthdavid commented 6 years ago

I agree with @odahcam the core library should be there to just decode the data. Even though I understand that it is the desirable to have the performance advantages of a web-worker I think we should split the library a suggested in gitter.

odahcam commented 6 years ago

Me and @andrevargas get to work in a example that's able to fetch a arrayBuffer from a image and then passes it to some decode method in the needed format, this will run on React Native and can also run in a Web Worker. 😄

simonedavico commented 6 years ago

@odahcam that's great! Any fork/branch where I can see the code? I'd love to contribute :)

odahcam commented 6 years ago

No branches for now, as soon as we have one I will update here. 🙂

github-actions[bot] commented 5 years ago

Stale issue message

plague69 commented 4 years ago

Might be interesting, these guys have it working Zxing-JS+Comlink, but only for react https://github.com/pocesar/react-use-qrcode

odahcam commented 3 years ago

Sorry for the bot inconvinience, let's keep this one open.


Thanks @plague69 for the link, that is very interesting. It seems this wouldn't be a thing to implement inside the project, but to document how users could benefit from a web worker to run the library inside. And this Comlink implementation seems very simple and straight forward, I wonder how difficult is to use it in pure JS.