Skip to content

jpegstream is broken since 1.2.6 #674

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

Closed
mnill opened this issue Nov 20, 2015 · 9 comments
Closed

jpegstream is broken since 1.2.6 #674

mnill opened this issue Nov 20, 2015 · 9 comments

Comments

@mnill
Copy link

mnill commented Nov 20, 2015

After 1.2.6 jpeg stream makes for me really big files with some trash. Tested on mac and linux.

@mnill
Copy link
Author

mnill commented Nov 20, 2015

Really big - 10 mb for file 300kb

@zbjornson
Copy link
Collaborator

Can you provide your test code please?

@mnill
Copy link
Author

mnill commented Nov 22, 2015

@zbjornson

Sure:

var canvas = require('canvas');

var c = new canvas(2048, 1536);

c.context = c.getContext('2d');
for (i=0; i<2048*1536; i++) {
    c.context.fillStyle = "rgb("+ i%255 +","+i%255+","+i%255+")";
    c.context.fillRect(i%2048, parseInt(i/2048), 1, 1);
}

var stream = c.jpegStream({
    bufsize: 1024*1024*10 // output buffer size in bytes, default: 4096
    , quality: 75 // JPEG quality (0-100) default: 75
    , progressive: false // true for progressive compression, default: false
});
var bufs = [];
stream.on('data', function(d){ bufs.push(d); });
stream.on('end', function() {
    var buf = Buffer.concat(bufs);
    require('fs').writeFileSync(require('path').join(__dirname, '/test.jpg'), buf);
    console.log('all done', buf.length);
});

With latest version i have: 'all done 10485760'
With 1.2.9 i have 'all done 571936'
I found it has been broken in 1.2.10
On 1.2.9 it is worked correctly.

@targos
Copy link
Contributor

targos commented Nov 22, 2015

The bufsize option seems important here. If I remove it I get all done 573440

@mnill
Copy link
Author

mnill commented Nov 22, 2015

Hm, really missed that 10485760 = 1024 * 1024 * 10. Maybe it is not issue. Thanks.

@mnill mnill closed this as completed Nov 22, 2015
@zbjornson
Copy link
Collaborator

zbjornson commented Nov 22, 2015 via email

@LinusU
Copy link
Collaborator

LinusU commented Nov 22, 2015

This still feels like a bug to me

@LinusU LinusU reopened this Nov 22, 2015
@LinusU
Copy link
Collaborator

LinusU commented Nov 22, 2015

Another thing that I discovered just now is that the stream isn't a spec-compliant Readable stream. Namely there isn't a function .read([n]).

It also emits data events before anything has started to listen with .on('data', _)...

@LinusU
Copy link
Collaborator

LinusU commented Nov 22, 2015

Okay, here is some very short code demonstrating the issue:

const assert = require('assert')
const Canvas = require('canvas')
const SIZE = 10 * 1024 * 1024

let c = new Canvas(10, 10)
let s = c.jpegStream({ bufsize: SIZE })

s.on('data', function (chunk) {
  assert(chunk.length < SIZE, `Expected chunk size (${chunk.length}) to be smaller than maximum (${SIZE})`)
})

@LinusU LinusU closed this as completed in 6edfe44 May 5, 2016
LinusU added a commit that referenced this issue May 5, 2016
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

4 participants