Deprecate limitInputPixels and sequentialRead, move to input options

docs/api-constructor.md

@@ -11,6 +11,11 @@
 -   `options` **[Object][3]?** if present, is an Object with optional attributes.
     -   `options.failOnError` **[Boolean][4]** by default halt processing and raise an error when loading invalid images.
          Set this flag to `false` if you'd rather apply a "best effort" to decode images, even if the data is corrupt or invalid. (optional, default `true`)
+    -   `options.limitInputPixels` **([Number][5] \| [Boolean][4])** Do not process input images where the number of pixels
+         (width x height) exceeds this limit. Assumes image dimensions contained in the input metadata can be trusted.
+         An integral Number of pixels, zero or false to remove limit, true to use default limit of 268402689 (0x3FFF x 0x3FFF). (optional, default `268402689`)
+    -   `options.sequentialRead` **[Boolean][4]** Set this to `true` to use sequential rather than random access where possible.
+         This can reduce memory usage and might improve performance on some systems. (optional, default `false`)
     -   `options.density` **[Number][5]** number representing the DPI for vector images. (optional, default `72`)
     -   `options.pages` **[Number][5]** number of pages to extract for multi-page input (GIF, TIFF, PDF), use -1 for all pages. (optional, default `1`)
     -   `options.page` **[Number][5]** page number to start extracting from for multi-page input (GIF, TIFF, PDF), zero based. (optional, default `0`)

docs/api-input.md

@@ -88,34 +88,6 @@ image
 
 Returns **[Promise][5]<[Object][6]>** 
 
-## limitInputPixels
-
-Do not process input images where the number of pixels (width x height) exceeds this limit.
-Assumes image dimensions contained in the input metadata can be trusted.
-The default limit is 268402689 (0x3FFF x 0x3FFF) pixels.
-
-### Parameters
-
--   `limit` **([Number][7] \| [Boolean][8])** an integral Number of pixels, zero or false to remove limit, true to use default limit.
-
-
--   Throws **[Error][9]** Invalid limit
-
-Returns **Sharp** 
-
-## sequentialRead
-
-An advanced setting that switches the libvips access method to `VIPS_ACCESS_SEQUENTIAL`.
-This will reduce memory usage and can improve performance on some systems.
-
-The default behaviour _before_ function call is `false`, meaning the libvips access method is not sequential.
-
-### Parameters
-
--   `sequentialRead` **[Boolean][8]**  (optional, default `true`)
-
-Returns **Sharp** 
-
 [1]: https://github.com/libvips/libvips/blob/master/libvips/iofuncs/enumtypes.c#L636
 
 [2]: https://github.com/libvips/libvips/blob/master/libvips/iofuncs/enumtypes.c#L672
@@ -127,9 +99,3 @@ Returns **Sharp**
 [5]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Promise
 
 [6]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Object
-
-[7]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number
-
-[8]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean
-
-[9]: https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Error

docs/changelog.md

