axxel / zxing-bench

Simple benchmarking for zxing related libraries.
2 stars 1 forks source link

rxing bug found from zxing-bench #1

Closed hschimke closed 7 months ago

hschimke commented 7 months ago

Looking at this I found an issue where not all the barcodes in your image were being reported. This turns out to be related to a bug which is currently open in the rxing repository: https://github.com/rxing-core/rxing/issues/48

Thanks!

axxel commented 7 months ago

As far as I understand, you fixed your mentioned bug in 0.5.5? But the number of detected barcodes in my sample image did not change: https://github.com/axxel/zxing-bench/commit/35ac29bc8710106eb1b3d393a07d558717c4a5f7

hschimke commented 7 months ago

Hmm, interesting. When I run the image myself (with TryHarder = true) I get all the symbols.

Found 9 results
Result 0:
(code 39) CODE39
Result 1:
(codabar) 012345
Result 2:
(code 128) CODE128
Result 3:
(itf) 00123456
Result 4:
(code 93) CODE93
Result 5:
(upc a) 012345678905
Result 6:
(ean 8) 01234565
Result 7:
(upc e) 01234565
Result 8:
(ean 13) 1234567890128

I have a suspicion of what's going on, but I'll have to investigate a bit.

Great close reason, btw.

axxel commented 7 months ago

Hmm, interesting. When I run the image myself (with TryHarder = true) I get all the symbols.

Oh, so there is indeed a 'bug' in my benchmarking code? Can you spot it?

Great close reason, btw.

Well, I thought the issues was in your code, so I did not quite know what to do with this issue reporting here other than "close" it. ;)

axxel commented 7 months ago

explicitly adding hints.insert(DecodeHintType::TRY_HARDER, DecodeHintValue::TryHarder(true)); does not change anything for me.

hschimke commented 7 months ago

I think that it's an issue with how rxing handles images. When it gets a full color image it has it's own desaturation algorithm that it runs:

for (x, y, new_pixel) in raster.enumerate_pixels_mut() {
            let pixel = img.get_pixel(x, y);
            let [red, green, blue, alpha] = pixel.0;
            if alpha == 0 {
                // white, so we know its luminance is 255
                *new_pixel = Luma([0xFF])
            } else {
                // .299R + 0.587G + 0.114B (YUV/YIQ for PAL and NTSC),
                // (306*R) >> 10 is approximately equal to R*0.299, and so on.
                // 0x200 >> 10 is 0.5, it implements rounding.
                *new_pixel = Luma([((306 * (red as u64)
                    + 601 * (green as u64)
                    + 117 * (blue as u64)
                    + 0x200)
                    >> 10) as u8])
            }
        }

For reasons that I need to figure out, it doesn't handle images converted to grey by the image crate very well. It's an open issue for me. If I had to guess, it's that there is an issue with my binarizer that doesn't handle the input very well.

axxel commented 7 months ago

So "not my problem" after all? ;)

hschimke commented 7 months ago

Oh yes, definitely a problem with how I was handling images.

I've changed it up a bit for my next release, but the path was [external conversion to Luma8] -> Convert to RGBa -> Custom conversion to Luma8 -> Binarizer. That's probably part of it.

hschimke commented 7 months ago

Oh wow, I found the bug, and it's been in the code for months.

I noticed that it didn't occur with any LuminanceSource besides the Luma8LuminanceSource. It turns out I had messed up the crop() function so that it wasn't correctly cropping images that were already cropped. I duplicated your benchmark code, and managed to recreate the issue as well. This seems to fully resolve it. A couple more fixes to roll up and this will be resolved.

axxel commented 7 months ago

I updated the README with the results from 0.5.7...