search

zxingify / zxingify-objc

An Objective-C Port of ZXing
Apache License 2.0
3.07k stars 753 forks source link

Luminance shades formula #446

Closed neacao closed 6 years ago

benjohnde commented 6 years ago

I just wrote a few suggestions. Very nice work @neacao !

neacao commented 6 years ago

@benjohnde Here is the problem that I faced: in manual tests (moving live camera), the shades formula was working well, but in test case, it's really hard (could not) to detect. So I have to use DecomposingMin (use min value of 3 color channels) to make grayscale of the image darker.

But in real application, we have to detect the Illumination value to determine which formula should to use or not. Can we ?

benjohnde commented 6 years ago

Hmmm that's tricky, we only have the RGB values. You could possibly use something like https://stackoverflow.com/a/46842115/5173688 to get the actual luminosity value from the camera.

But from what I understood in the beginning: you have images which are too bright and therefore the yellow gets too bright and not marked as black during binarization. So, if your image is too bright, your work should basically discover those pixels as black.

You basically implemented different approaches for grayscale conversion, which is totally fine. But you should find at least one image which can be read better with the new approach or all other images should work as well with the new implementation as with the old variant.

benjohnde commented 6 years ago

Sorry, right now I am very busy. I will try to look at your implementation in more detail tomorrow and also play around with some Starbucks QR codes.

neacao commented 6 years ago

Yes you'r right, new approaches do not fit all images, but some ad-hoc cases, they can decode it faster An example: qr5

What I'm doing is trying to make strategy to switch from original formula of library & new ones while scanning. This snippet code to determine if an image is darker https://gist.github.com/justinHowlett/4611988

Ps: I'll check the link you gave to see if it works

benjohnde commented 6 years ago

Well, that is in general a way to measure the overall amount of dark pixels. You could also use the overall mean value of the all intensities to get a feeling for the overall brightness of the image.

neacao commented 6 years ago

Yes, I was trying to use that function to measure brightness of the image, so I can switch from DIMP (original formula) & DecomposingMin for better detection.

benjohnde commented 6 years ago

@neacao I will review the code again and test it. Basically, if you need some additional logic to determine the brightness of your image, you could also do it in your own application.

In my opinion, to offer the user different grayscale methods for binarization is ok and does not alter any original zxing code. Even though you don't have a specific test case, which does only work with your optimizations, I am cool with merging it into master.

benjohnde commented 6 years ago

Ah damn @neacao...sorry I made a code review but did not submit it :/ Now you should see it 🎉

benjohnde commented 6 years ago

Tests are green, looks good to me! I am deep diving in your changes a bit. From your side fine to merge? I would change some code parts, mostly formatting stuff (cf. code review).

benjohnde commented 6 years ago

I am basically done with reformatting and review. LGTM, as mentioned above. I would rename BOOL:heuristic as the name is pretty generic. Either rename it or write a small comment. After your approval @neacao I would merge your work into master.

neacao commented 6 years ago

@benjohnde thanks for your working hard, please help to rename & make some comments if needed 😁