New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add multi-eci decoding for PDF417 #1507
Add multi-eci decoding for PDF417 #1507
Conversation
- Fixed issue that some multi-eci encoded PDF417 codes were missing ECIs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments.
Seems OK if this strictly increases the number of symbols that can be decoded
core/src/main/java/com/google/zxing/pdf417/decoder/DecodedBitStreamParser.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/google/zxing/pdf417/decoder/DecodedBitStreamParser.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/google/zxing/pdf417/decoder/DecodedBitStreamParser.java
Show resolved
Hide resolved
Great. Yes, it does just that and it also fixes a bug in the encoding. Regarding the risks of this PR, I have the following thoughts:
|
Please don't merge it yet. The spec says that ECIs may occur in the middle of 901 data (less than 6 bytes) and at the boundaries of 924 blocks (every 5 codewords) without requiring a new latch. The encoder never creates that but I am making sure that the decoder can handle it. |
…ocations in binary encoded data as specified in section 5.5.3.2 of the spec - Added verifying unit test
I condensed the code of the function DecodedBitStreamParser.byteCompaction() so that it is about half as long as before. |
Also fixed issue that some multi-eci encoded PDF417 codes were missing ECIs
The unit test now also decodes the multi-eci encoding tests and verifies that the result is identical with the input.