koder icon indicating copy to clipboard operation
koder copied to clipboard

wasmWorker example: Add option to receive empty response when QR not found

Open JL102 opened this issue 7 months ago • 3 comments

Issue #, if available: N/A

Description of changes: In my own app, I was unable to send messages to the worker too frequently, lest I send messages while it's still working, leading to the worker stalling over a pile of work that it can never catch up on, delaying the time it takes to actually register a successful scan.

In my use case, I was able to remedy this by having the worker respond with data/type undefined, and then once the app receives word that the worker is no longer busy, it's free to send another snapshot from the camera. This significantly improved the performance of my scanner, because I can essentially always keep the worker busy & not be afraid of piling it with more work than it can handle.

I added it under the optional event.data.alwaysRespond, because this is a change in the API behavior, and therefore the programmer must update the way they call the worker in order to take advantage of this change. (i.e., if you request alwaysRespond: true, then you can no longer assume that data is present when receiving a message from the worker. So it makes sense to me to have this an "opt-in" change.

Side note: I noticed that public/jsQrWorker.js has a console.log statement where wasmWorker doesn't. I didn't include a change to it in this PR cuz it's outs of the scope of this issue, but I thought I'd mention it if you didn't realize it was there.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

JL102 avatar Nov 09 '23 17:11 JL102

Commit https://github.com/maslick/koder/pull/87/commits/112272897de530fde93333d21278dcf681dc0de3 was because I saw a diff on the last line of jsQrWorker.js, so I thought it was because i accidentally deleted a newline at the end of the file, but the diff is still there. Not sure what the deal is with that.

JL102 avatar Nov 09 '23 17:11 JL102

Hey @JL102, why can't you adjust the scan rate (see https://github.com/maslick/koder/blob/master/src/index.js#L8) instead? It's set to 250 ms by default, resulting in 4 images per second sent to the Web worker. In my experience, especially with iPhones 11 Pro, 13 Pro, and 14 Plus, koder can handle image processing at 10 ms (100 images per second) and below without any problem. I personally find 250 ms optimal for balancing performance and usability. Anything below 100 ms seems like a bit of a resource overkill. What are your thoughts on adjusting the scan rate?

maslick avatar Nov 09 '23 18:11 maslick

The app I'm working on may be run on a wide variety of devices, including older and slower phones. Also, sometimes the device/browser may give a video stream that's a bit higher resolution than expected, which takes longer to parse. For me personally, "blindly" adjusting the scan rate without the main thread receiving any concrete feedback on how long the parsing really takes makes me really uncomfortable. When I was playing with different video resolutions, I was encountering issues where the web worker was overloaded with requests as described above, and I didn't want that to happen again, so I turned it up to 500 ms but then found that the scanning experience was terrible and inconsistent. For me, being able to know for sure when the web worker has stopped scanning the last frame gives me the peace of mind necessary to push it further with more frames.

That being said, you're probably right about it being a bit overkill to crank up the frequency too high, so I might cap the scan rate anyways instead of requesting a new scan as soon as it's possible. I'll do some tests to see where the point of diminishing returns is, and cap it there.

About the PR: I've copied the WASM and JS into the project's source (with my suggested changes) so I'm not dependent on the PR being accepted & uploaded to a new version on NPM. I just figured I'd submit it as PR because it's an upgrade IMO, in case others would benefit from it too. If you don't want to merge it, that's alright!

JL102 avatar Nov 10 '23 03:11 JL102