ericblade / quagga2

An advanced barcode-scanner written in Javascript and TypeScript - Continuation from https://github.com/serratus/quaggajs
MIT License
765 stars 85 forks source link

fix/set dimensions greater or equal patch size #507

Closed hadrien-f closed 1 year ago

hadrien-f commented 1 year ago

Hello,

I have been using quagga2 successfully in my project, thank you for your work !

We only use the api method Quagga.decodeSingle for our needs and I have found a random error :

IndexSizeError DOMException: Index or size is negative or greater than the allowed amount

This error is thrown by the browser when we create a canva of size 0.

After digging into the project I identified that the canvas could be of size 0 when the BarcodeLocator.checkImageConstraints method would set the width and height to 0 with some images.

This pull request fixes the issue by adding a minimum size equal to the patchSize.

I tried running the project tests but some failed. For exemple the End-To-End Decoder Tests with Quagga.decodeSingle image-006.jpg with the following error :

Trace: * error during test AssertionError: expected '419056690690101' to equal '412056690699101'

If the fix makes sense for you it would be nice to add it to the project, otherwise I can try harder to create easier way to reproduce the error.

rollingversions[bot] commented 1 year ago

There is no change log for this pull request yet.

Create a changelog

ericblade commented 1 year ago

Thank you! I will definitely take a look at this, and compare the test results before and after. At a glance, it does seem like the change makes sense.

Many of the tests that I inherited with the original test system from before I forked the lib, also failed under that system, and I kept them as I wanted to try to get all tests to pass eventually. So far, there has been a little movement in that direction, but not as much as I'd hoped.

I'm hoping that finding glitches like this will improve things :)

If you'd like, you may add yourself to the contributors list in package.json . And if you have a known way to reproduce the problem, adding a test specifically for it would be really helpful!

Thank you!

hadrien-f commented 1 year ago

Thanks for the quick response.

I am not surprised the change does not make sense since I did it without understand well the algorithm behind it.

However I did not find that the change made the scanner result worse.

It is not easy to add a test case for such a edge case for me since I don't know the code but I will try if I have the time.

I will also try to find another solution.

ericblade commented 1 year ago

oh, no, the change looks like it does make sense. I will need to spend some time looking at it a little closer, this is deep in the stuff that I try to really understand any effects that might arise from it, and the cause of the problem, though.

hadrien-f commented 1 year ago

Ok following your message I have successfully added a test for the issue.

To replicate the issue you can revert the first commit on my branch, the tests will then fail with the following error :

CypressError: Cypress test was stopped while running this command.

Because the browser throws a DOMException.

Please tell me if you need more information.

ericblade commented 1 year ago

ah, that's fantastic. I didn't get a chance to take a look at this this weekend, had some transportation issues crop up that had to be dealt with, but will try to get at it this evening! Thanks!