jpegStream: bugfix, tests, simplification with LinusU. Fix #629

Remove the undocumented 3rd argument of the emitters (bytes left in buffer).
Add a column to the browser tests that displays JPEGs.
Revise how buffers are allocated.

lib/jpegstream.js

@@ -40,12 +40,12 @@ var JPEGStream = module.exports = function JPEGStream(canvas, options, sync) {
   // TODO: implement async
   if ('streamJPEG' == method) method = 'streamJPEGSync';
   process.nextTick(function(){
-    canvas[method](options.bufsize, options.quality, options.progressive, function(err, chunk, len){
+    canvas[method](options.bufsize, options.quality, options.progressive, function(err, chunk){
       if (err) {
         self.emit('error', err);
         self.readable = false;
-      } else if (len) {
-        self.emit('data', chunk, len);
+      } else if (chunk) {
+        self.emit('data', chunk);
       } else {
         self.emit('end');
         self.readable = false;

src/JPEGStream.h

@@ -31,13 +31,17 @@ boolean
 empty_closure_output_buffer(j_compress_ptr cinfo){
   Nan::HandleScope scope;
   closure_destination_mgr *dest = (closure_destination_mgr *) cinfo->dest;
+
   Local<Object> buf = Nan::NewBuffer((char *)dest->buffer, dest->bufsize).ToLocalChecked();
-  Local<Value> argv[3] = {
+
+  // emit "data"
+  Local<Value> argv[2] = {
       Nan::Null()
     , buf
-    , Nan::New<Integer>(dest->bufsize)
   };
-  Nan::MakeCallback(Nan::GetCurrentContext()->Global(), (v8::Local<v8::Function>)dest->closure->fn, 3, argv);
+  Nan::MakeCallback(Nan::GetCurrentContext()->Global(), (v8::Local<v8::Function>)dest->closure->fn, 2, argv);
+
+  dest->buffer = (JOCTET *)malloc(dest->bufsize);
   cinfo->dest->next_output_byte = dest->buffer;
   cinfo->dest->free_in_buffer = dest->bufsize;
   return true;
@@ -47,26 +51,24 @@ void
 term_closure_destination(j_compress_ptr cinfo){
   Nan::HandleScope scope;
   closure_destination_mgr *dest = (closure_destination_mgr *) cinfo->dest;
+
   /* emit remaining data */
-  size_t remaining = dest->bufsize - cinfo->dest->free_in_buffer;
-  Local<Object> buf = Nan::NewBuffer((char *)dest->buffer, remaining).ToLocalChecked();
+  Local<Object> buf = Nan::NewBuffer((char *)dest->buffer, dest->bufsize).ToLocalChecked();
 
-  Local<Value> data_argv[3] = {
+  Local<Value> data_argv[2] = {
       Nan::Null()
     , buf
-    , Nan::New<Number>(remaining)
   };
 
-  Nan::MakeCallback(Nan::GetCurrentContext()->Global(), (v8::Local<v8::Function>)dest->closure->fn, 3, data_argv);
+  Nan::MakeCallback(Nan::GetCurrentContext()->Global(), (v8::Local<v8::Function>)dest->closure->fn, 2, data_argv);
 
   // emit "end"
-  Local<Value> end_argv[3] = {
+  Local<Value> end_argv[2] = {
       Nan::Null()
     , Nan::Null()
-    , Nan::New<Integer>(0)
   };
 
-  Nan::MakeCallback(Nan::GetCurrentContext()->Global(), (v8::Local<v8::Function>)dest->closure->fn, 3, end_argv);
+  Nan::MakeCallback(Nan::GetCurrentContext()->Global(), (v8::Local<v8::Function>)dest->closure->fn, 2, end_argv);
 }
 
 void
@@ -96,16 +98,6 @@ jpeg_closure_dest(j_compress_ptr cinfo, closure_t * closure, int bufsize){
   cinfo->dest->free_in_buffer = dest->bufsize;
 }
 
-void
-jpeg_free_custom_allocations(j_compress_ptr cinfo){
-  closure_destination_mgr * dest;
-  dest = (closure_destination_mgr *) cinfo->dest;
-  if (dest->buffer) {
-    free(dest->buffer);
-    dest->buffer = NULL;
-  }
-}
-
 void
 write_to_jpeg_stream(cairo_surface_t *surface, int bufsize, int quality, bool progressive, closure_t *closure){
   int w = cairo_image_surface_get_width(surface);
@@ -148,7 +140,6 @@ write_to_jpeg_stream(cairo_surface_t *surface, int bufsize, int quality, bool pr
   }
   free(dst);
   jpeg_finish_compress(&cinfo);
-  jpeg_free_custom_allocations(&cinfo);
   jpeg_destroy_compress(&cinfo);
 }
 

test/canvas.test.js

@@ -657,5 +657,28 @@ module.exports = {
     stream.on('error', function(err) {
       done(err);
     });
+  },
+
+  'test Canvas#jpegStream()': function(done) {
+    var canvas = new Canvas(640, 480);
+    var stream = canvas.jpegStream();
+    var firstChunk = true;
+    var bytes = 0;
+    stream.on('data', function(chunk){
+      if (firstChunk) {
+        firstChunk = false;
+        assert.equal(0xFF, chunk[0]);
+        assert.equal(0xD8, chunk[1]);
+        assert.equal(0xFF, chunk[2]);
+      }
+      bytes += chunk.length;
+    });
+    stream.on('end', function(){
+      assert.equal(bytes, 8192);
+      done();
+    });
+    stream.on('error', function(err) {
+      done(err);
+    });
   }
 }

test/public/app.js

@@ -65,27 +65,28 @@ function runTests() {
     var fn = tests[name]
       , canvas = create('canvas')
       , tr = create('tr')
-      , tds = [create('td'), create('td'), create('td')];
+      , tds = [create('td'), create('td'), create('td'), create('td')];
 
     canvas.width = 200;
     canvas.height = 200;
     canvas.title = name;
 
-    tds[1].appendChild(canvas);
-    tds[2].appendChild(create('h3', name));
-    tds[2].appendChild(pdfForm(fn, canvas));
+    tds[2].appendChild(canvas);
+    tds[3].appendChild(create('h3', name));
+    tds[3].appendChild(pdfForm(fn, canvas));
 
     tr.appendChild(tds[0]);
     tr.appendChild(tds[1]);
     tr.appendChild(tds[2]);
+    tr.appendChild(tds[3]);
 
     tbody.appendChild(tr);
     table.appendChild(tbody);
-    runTest(name, canvas, tds[0], tds[2]);
+    runTest(name, canvas, tds[0], tds[1], tds[3]);
   }
 }
 
-function runTest(name, canvas, dest, stats) {
+function runTest(name, canvas, dest, jpegDest, stats) {
   var fn = tests[name]
     , start = new Date;
   try {
@@ -96,7 +97,7 @@ function runTest(name, canvas, dest, stats) {
   var duration = new Date - start;
   stats.appendChild(create('p', 'browser: ' + duration + 'ms'));
   stats.appendChild(create('p', 'fps: ' + (1000 / duration).toFixed(0)));
-  renderOnServer(name, canvas, function(res){
+  renderOnServer('/render', name, canvas, function(res){
     if (res.error) {
       var p = create('p');
       p.innerText = res.error;
@@ -109,16 +110,27 @@ function runTest(name, canvas, dest, stats) {
       dest.appendChild(img);
     }
   });
+  renderOnServer('/jpeg', name, canvas, function(res){
+    if (res.error) {
+      var p = create('p');
+      p.innerText = res.error;
+      jpegDest.appendChild(p);
+    } else if (res.data) {
+      var img = create('img');
+      img.src = res.data;
+      jpegDest.appendChild(img);
+    }
+  });
 }
 
-function renderOnServer(name, canvas, fn) {
+function renderOnServer(url, name, canvas, fn) {
   var req = new XMLHttpRequest
     , json = JSON.stringify({
       fn: tests[name].toString()
       , width: canvas.width
       , height: canvas.height
     });
-  req.open('POST', '/render');
+  req.open('POST', url);
   req.setRequestHeader('Content-Type', 'application/json');
   req.onreadystatechange = function(){
     if (4 == req.readyState) {

test/server.js

@@ -78,6 +78,26 @@ app.post('/pdf', function(req, res, next){
     : fn(ctx), done();
 });
 
+app.post('/jpeg', function(req, res, next){
+  var fn = testFn(req)
+    , canvas = createCanvas(req)
+    , ctx = canvas.getContext('2d');
+
+  function done(){
+    var stream = canvas.jpegStream();
+    var buffers = [];
+    stream.on('data', function (chunk) {
+      buffers.push(chunk);
+    });
+    stream.on('end', function() {
+      res.send({data: 'data:image/jpeg;base64,' + Buffer.concat(buffers).toString('base64')});
+    });
+  }
+
+  2 == fn.length
+    ? fn(ctx, done)
+    : fn(ctx), done();
+});
 
 var port = parseInt(process.argv[2] || '4000', 10);
 app.listen(port);

test/views/tests.jade

@@ -18,6 +18,7 @@ block content
     thead
       tr
         th node-canvas
+        th node-canvas (JPEG)
         th target
         th
     tbody