detect_in_luma supports only square input about rxing HOT 14 CLOSED

I talked with them, and they're ok with it, the data is now part of the integration test set.

from rxing.

Excellent! Thank you. I have an idea of what causes this. I should have a fix tomorrow.

Also, don't worry about being annoying, I'm very excited that someone is using this and finding bugs that I likely would never have found!

from rxing.

It should support rectangular images, that's a bug for sure.

The luma8 format is likely the best choice in most situations (it is what the WASM wrapper uses so it is the best tested).

I'll look into this and get a fix.

from rxing.

I'm unfamiliar with the opencv formats, the rgba base format avoids the transpose issue as it uses the image crate to handle rotation. However, if the opencv formats are a common input I could probably add a trait implantation that more natively handles them.

Until then, I'll have a fix out tomorrow for this on the 0.3 and 0.4 versions.

from rxing.

@chemicstry I've pushed an update to both the v3 and v4 versions, this is now fixed in v0.3.3 and v0.4.1.

Let me know if this resolves your issue, I'm happy to take a look at adding opencv native support in the future, it would likely need to live behind a feature flag, but that doesn't seem like a huge issue.

from rxing.

Thanks for the fix. I think it resolved the transpose issue, but another issue popped up:

panicked at 'index out of bounds: the len is 0 but the index is 0', /home/chemicstry/.cargo/registry/src/github.com-1ecc6299db9ec823/rxing-0.4.1/src/common/global_histogram_binarizer.rs:75:31

I'm processing live images from camera and this seems to only panic in ~1/6 frames so it depends on data. Attached a file containing data that triggers this issue with the following usage:

rxing::helpers::detect_multiple_in_luma(panic_data::DATA.to_vec(), 665, 286)

I wasn't sure what's the best way to send over a vec, so just copy pasted debug print into a panic_data.rs file as a static &[u8].

panic_data.zip

EDIT: regarding opencv support, I'm not sure if this crate should support conversions for every data format, because there are a lot of them (image, opencv, gstreamer, nalgebra, etc). Adding support for everything will be a maintenance nightmare to update with each version. Ability to supply luma8 &[u8] is enough IMO, any image processing library can convert into that format.

from rxing.

I'll look into this and see what I can find. I don't have any initial hunches given where it's happening.

Do you mind if I use the data you provided in a permanent test case to prevent regression?

from rxing.

@chemicstry Found it! It was a new issue introduced by my previous fix, I hadn't swapped the cropped coordinate adjustments after a transpose for rectangular images. It makes sense that it didn't fail all the time, as the crop was likely not always occurring on every frame. I'll have a fix out this afternoon.

from rxing.

v0.4.2 should resolve this issue.
edit: and v0.3.4

from rxing.

Thanks again for the quick fix. I will test it tomorrow.

Do you mind if I use the data you provided in a permanent test case to prevent regression?

The image is taken by printing this so I don't know about their copyrights, but otherwise I have no issues with it.

from rxing.

The previous issue is fixed, but I'm sorry to tell you that I found another one 😅 This one was a bit harder to hit and took hundreds of frames to trigger.

This time I saved it as binary file so it is smaller and doesn't crash the rust-analyzer. Test code:

    let mut data = Vec::new();
    std::fs::File::open("panic_data2.bin")
        .unwrap()
        .read_to_end(&mut data)
        .unwrap();

    rxing::helpers::detect_multiple_in_luma(data, 720, 618).unwrap();

Results in: panicked at 'index out of bounds: the len is 5175 but the index is 5175', /home/chemicstry/.cargo/git/checkouts/rxing-cd9b60f7160b7efd/33ea52c/src/common/bit_matrix.rs:213:11

panic_data2.zip

Maybe fuzzing tests would help, but I'm not sure if random data could trigger these edge cases that likely need to pass detection stage first.

Thanks for working on this, I hope I'm not getting too annoying xD

from rxing.

v0.4.3 (which includes this fix and some others) should resolve the issue.

from rxing.

In general I think fuzzing would help, it would at the very least catch issue like the above. I have only limited experience with fuzzing, so I'll have to learn how to build useful cases.

from rxing.

Seems to work fine now! I will open a new issue if I encounter any more panics.

Thanks

from rxing.