Skip to content

Support horizontally flipped QR-codes according to ISO 18004:2015#89

Merged
kaworu merged 4 commits intodlbeer:masterfrom
claudiofelber:master
Sep 23, 2020
Merged

Support horizontally flipped QR-codes according to ISO 18004:2015#89
kaworu merged 4 commits intodlbeer:masterfrom
claudiofelber:master

Conversation

@claudiofelber
Copy link
Copy Markdown
Contributor

The QR-code specification supports various optional features. A newer one is "mirror imaging". I don't know in which version of the specification (2006, 2009 or 2015) it was added, but it certainly is mentioned in ISO 18004:2015:

ISO 18004: 2015, page 6, "Mirror lmaging":

The arrangement of modules defined in this International Standard represents the "normal"
orientation of the symbol. lt ls, however, possible to achieve a valid decode of a symbol in which the
arrangement of the modules has been laterally transposed. When viewed with the finder patterns
at the top left, top right and bottom left corners of the symbol, the effect of mirror imaging is to
interchange the row and column positions of the modules.

ISO 18004: 2015, page 62, "Decoding procedure overview":


2. Read the format Information. Release the masking pattem and perform error correction on the
format information modules as necessary; if successful, symbol is in normal orientation, otherwise
attempt mirror image decoding of format Information. ldentify Error Correction Level, either
directly, in QR Code symbols, or from Micro QR Code symbol number, and data mask pattern
reference.

For the moment I decided on making the flipped detection optional by providing a new function quirc_flip(). It flips the modules pixmap and rotates it back to the normalized position. Then a second decode attempt can be initiated. A typical call sequence would look like this:

struct quirc_data data;

quirc_decode_error_t err = quirc_decode(&code, &data);
if (err == QUIRC_ERROR_DATA_ECC) {
    quirc_flip(&code);
    err = quirc_decode(&code, &data);
}

if (err) {
    // handle error
} else {
    // use decoded payload
}

The following two QR-codes represent a normal and a horizontally flipped one. Both can be correctly decoded with this small extension:

qrcodes

This resolves the problem I reported in issue #88.

@kaworu kaworu added the feature feature request label Sep 21, 2020
Copy link
Copy Markdown
Collaborator

@kaworu kaworu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great design, thanks a lot for the PR!

Couple of nitpick but feel free to ignore them.

The changes I would like to see is more light on this feature. First, I think both qrtest and inspect (from the test/ directory) should "do the flip dance". Then, it definitely deserve a spot in the README.

lib/decode.c Outdated
Comment on lines +922 to +923
struct quirc_code flipped;
memset(&flipped, 0, sizeof(flipped));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We mandate c99 support, so you could write struct quirc_code flipped = {0}; instead. In practice, I believe most compilers will generate the same output for both cases, so it's only a matter of readability.

lib/decode.c Outdated
int offset = 0;
for (int y = 0, sx = 0; y < code->size; y++, sx++) {
for (int x = 0, sy = 0; x < code->size; x++, sy++) {
if (grid_bit(code, sx, sy)) {
Copy link
Copy Markdown
Collaborator

@kaworu kaworu Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: My understanding here is that y == sx and x == sy always hold, and thus we could get rid of both sx and sy.

I guess the intent is to avoid grid_bit(code, y, x) because x and y would be in the inverse order of the definition:

static inline int grid_bit(const struct quirc_code *code, int x, int y)

Personally, in the context of "flipping", I wouldn't mind this inversion. It would actually explicit the flipping if anything.

@claudiofelber
Copy link
Copy Markdown
Contributor Author

Thanks for your feedback. I have added support for flipped QR-codes to the inspect and qrtest tools, optimized quirc_flip() according to your suggestions and described its usage in the README file. You were absolutely right with sx and sy not being necessary. They were a relic of some earlier experiments and I forgot to clean up the code prior to committing it – shame on me!

@kaworu kaworu merged commit 7e7ab59 into dlbeer:master Sep 23, 2020
@kaworu
Copy link
Copy Markdown
Collaborator

kaworu commented Sep 23, 2020

LGTM, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants