Improve extractChannel support for 16-bit output #3453

lovell · Nov 15, 2022 · 01ffa80 · 01ffa80
1 parent 789d485
commit 01ffa80
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 39 deletions.
diff --git a/docs/changelog.md b/docs/changelog.md
@@ -12,6 +12,9 @@ Requires libvips v8.13.3
 * Prevent possible race condition awaiting metadata of Stream-based input.
   [#3451](https://github.com/lovell/sharp/issues/3451)
 
+* Improve `extractChannel` support for 16-bit output colourspaces.
+  [#3453](https://github.com/lovell/sharp/issues/3453)
+
 ### v0.31.2 - 4th November 2022
 
 * Upgrade to libvips v8.13.3 for upstream bug fixes.

diff --git a/lib/channel.js b/lib/channel.js
@@ -98,7 +98,7 @@ function extractChannel (channel) {
   } else {
     throw is.invalidParameterError('channel', 'integer or one of: red, green, blue, alpha', channel);
   }
-  return this.toColourspace('b-w');
+  return this;
 }
 
 /**

diff --git a/src/pipeline.cc b/src/pipeline.cc
@@ -701,24 +701,6 @@ class PipelineWorker : public Napi::AsyncWorker {
         image = sharp::Tint(image, baton->tintA, baton->tintB);
       }
 
-      // Extract an image channel (aka vips band)
-      if (baton->extractChannel > -1) {
-        if (baton->extractChannel >= image.bands()) {
-          if (baton->extractChannel == 3 && sharp::HasAlpha(image)) {
-            baton->extractChannel = image.bands() - 1;
-          } else {
-            (baton->err).append("Cannot extract channel from image. Too few channels in image.");
-            return Error();
-          }
-        }
-        VipsInterpretation const interpretation = sharp::Is16Bit(image.interpretation())
-          ? VIPS_INTERPRETATION_GREY16
-          : VIPS_INTERPRETATION_B_W;
-        image = image
-          .extract_band(baton->extractChannel)
-          .copy(VImage::option()->set("interpretation", interpretation));
-      }
-
       // Remove alpha channel, if any
       if (baton->removeAlpha) {
         image = sharp::RemoveAlpha(image);
@@ -744,6 +726,26 @@ class PipelineWorker : public Napi::AsyncWorker {
         }
       }
 
+      // Extract channel
+      if (baton->extractChannel > -1) {
+        if (baton->extractChannel >= image.bands()) {
+          if (baton->extractChannel == 3 && sharp::HasAlpha(image)) {
+            baton->extractChannel = image.bands() - 1;
+          } else {
+            (baton->err)
+              .append("Cannot extract channel ").append(std::to_string(baton->extractChannel))
+              .append(" from image with channels 0-").append(std::to_string(image.bands() - 1));
+            return Error();
+          }
+        }
+        VipsInterpretation colourspace = sharp::Is16Bit(image.interpretation())
+          ? VIPS_INTERPRETATION_GREY16
+          : VIPS_INTERPRETATION_B_W;
+        image = image
+          .extract_band(baton->extractChannel)
+          .copy(VImage::option()->set("interpretation", colourspace));
+      }
+
       // Apply output ICC profile
       if (!baton->withMetadataIcc.empty()) {
         image = image.icc_transform(

diff --git a/test/fixtures/expected/extract-lch.jpg b/test/fixtures/expected/extract-lch.jpg
diff --git a/test/unit/extractChannel.js b/test/unit/extractChannel.js
@@ -54,19 +54,13 @@ describe('Image channel extraction', function () {
       });
   });
 
-  it('With colorspace conversion', function (done) {
-    const output = fixtures.path('output.extract-lch.jpg');
-    sharp(fixtures.inputJpg)
-      .extractChannel(1)
+  it('With colorspace conversion', async () => {
+    const [chroma] = await sharp({ create: { width: 1, height: 1, channels: 3, background: 'red' } })
       .toColourspace('lch')
-      .resize(320, 240, { fastShrinkOnLoad: false })
-      .toFile(output, function (err, info) {
-        if (err) throw err;
-        assert.strictEqual(320, info.width);
-        assert.strictEqual(240, info.height);
-        fixtures.assertMaxColourDistance(output, fixtures.expected('extract-lch.jpg'), 9);
-        done();
-      });
+      .extractChannel(1)
+      .toBuffer();
+
+    assert.strictEqual(chroma, 104);
   });
 
   it('Alpha from 16-bit PNG', function (done) {
@@ -108,12 +102,12 @@ describe('Image channel extraction', function () {
     });
   });
 
-  it('Non-existent channel', function (done) {
-    sharp(fixtures.inputPng)
-      .extractChannel(1)
-      .toBuffer(function (err) {
-        assert(err instanceof Error);
-        done();
-      });
-  });
+  it('Non-existent channel', async () =>
+    await assert.rejects(
+      () => sharp({ create: { width: 1, height: 1, channels: 3, background: 'red' } })
+        .extractChannel(3)
+        .toBuffer(),
+      /Cannot extract channel 3 from image with channels 0-2/
+    )
+  );
 });