@@ -12,6 +12,8 @@ Requires libvips v8.9.0.
 * Drop support for undefined input where options also provided.
   [#1768](https://github.com/lovell/sharp/issues/1768)
 
+* Move `limitInputPixels` and `sequentialRead` to input options, deprecating functions of the same name.
+
 * Expose `delay` and `loop` metadata for animated images.
   [#1905](https://github.com/lovell/sharp/issues/1905)
 

lib/constructor.js

@@ -86,6 +86,11 @@ const debuglog = util.debuglog('sharp');
  * @param {Object} [options] - if present, is an Object with optional attributes.
  * @param {Boolean} [options.failOnError=true] - by default halt processing and raise an error when loading invalid images.
  *  Set this flag to `false` if you'd rather apply a "best effort" to decode images, even if the data is corrupt or invalid.
+ * @param {Number|Boolean} [options.limitInputPixels=268402689] - Do not process input images where the number of pixels
+ *  (width x height) exceeds this limit. Assumes image dimensions contained in the input metadata can be trusted.
+ *  An integral Number of pixels, zero or false to remove limit, true to use default limit of 268402689 (0x3FFF x 0x3FFF).
+ * @param {Boolean} [options.sequentialRead=false] - Set this to `true` to use sequential rather than random access where possible.
+ *  This can reduce memory usage and might improve performance on some systems.
  * @param {Number} [options.density=72] - number representing the DPI for vector images.
  * @param {Number} [options.pages=1] - number of pages to extract for multi-page input (GIF, TIFF, PDF), use -1 for all pages.
  * @param {Number} [options.page=0] - page number to start extracting from for multi-page input (GIF, TIFF, PDF), zero based.
@@ -110,9 +115,6 @@ const Sharp = function (input, options) {
   }
   stream.Duplex.call(this);
   this.options = {
-    // input options
-    sequentialRead: false,
-    limitInputPixels: Math.pow(0x3FFF, 2),
     // resize options
     topOffsetPre: -1,
     leftOffsetPre: -1,

lib/input.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const util = require('util');
 const color = require('color');
 const is = require('./is');
 const sharp = require('../build/Release/sharp.node');
@@ -9,7 +10,11 @@ const sharp = require('../build/Release/sharp.node');
  * @private
  */
 function _createInputDescriptor (input, inputOptions, containerOptions) {
-  const inputDescriptor = { failOnError: true };
+  const inputDescriptor = {
+    failOnError: true,
+    limitInputPixels: Math.pow(0x3FFF, 2),
+    sequentialRead: false
+  };
   if (is.string(input)) {
     // filesystem
     inputDescriptor.file = input;
@@ -48,6 +53,26 @@ function _createInputDescriptor (input, inputOptions, containerOptions) {
         throw is.invalidParameterError('density', 'number between 1 and 2400', inputOptions.density);
       }
     }
+    // limitInputPixels
+    if (is.defined(inputOptions.limitInputPixels)) {
+      if (is.bool(inputOptions.limitInputPixels)) {
+        inputDescriptor.limitInputPixels = inputOptions.limitInputPixels
+          ? Math.pow(0x3FFF, 2)
+          : 0;
+      } else if (is.integer(inputOptions.limitInputPixels) && inputOptions.limitInputPixels >= 0) {
+        inputDescriptor.limitInputPixels = inputOptions.limitInputPixels;
+      } else {
+        throw is.invalidParameterError('limitInputPixels', 'integer >= 0', inputOptions.limitInputPixels);
+      }
+    }
+    // sequentialRead
+    if (is.defined(inputOptions.sequentialRead)) {
+      if (is.bool(inputOptions.sequentialRead)) {
+        inputDescriptor.sequentialRead = inputOptions.sequentialRead;
+      } else {
+        throw is.invalidParameterError('sequentialRead', 'boolean', inputOptions.sequentialRead);
+      }
+    }
     // Raw pixel input
     if (is.defined(inputOptions.raw)) {
       if (
@@ -307,14 +332,9 @@ function stats (callback) {
 }
 
 /**
- * Do not process input images where the number of pixels (width x height) exceeds this limit.
- * Assumes image dimensions contained in the input metadata can be trusted.
- * The default limit is 268402689 (0x3FFF x 0x3FFF) pixels.
- * @param {(Number|Boolean)} limit - an integral Number of pixels, zero or false to remove limit, true to use default limit.
- * @returns {Sharp}
- * @throws {Error} Invalid limit
+ * @private
  */
-function limitInputPixels (limit) {
+const limitInputPixels = util.deprecate(function limitInputPixels (limit) {
   // if we pass in false we represent the integer as 0 to disable
   if (limit === false) {
     limit = 0;
@@ -322,26 +342,20 @@ function limitInputPixels (limit) {
     limit = Math.pow(0x3FFF, 2);
   }
   if (is.integer(limit) && limit >= 0) {
-    this.options.limitInputPixels = limit;
+    this.options.input.limitInputPixels = limit;
   } else {
     throw is.invalidParameterError('limitInputPixels', 'integer', limit);
   }
   return this;
-}
+}, 'limitInputPixels is deprecated, use sharp(input, { limitInputPixels: false }) instead');
 
 /**
- * An advanced setting that switches the libvips access method to `VIPS_ACCESS_SEQUENTIAL`.
- * This will reduce memory usage and can improve performance on some systems.
- *
- * The default behaviour *before* function call is `false`, meaning the libvips access method is not sequential.
- *
- * @param {Boolean} [sequentialRead=true]
- * @returns {Sharp}
+ * @private
  */
-function sequentialRead (sequentialRead) {
-  this.options.sequentialRead = is.bool(sequentialRead) ? sequentialRead : true;
+const sequentialRead = util.deprecate(function sequentialRead (sequentialRead) {
+  this.options.input.sequentialRead = is.bool(sequentialRead) ? sequentialRead : true;
   return this;
-}
+}, 'sequentialRead is deprecated, use sharp(input, { sequentialRead: true }) instead');
 
 /**
  * Decorate the Sharp prototype with input-related functions.
@@ -357,6 +371,7 @@ module.exports = function (Sharp) {
     // Public
     metadata,
     stats,
+    // Deprecated
     limitInputPixels,
     sequentialRead
   });

src/common.cc

@@ -86,6 +86,10 @@ namespace sharp {
       descriptor->createHeight = AttrTo<uint32_t>(input, "createHeight");
       descriptor->createBackground = AttrAsRgba(input, "createBackground");
     }
+    // Limit input images to a given number of pixels, where pixels = width * height
+    descriptor->limitInputPixels = AttrTo<uint32_t>(input, "limitInputPixels");
+    // Allow switch from random to sequential access
+    descriptor->access = AttrTo<bool>(input, "sequentialRead") ? VIPS_ACCESS_SEQUENTIAL : VIPS_ACCESS_RANDOM;
     return descriptor;
   }
 
@@ -244,7 +248,7 @@ namespace sharp {
   /*
     Open an image from the given InputDescriptor (filesystem, compressed buffer, raw pixel data)
   */
-  std::tuple<VImage, ImageType> OpenInput(InputDescriptor *descriptor, VipsAccess accessMethod) {
+  std::tuple<VImage, ImageType> OpenInput(InputDescriptor *descriptor) {
     VImage image;
     ImageType imageType;
     if (descriptor->isBuffer) {
@@ -264,7 +268,7 @@ namespace sharp {
         if (imageType != ImageType::UNKNOWN) {
           try {
             vips::VOption *option = VImage::option()
-              ->set("access", accessMethod)
+              ->set("access", descriptor->access)
               ->set("fail", descriptor->failOnError);
             if (imageType == ImageType::SVG || imageType == ImageType::PDF) {
               option->set("dpi", descriptor->density);
@@ -310,7 +314,7 @@ namespace sharp {
         if (imageType != ImageType::UNKNOWN) {
           try {
             vips::VOption *option = VImage::option()
-              ->set("access", accessMethod)
+              ->set("access", descriptor->access)
               ->set("fail", descriptor->failOnError);
             if (imageType == ImageType::SVG || imageType == ImageType::PDF) {
               option->set("dpi", descriptor->density);
@@ -334,6 +338,11 @@ namespace sharp {
         }
       }
     }
+    // Limit input images to a given number of pixels, where pixels = width * height
+    if (descriptor->limitInputPixels > 0 &&
+      static_cast<uint64_t>(image.width() * image.height()) > static_cast<uint64_t>(descriptor->limitInputPixels)) {
+      throw vips::VError("Input image exceeds pixel limit");
+    }
     return std::make_tuple(image, imageType);
   }
 

src/common.h

@@ -48,6 +48,8 @@ namespace sharp {
     std::string file;
     char *buffer;
     bool failOnError;
+    int limitInputPixels;
+    VipsAccess access;
     size_t bufferLength;
     bool isBuffer;
     double density;
@@ -64,6 +66,8 @@ namespace sharp {
     InputDescriptor():
       buffer(nullptr),
       failOnError(TRUE),
+      limitInputPixels(0x3FFF * 0x3FFF),
+      access(VIPS_ACCESS_RANDOM),
       bufferLength(0),
       isBuffer(FALSE),
       density(72.0),
@@ -156,7 +160,7 @@ namespace sharp {
   /*
     Open an image from the given InputDescriptor (filesystem, compressed buffer, raw pixel data)
   */
-  std::tuple<VImage, ImageType> OpenInput(InputDescriptor *descriptor, VipsAccess accessMethod);
+  std::tuple<VImage, ImageType> OpenInput(InputDescriptor *descriptor);
 
   /*
     Does this image have an embedded profile?

src/metadata.cc

@@ -46,7 +46,7 @@ class MetadataWorker : public Nan::AsyncWorker {
     vips::VImage image;
     sharp::ImageType imageType = sharp::ImageType::UNKNOWN;
     try {
-      std::tie(image, imageType) = OpenInput(baton->input, VIPS_ACCESS_SEQUENTIAL);
+      std::tie(image, imageType) = OpenInput(baton->input);
     } catch (vips::VError const &err) {
       (baton->err).append(err.what());
     }

src/pipeline.cc

@@ -77,15 +77,7 @@ class PipelineWorker : public Nan::AsyncWorker {
       // Open input
       vips::VImage image;
       ImageType inputImageType;
-      std::tie(image, inputImageType) = sharp::OpenInput(baton->input, baton->accessMethod);
-
-      // Limit input images to a given number of pixels, where pixels = width * height
-      // Ignore if 0
-      if (baton->limitInputPixels > 0 &&
-        static_cast<uint64_t>(image.width() * image.height()) > static_cast<uint64_t>(baton->limitInputPixels)) {
-        (baton->err).append("Input image exceeds pixel limit");
-        return Error();
-      }
+      std::tie(image, inputImageType) = sharp::OpenInput(baton->input);
 
       // Calculate angle of rotation
       VipsAngle rotation;
@@ -270,7 +262,7 @@ class PipelineWorker : public Nan::AsyncWorker {
       if (shrink_on_load > 1) {
         // Reload input using shrink-on-load
         vips::VOption *option = VImage::option()
-          ->set("access", baton->accessMethod)
+          ->set("access", baton->input->access)
           ->set("shrink", shrink_on_load)
           ->set("fail", baton->input->failOnError);
         if (baton->input->buffer != nullptr) {
@@ -426,7 +418,7 @@ class PipelineWorker : public Nan::AsyncWorker {
         ImageType joinImageType = ImageType::UNKNOWN;
 
         for (unsigned int i = 0; i < baton->joinChannelIn.size(); i++) {
-          std::tie(joinImage, joinImageType) = sharp::OpenInput(baton->joinChannelIn[i], baton->accessMethod);
+          std::tie(joinImage, joinImageType) = sharp::OpenInput(baton->joinChannelIn[i]);
           image = image.bandjoin(joinImage);
         }
         image = image.copy(VImage::option()->set("interpretation", baton->colourspace));
@@ -479,7 +471,7 @@ class PipelineWorker : public Nan::AsyncWorker {
               baton->height = image.height();
             }
             image = image.tilecache(VImage::option()
-              ->set("access", baton->accessMethod)
+              ->set("access", VIPS_ACCESS_RANDOM)
               ->set("threaded", TRUE));
             image = image.smartcrop(baton->width, baton->height, VImage::option()
               ->set("interesting", baton->position == 16 ? VIPS_INTERESTING_ENTROPY : VIPS_INTERESTING_ATTENTION));
@@ -556,7 +548,7 @@ class PipelineWorker : public Nan::AsyncWorker {
         for (Composite *composite : baton->composite) {
           VImage compositeImage;
           ImageType compositeImageType = ImageType::UNKNOWN;
-          std::tie(compositeImage, compositeImageType) = OpenInput(composite->input, baton->accessMethod);
+          std::tie(compositeImage, compositeImageType) = OpenInput(composite->input);
           // Verify within current dimensions
           if (compositeImage.width() > image.width() || compositeImage.height() > image.height()) {
             throw vips::VError("Image to composite must have same dimensions or smaller");
@@ -646,7 +638,7 @@ class PipelineWorker : public Nan::AsyncWorker {
       if (baton->boolean != nullptr) {
         VImage booleanImage;
         ImageType booleanImageType = ImageType::UNKNOWN;
-        std::tie(booleanImage, booleanImageType) = sharp::OpenInput(baton->boolean, baton->accessMethod);
+        std::tie(booleanImage, booleanImageType) = sharp::OpenInput(baton->boolean);
         image = sharp::Boolean(image, booleanImage, baton->booleanOp);
       }
 
@@ -1193,10 +1185,6 @@ NAN_METHOD(pipeline) {
 
   // Input
   baton->input = CreateInputDescriptor(AttrAs<v8::Object>(options, "input"), buffersToPersist);
-  baton->accessMethod = AttrTo<bool>(options, "sequentialRead") ?
-    VIPS_ACCESS_SEQUENTIAL : VIPS_ACCESS_RANDOM;
-  // Limit input images to a given number of pixels, where pixels = width * height
-  baton->limitInputPixels = AttrTo<int32_t>(options, "limitInputPixels");
   // Extract image options
   baton->topOffsetPre = AttrTo<int32_t>(options, "topOffsetPre");
   baton->leftOffsetPre = AttrTo<int32_t>(options, "leftOffsetPre");
@@ -1408,11 +1396,12 @@ NAN_METHOD(pipeline) {
     // signal that we do not want to pass any value to dzSave
     baton->tileDepth = VIPS_FOREIGN_DZ_DEPTH_LAST;
   }
+
   // Force random access for certain operations
-  if (baton->accessMethod == VIPS_ACCESS_SEQUENTIAL && (
-    baton->trimThreshold > 0.0 || baton->normalise ||
-    baton->position == 16 || baton->position == 17)) {
-    baton->accessMethod = VIPS_ACCESS_RANDOM;
+  if (baton->input->access == VIPS_ACCESS_SEQUENTIAL) {
+    if (baton->trimThreshold > 0.0 || baton->normalise || baton->position == 16 || baton->position == 17) {
+      baton->input->access = VIPS_ACCESS_RANDOM;
+    }
   }
 
   // Function to notify of libvips warnings

src/pipeline.h

@@ -55,7 +55,6 @@ struct Composite {
 
 struct PipelineBaton {
   sharp::InputDescriptor *input;
-  int limitInputPixels;
   std::string formatOut;
   std::string fileOut;
   void *bufferOut;
@@ -119,7 +118,6 @@ struct PipelineBaton {
   int extendRight;
   std::vector<double> extendBackground;
   bool withoutEnlargement;
-  VipsAccess accessMethod;
   int jpegQuality;
   bool jpegProgressive;
   std::string jpegChromaSubsampling;
@@ -182,7 +180,6 @@ struct PipelineBaton {
 
   PipelineBaton():
     input(nullptr),
-    limitInputPixels(0),
     bufferOutLength(0),
     topOffsetPre(-1),
     topOffsetPost(-1),

src/stats.cc

@@ -62,7 +62,7 @@ class StatsWorker : public Nan::AsyncWorker {
     sharp::ImageType imageType = sharp::ImageType::UNKNOWN;
 
     try {
-      std::tie(image, imageType) = OpenInput(baton->input, baton->accessMethod);
+      std::tie(image, imageType) = OpenInput(baton->input);
     } catch (vips::VError const &err) {
       (baton->err).append(err.what());
     }
@@ -178,7 +178,6 @@ NAN_METHOD(stats) {
 
   // Input
   baton->input = sharp::CreateInputDescriptor(sharp::AttrAs<v8::Object>(options, "input"), buffersToPersist);
-  baton->accessMethod = AttrTo<bool>(options, "sequentialRead") ? VIPS_ACCESS_SEQUENTIAL : VIPS_ACCESS_RANDOM;
 
   // Function to notify of libvips warnings
   Nan::Callback *debuglog = new Nan::Callback(sharp::AttrAs<v8::Function>(options, "debuglog"));

src/stats.h

@@ -46,7 +46,6 @@ struct ChannelStats {
 struct StatsBaton {
   // Input
   sharp::InputDescriptor *input;
-  VipsAccess accessMethod;
 
   // Output
   std::vector<ChannelStats> channelStats;

test/bench/perf.js

@@ -460,8 +460,7 @@ async.series({
     }).add('sharp-sequentialRead', {
       defer: true,
       fn: function (deferred) {
-        sharp(inputJpgBuffer)
-          .sequentialRead()
+        sharp(inputJpgBuffer, { sequentialRead: true })
           .resize(width, height)
           .toBuffer(function (err, buffer) {
             if (err) {

test/unit/io.js

@@ -214,7 +214,27 @@ describe('Input/output', function () {
     });
   });
 
-  it('Sequential read, force JPEG', function (done) {
+  it('Invalid sequential read option throws', () => {
+    assert.throws(() => {
+      sharp({ sequentialRead: 'fail' });
+    }, /Expected boolean for sequentialRead but received fail of type string/);
+  });
+
+  it('Sequential read, force JPEG', () =>
+    sharp(fixtures.inputJpg, { sequentialRead: true })
+      .resize(320, 240)
+      .toFormat(sharp.format.jpeg)
+      .toBuffer({ resolveWithObject: true })
+      .then(({ data, info }) => {
+        assert.strictEqual(data.length > 0, true);
+        assert.strictEqual(data.length, info.size);
+        assert.strictEqual(info.format, 'jpeg');
+        assert.strictEqual(info.width, 320);
+        assert.strictEqual(info.height, 240);
+      })
+  );
+
+  it('Sequential read, force JPEG - deprecated', function (done) {
     sharp(fixtures.inputJpg)
       .sequentialRead()
       .resize(320, 240)
@@ -230,7 +250,21 @@ describe('Input/output', function () {
       });
   });
 
-  it('Not sequential read, force JPEG', function (done) {
+  it('Not sequential read, force JPEG', () =>
+    sharp(fixtures.inputJpg, { sequentialRead: false })
+      .resize(320, 240)
+      .toFormat('jpeg')
+      .toBuffer({ resolveWithObject: true })
+      .then(({ data, info }) => {
+        assert.strictEqual(data.length > 0, true);
+        assert.strictEqual(data.length, info.size);
+        assert.strictEqual(info.format, 'jpeg');
+        assert.strictEqual(info.width, 320);
+        assert.strictEqual(info.height, 240);
+      })
+  );
+
+  it('Not sequential read, force JPEG - deprecated', function (done) {
     sharp(fixtures.inputJpg)
       .sequentialRead(false)
       .resize(320, 240)
@@ -559,7 +593,67 @@ describe('Input/output', function () {
       });
   });
 
-  describe('Limit pixel count of input image', function () {
+  describe('Limit pixel count of input image', () => {
+    it('Invalid fails - negative', () => {
+      assert.throws(() => {
+        sharp({ limitInputPixels: -1 });
+      });
+    });
+
+    it('Invalid fails - float', () => {
+      assert.throws(() => {
+        sharp({ limitInputPixels: 12.3 });
+      });
+    });
+
+    it('Invalid fails - string', () => {
+      assert.throws(() => {
+        sharp({ limitInputPixels: 'fail' });
+      });
+    });
+
+    it('Same size as input works', () =>
+      sharp(fixtures.inputJpg)
+        .metadata()
+        .then(({ width, height }) =>
+          sharp(fixtures.inputJpg, { limitInputPixels: width * height }).toBuffer()
+        )
+    );
+
+    it('Disabling limit works', () =>
+      sharp(fixtures.inputJpgLarge, { limitInputPixels: false })
+        .resize(2)
+        .toBuffer()
+    );
+
+    it('Enabling default limit works and fails with a large image', () =>
+      sharp(fixtures.inputJpgLarge, { limitInputPixels: true })
+        .toBuffer()
+        .then(() => {
+          assert.fail('Expected to fail');
+        })
+        .catch(err => {
+          assert.strictEqual(err.message, 'Input image exceeds pixel limit');
+        })
+    );
+
+    it('Smaller than input fails', () =>
+      sharp(fixtures.inputJpg)
+        .metadata()
+        .then(({ width, height }) =>
+          sharp(fixtures.inputJpg, { limitInputPixels: width * height - 1 })
+            .toBuffer()
+            .then(() => {
+              assert.fail('Expected to fail');
+            })
+            .catch(err => {
+              assert.strictEqual(err.message, 'Input image exceeds pixel limit');
+            })
+        )
+    );
+  });
+
+  describe('Limit pixel count of input image - deprecated', function () {
     it('Invalid fails - negative', function (done) {
       let isValid = false;
       try {