Skip to content
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

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

Closed
anders94 opened this issue Dec 9, 2014 · 31 comments
Closed

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

anders94 opened this issue Dec 9, 2014 · 31 comments
Assignees

Comments

@anders94
Copy link

anders94 commented Dec 9, 2014

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 alexeyten self-assigned this Dec 9, 2014
@alexeyten
Copy link
Owner

It's something bad with CRC-32 calculation.

alexeyten added a commit that referenced this issue Dec 10, 2014
@alexeyten
Copy link
Owner

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

@alexeyten
Copy link
Owner

also node lib/crc32 should print prefilled table.

@anders94
Copy link
Author

# ~/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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

Probably you should file bug to node.js

I'll think of some kind workaround

@alexeyten
Copy link
Owner

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

@anders94
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

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 added a commit that referenced this issue Dec 12, 2014
Probable fix for issue #13
@alexeyten
Copy link
Owner

Could you test branch issue-13-crc32

alexeyten added a commit that referenced this issue Dec 12, 2014
Probable fix for issue #13
@anders94
Copy link
Author

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

Testing PNG file creation now...

@anders94
Copy link
Author

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

@alexeyten
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

@alexeyten
Copy link
Owner

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

@anders94
Copy link
Author

This will return true if it is ARMv6:

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

@alexeyten
Copy link
Owner

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
#13 (comment).

@anders94
Copy link
Author

Thanks for all the help.

@alexeyten
Copy link
Owner

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

@anders94
Copy link
Author

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

@alexeyten
Copy link
Owner

It's os.cpus() I guess?

Should be something

> os.arch()
'ia32'

@anders94
Copy link
Author

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

arm

@alexeyten
Copy link
Owner

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

alexeyten added a commit that referenced this issue Dec 15, 2014
Probable fix for issue #13
@alexeyten
Copy link
Owner

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
Copy link
Author

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

@alexeyten
Copy link
Owner

@bosson
Copy link

bosson commented Sep 7, 2018

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
Copy link
Owner

alexeyten commented Sep 14, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants