c++ bugfixes

57 views
Skip to first unread message

Sebastian Göbel

unread,
Aug 7, 2013, 10:19:44 AM8/7/13
to zx...@googlegroups.com
I've done some work with the c++ zxing version. Everything works fine for me, many thanks for the great lib!

Nevertheless I found some minor bugs, I hope this is the right place to report them.

1. decodedBitStreamParser.cpp, line 402
- huge memory leak

char* bytes = new char[count];
 
- possible fix:
ArrayRef<char> bytes(count);

2. Code39Reader.cpp, line 164
float right = (float) (nextStart + lastStart) / 2.0f;

The right result point is set incorrectly because nextStart is set to the next set (or line end) before leaving the while loop (line 124).

- possible fix:
use lastStart + lastPatternSize instead of nextStart:
float right = (float) (lastStart + lastPatternSize + lastStart) / 2.0f;

3. PDF417 detector, LinesSampler.cpp, line 413
- is not a bug but a huge performance increase if we leave the BARS_IN_SYMBOL loop when error >= bestMatchError

just add:
if (error >= bestMatchError)
  break;

Best regards,
Sebastian



Sean Owen

unread,
Aug 7, 2013, 10:25:20 AM8/7/13
to zx...@googlegroups.com
smparkes can comment on these. Have a look at the corresponding Java code too to see if there has already been a similar change.

Steven Parkes

unread,
Aug 7, 2013, 2:13:07 PM8/7/13
to Sebastian Göbel, zx...@googlegroups.com
Thanks.

I'll try to look at these this weekend (if my mbp gets back from Apple).

FWIW, did you check the non-C++-specific changes against the Java code? [Ah, srowen beat me to it :-)]
> --
>
> ---
> You received this message because you are subscribed to the Google Groups "zxing" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to zxing+un...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

Sebastian Göbel

unread,
Aug 13, 2013, 10:30:10 AM8/13/13
to zx...@googlegroups.com, Sebastian Göbel
> FWIW, did you check the non-C++-specific changes against the Java code? [Ah, srowen beat me to it :-)]

The first error is c++ specific.

The second error: "2. Code39Reader.cpp, line 164" should be the same problem in java (Code39Reader.java, line 168). Maybe it never comes out because you operate generally on small samples?

The performance problem (third error) should be the same issue in java:
PDF417CodewordDecoder.java -> line 101++):

for (int j = 0; j < RATIOS_TABLE.length; j++) {
      float error = 0.0f;
      for (int k = 0; k < PDF417Common.BARS_IN_MODULE; k++) {
        float diff = RATIOS_TABLE[j][k] - bitCountRatios[k];
        error += diff * diff;
      }
      if (error < bestMatchError) {
        bestMatchError = error;
        bestMatch = PDF417Common.SYMBOL_TABLE[j];
      }
    }

Is not necessary to continue the iteration through the PDF417Common.BARS_IN_MODULE loop if error >= bestMatchError.

Sean Owen

unread,
Aug 15, 2013, 1:35:02 PM8/15/13
to zx...@googlegroups.com
PS, I would make an issue on code.google.com/p/zxing to track this so that it doesn't get lost.

flyashi

unread,
Aug 16, 2013, 10:43:56 AM8/16/13
to zx...@googlegroups.com
Regarding the performance improvement in #3, Sebastian actually mentioned two performance improvements:
(1) switch from pow(diff, 2) to diff*diff (as Java does)
(2) break when error > bestMatchError

I modified the cli to accept a --try-rotated parameter and ran the following timing tests reading a Walgreens/Duane Reade loyalty card PDF417 (run in a VM):

Current implementation: 1295ms horizontal, 1390ms rotated
Just adding the break: 661ms horizontal, 755ms rotated

diff*diff, break: 509ms, 607ms rotated

On my 2011 MacBook Air, adding both those improvements dropped the time to scan a PDF417 from 2758ms to 953ms.

Sebastian, would you like to file an issue or should I?

Thanks!

 - flyashi

(--try-rotated is really only helpful for PDF417; the OneDReader will rotate if --try-harder is passed, and most 2D codes handle rotation on their own, but PDF417 doesn't.)

Reply all
Reply to author
Forward
0 new messages