Fixed error with multiple operations on the same HEIC file (#20)

* fixed multiple heic conversions * Fixed heic unit tests after dropping target param
thumbsup · Apr 8, 2021 · d702910 · d702910
1 parent a4596cf
commit d702910
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 17 deletions.
diff --git a/lib/image/heic.js b/lib/image/heic.js
@@ -1,6 +1,7 @@
 const async = require('async')
 const childProcess = require('child_process')
 const path = require('path')
+const tmp = require('tmp')
 const trace = require('debug')('thumbsup:trace')
 
 // IMPORTANT NOTE
@@ -30,11 +31,12 @@ const processed = {}
 
 // This function is typically called several times, for the thumbnail, small version, large version...
 // To avoid converting the image 3 times we re-use previously converted images
-exports.convert = function (source, target, callback) {
+exports.convert = function (source, callback) {
   if (!processed[source]) {
-    processed[source] = processFile(source, target)
+    const tmpfile = tmp.fileSync({ postfix: '.jpg' })
+    processed[source] = processFile(source, tmpfile.name)
   }
-  processed[source].then(() => callback(null)).catch(err => callback(err))
+  processed[source].then((target) => callback(null, target)).catch(err => callback(err))
 }
 
 // Return a promise so multiple callers can subscribe to when it's finished
@@ -43,7 +45,7 @@ function processFile (source, target) {
     done => convertToJpeg(source, target, done),
     done => copyColorProfile(source, target, done),
     done => convertToSRGB(target, done)
-  ])
+  ]).then(() => target)
 }
 
 function convertToJpeg (source, target, done) {

diff --git a/lib/index.js b/lib/index.js
@@ -8,7 +8,6 @@ const heic = require('./image/heic')
 const ffmpeg = require('./video/ffmpeg')
 const ffprobe = require('./video/ffprobe')
 const ffargs = require('./video/ffargs')
-const tmp = require('tmp')
 
 const GIF_FILE = /\.gif$/i
 const HEIC_FILE = /\.heic$/i
@@ -34,10 +33,9 @@ exports.image = function (source, target, options, callback) {
 
   // HEIC files must be converted to JPG first
   if (source.match(HEIC_FILE)) {
-    const tmpfile = tmp.fileSync({ postfix: '.jpg' })
-    return async.series([
-      done => heic.convert(source, tmpfile.name, done),
-      done => gmagick.prepare(tmpfile.name, options).write(target, done)
+    return async.waterfall([
+      done => heic.convert(source, done),
+      (jpgFile, done) => gmagick.prepare(jpgFile, options).write(target, done)
     ], callback)
   }
 

diff --git a/test/unit/heic.test.js b/test/unit/heic.test.js
@@ -11,7 +11,7 @@ afterEach(() => {
 describe('heic', () => {
   it('calls gmagick and exiftool', done => {
     sinon.stub(childProcess, 'execFile').callsFake(fakeExecFile)
-    heic.convert('input1.heic', 'output.jpg', err => {
+    heic.convert('input1.heic', err => {
       should(err).eql(null)
       should(childProcess.execFile.callCount).eql(3)
       should(childProcess.execFile.getCall(0).args[0]).eql('magick')
@@ -23,7 +23,7 @@ describe('heic', () => {
 
   it('stops at the first failing call', done => {
     sinon.stub(childProcess, 'execFile').callsFake(fakeExecFileFail)
-    heic.convert('input2.heic', 'output.jpg', err => {
+    heic.convert('input2.heic', err => {
       should(err.message).eql('FAIL')
       should(childProcess.execFile.callCount).eql(1)
       should(childProcess.execFile.getCall(0).args[0]).eql('magick')
@@ -34,8 +34,8 @@ describe('heic', () => {
   it('only processes each file once', done => {
     sinon.stub(childProcess, 'execFile').callsFake(fakeExecFile)
     async.parallel([
-      done => heic.convert('input3.heic', 'output.jpg', done),
-      done => heic.convert('input3.heic', 'output.jpg', done)
+      done => heic.convert('input3.heic', done),
+      done => heic.convert('input3.heic', done)
     ]).then(res => {
       should(childProcess.execFile.callCount).eql(3)
       done()
@@ -45,10 +45,10 @@ describe('heic', () => {
   it('keeps track of files already processed', done => {
     sinon.stub(childProcess, 'execFile').callsFake(fakeExecFile)
     async.parallel([
-      done => heic.convert('input4.heic', 'output.jpg', done),
-      done => heic.convert('input5.heic', 'output.jpg', done),
-      done => heic.convert('input6.heic', 'output.jpg', done),
-      done => heic.convert('input4.heic', 'output.jpg', done)
+      done => heic.convert('input4.heic', done),
+      done => heic.convert('input5.heic', done),
+      done => heic.convert('input6.heic', done),
+      done => heic.convert('input4.heic', done)
     ]).then(res => {
       should(childProcess.execFile.callCount).eql(3 * 3)
       done()