alexeyten / qr-image

Yet another QR code generator
MIT License
1.05k stars 168 forks source link

Incompatible PNG when running on Raspberry Pi (ARMv6) #13

Closed anders94 closed 9 years ago

anders94 commented 9 years ago

When running this on X86 Linux or X86 Mac:

var qr = require('qr-image');
var fs = require('fs');
var readable = qr.image('hi', {type: 'png', size: 1});
var writable = fs.createWriteStream('qr.png');
readable.pipe(writable);

I get good images.

However, when running on a Raspberry Pi (ARMv6) I get an invalid PNG file.

Here's a hex dump of the invalid PNG:

89504E47 0D0A1A0A 0000000D 49484452 0000001D 0000001D 08000000 00FFFFFF 
FE000000 80494441 5478DAC5 525B0EC0 3008E2FE 97665914 C5CDF46F 593F9A5A 
7C800A9E 0EBE4511 271E6D09 95E38DB5 5517E21F 89033BCA 03CA3D56 3EDCEA26 
4B28EFE0 DCF2F277 EA65798F B78A14A9 B826A1E0 A3C2B444 156A9908 3E515120 
CD9A1473 1A9B5EF5 D107E3BD C2166B95 0D7BCD08 2794A597 5C7763D5 8BDE8E67 
AF7ED8F6 0B896C64 B8000005 75000000 0049454E 44AE4260 82

Here's a hex dump of the good PNG:

89504E47 0D0A1A0A 0000000D 49484452 0000001D 0000001D 08000000 0073F838 
D3000000 80494441 5478DAC5 525B0EC0 3008E2FE 97665914 C5CDF46F 593F9A5A 
7C800A9E 0EBE4511 271E6D09 95E38DB5 5517E21F 89033BCA 03CA3D56 3EDCEA26 
4B28EFE0 DCF2F277 EA65798F B78A14A9 B826A1E0 A3C2B444 156A9908 3E515120 
CD9A1473 1A9B5EF5 D107E3BD C2166B95 0D7BCD08 2794A597 5C7763D5 8BDE8E67 
AF7ED8F6 0B896C64 B8974AAE 14000000 0049454E 44AE4260 82

The header looks good but it goes off the rails around the 00FFFFFF at the end of the top line.

Possibly some sort of big-endian / little-endian issue?

node version: v0.11.11-pre

alexeyten commented 9 years ago

It's something bad with CRC-32 calculation.

alexeyten commented 9 years ago

Could you please test this script https://github.com/alexeyten/qr-image/blob/master/test-crc.js

alexeyten commented 9 years ago

also node lib/crc32 should print prefilled table.

anders94 commented 9 years ago
# ~/x/qr-image $ cat test-crc.js
var crc32 = require('./lib/crc32');
var b = Buffer('TEST STRING\n');
var c = crc32(b);
console.log(c.toString(16) == '6358ff17');
# ~/x/qr-image $ node test-crc
false
# ~/x/qr-image $ node lib/crc32.js
[ 0,
  31158422,
  812,
  698,
  25,
  399,
  821,
  675,
  50,
  420,
  798,
  648,
  43,
  445,
  775,
  657,
  100,
  498,
  840,
  734,
  125,
  491,
  849,
  711,
  86,
  448,
  890,
  748,
  79,
  473,
  867,
  757,
  200,
  350,
  996,
  626,
  209,
  327,
  1021,
  619,
  250,
  364,
  982,
  576,
  227,
  373,
  975,
  601,
  172,
  314,
  896,
  534,
  181,
  291,
  921,
  527,
  158,
  264,
  946,
  548,
  135,
  273,
  939,
  573,
  400,
  6,
  700,
  810,
  393,
  31,
  677,
  819,
  418,
  52,
  654,
  792,
  443,
  45,
  663,
  769,
  500,
  98,
  728,
  846,
  493,
  123,
  705,
  855,
  454,
  80,
  746,
  892,
  479,
  73,
  755,
  869,
  344,
  206,
  628,
  994,
  321,
  215,
  621,
  1019,
  362,
  252,
  582,
  976,
  371,
  229,
  607,
  969,
  316,
  170,
  528,
  902,
  293,
  179,
  521,
  927,
  270,
  152,
  546,
  948,
  279,
  129,
  571,
  941,
  800,
  694,
  12,
  410,
  825,
  687,
  21,
  387,
  786,
  644,
  62,
  424,
  779,
  669,
  39,
  433,
  836,
  722,
  104,
  510,
  861,
  715,
  113,
  487,
  886,
  736,
  90,
  460,
  879,
  761,
  67,
  469,
  1000,
  638,
  196,
  338,
  1009,
  615,
  221,
  331,
  986,
  588,
  246,
  352,
  963,
  597,
  239,
  377,
  908,
  538,
  160,
  310,
  917,
  515,
  185,
  303,
  958,
  552,
  146,
  260,
  935,
  561,
  139,
  285,
  688,
  806,
  412,
  10,
  681,
  831,
  389,
  19,
  642,
  788,
  430,
  56,
  667,
  781,
  439,
  33,
  724,
  834,
  504,
  110,
  717,
  859,
  481,
  119,
  742,
  880,
  458,
  92,
  767,
  873,
  467,
  69,
  632,
  1006,
  340,
  194,
  609,
  1015,
  333,
  219,
  586,
  988,
  358,
  240,
  595,
  965,
  383,
  233,
  540,
  906,
  304,
  166,
  517,
  915,
  297,
  191,
  558,
  952,
  258,
  148,
  567,
  929,
  283,
  141 ]
# ~/x/qr-image
anders94 commented 9 years ago

I think it might have to do with overflow on the ^ bitwise operator - have been working on a good test for it. I'll post when I get something.

anders94 commented 9 years ago

It is at least a problem with overflow in the bitwise XOR operations (^). Building up a longer and longer bit pattern like this:

var s1 = '';
var s2 = '';

for (var n=0; n<32; n++) {
    if (n%2==0)
    s1+='1';
    else
    s1+='0';
    s2+='1';

    console.log(n, s1);
    console.log(n, s2);

    var n1 = parseInt(s1, 2);
    var n2 = parseInt(s2, 2);

    console.log(n, (n1^n2).toString(2));
    console.log();
}

and running it on X86 ends in:

...
30 '1010101010101010101010101010101'
30 '1111111111111111111111111111111'
30 '101010101010101010101010101010'

31 '10101010101010101010101010101010'
31 '11111111111111111111111111111111'
31 '1010101010101010101010101010101'

while running it on ARM yields:

...
30 '1010101010101010101010101010101'
30 '1111111111111111111111111111111'
30 '101010101010101010101010101010'

31 '10101010101010101010101010101010'
31 '11111111111111111111111111111111'
31 '10101010101'
alexeyten commented 9 years ago

Probably you should file bug to node.js

I'll think of some kind workaround

alexeyten commented 9 years ago

How does other bit operators (<<, >>, >>>, |) work there? E.g. -1 >>> 1 or, 2863311530 | 0

anders94 commented 9 years ago

I'll try that.

If you don't depend on overflowing behavior, I was thinking about trying .xor(n) from https://github.com/substack/node-bigint

alexeyten commented 9 years ago

It uses libgmp. I try not to depend on any modules, especially binary ones.

I guess, it should be not hard to write workaround using buffers or typed arrays.

anders94 commented 9 years ago

Buffers sounds like a good idea. I can test code for you if you can work on it. If not, I can work on it and build a PR for you but it will take some time - I have to figure out how CRC32 is supposed to work!

alexeyten commented 9 years ago

Could you test branch issue-13-crc32

anders94 commented 9 years ago

test-crc.js returns true now. (!)

Testing PNG file creation now...

anders94 commented 9 years ago

Confirmed. PNG creation now works on ARM! Thanks for the quick turn-around.

alexeyten commented 9 years ago

Well, now I'd like some simple way to check if platform has buggy xor.

Could you provide me some simple expression like (-1 ^ 1) === -2 that fails on ARMv6 and work in x86.

anders94 commented 9 years ago

This is proving to be very difficult. It seems to only happen when after the first iteration inside a loop. I think it might have something to do with optimizations - particularly variable reuse. Still working...

This is clearly either a V8 bug or a node.js bug because they are relying on an outdated V8 engine.

anders94 commented 9 years ago

V8 issue logged:

https://code.google.com/p/v8/issues/detail?id=3757

alexeyten commented 9 years ago

Ok. Then how to know if it's armv6? Probably some method from os module?

anders94 commented 9 years ago

This will return true if it is ARMv6:

var os = require('os');
console.log(os.cpus()[0].model.indexOf('ARMv6') == 0);
alexeyten commented 9 years ago

OK. I'll look into it on Monday. On 12 Dec 2014 23:48, "Anders Brownworth" notifications@github.com wrote:

This will return true if it is ARMv6:

var os = require('os'); console.log(os.cpus()[0].model.indexOf('ARMv6') == 0);

— Reply to this email directly or view it on GitHub https://github.com/alexeyten/qr-image/issues/13#issuecomment-66832882.

anders94 commented 9 years ago

Thanks for all the help.

alexeyten commented 9 years ago

Could you tell, what os.arch() returns?

anders94 commented 9 years ago
[ { model: 'ARMv6-compatible processor rev 7 (v6l)',
    speed: 700,
    times:
     { user: 17490400,
       nice: 3260100,
       sys: 13177900,
       idle: 1526694304,
       irq: 54000 } } ]
alexeyten commented 9 years ago

It's os.cpus() I guess?

Should be something

> os.arch()
'ia32'
anders94 commented 9 years ago

Sorry, correct - that was os.cpus(). Here's os.arch():

arm
alexeyten commented 9 years ago

As I learn from https://code.google.com/p/v8/issues/detail?id=3757 bug is in old v8 engine?

alexeyten commented 9 years ago

Can you confirm that latest https://github.com/alexeyten/qr-image/commits/issue-13-crc32 works on both arm and x86? I'll merge it then.

anders94 commented 9 years ago

Confirmed - this works now. Thanks for the quick fix.

alexeyten commented 9 years ago

https://github.com/alexeyten/qr-image/releases/tag/v3.0

bosson commented 6 years ago

Hi, this patch assumes node with global "process" and breaks web use. The lib/crc32.js fix should test for process... pollyfilling in a process global causes confusion on a different level thats harder to manage.

if (process.arch === 'arm') { module.exports = require('./crc32buffer'); return; }

like

if (process && process.arch === 'arm') { ...

alexeyten commented 6 years ago

I'm going to get rid of this patch in new version. It was 4 years and several major versions of node ago, and I hope there is no need for it now.