Avoid pointer arguments in custom stream callbacks (#74)

kleisauke · Sep 18, 2024 · 26aaa0c · 26aaa0c
1 parent e323742
commit 26aaa0c
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 68 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 Uses libvips v8.15.3, compiled with Emscripten v3.1.67.
 
+### Changed
+
+- Avoid pointer arguments in `SourceCustom.onRead` and
+  `TargetCustom.onWrite` callbacks.
+  [#74](https://github.com/kleisauke/wasm-vips/issues/74)
+
 ## [v0.0.10] - 2024-08-14
 
 Uses libvips v8.15.3, compiled with Emscripten v3.1.64.

diff --git a/build/preamble_vips.d.ts b/build/preamble_vips.d.ts
@@ -344,11 +344,13 @@ declare module Vips {
     class SourceCustom extends Source {
         /**
          * Attach a read handler.
-         * @param ptr A pointer to an array of bytes where the read content is stored.
-         * @param size The maximum number of bytes to be read.
-         * @return The total number of bytes read into the buffer.
+         * The handler is given a number of bytes to fetch {@link length}, and should return a
+         * bytes-like object containing up to that number of bytes. If there's no more data
+         * available, it should return `undefined`.
+         * @param length The maximum number of bytes to be read.
+         * @return A blob up to {@link length} bytes or `undefined` if there's no more data available.
          */
-        onRead: (ptr: number, size: bigint) => bigint;
+        onRead: (length: number) => Blob | undefined;
 
         /**
          * Attach a seek handler.
@@ -358,7 +360,7 @@ declare module Vips {
          * @param size A value indicating the reference point used to obtain the new position.
          * @return The new position within the current source.
          */
-        onSeek: (offset: bigint, whence: number) => bigint;
+        onSeek: (offset: number, whence: number) => number;
     }
 
     /**
@@ -407,30 +409,28 @@ declare module Vips {
     class TargetCustom extends Target {
         /**
          * Attach a write handler.
-         * @param ptr A pointer to an array of bytes which will be written to.
-         * @param length The number of bytes to write.
+         * @param data A typed array of 8-bit unsigned integer values.
          * @return The number of bytes that were written.
          */
-        onWrite: (ptr: number, size: bigint) => bigint;
+        onWrite: (data: Uint8Array) => number;
 
         /* libtiff needs to be able to seek and read on targets, unfortunately.
          */
 
         /**
          * Attach a read handler.
-         * @param ptr A pointer to an array of bytes where the read content is stored.
-         * @param size The maximum number of bytes to be read.
-         * @return The total number of bytes read from the target.
+         * @param length The maximum number of bytes to be read.
+         * @return A blob up to {@link length} bytes or `undefined` if there's no more data available.
          */
-        onRead: (ptr: number, size: bigint) => bigint;
+        onRead: (length: number) => Blob | undefined;
 
         /**
          * Attach a seek handler.
          * @param offset A byte offset relative to the whence parameter.
          * @param size A value indicating the reference point used to obtain the new position.
          * @return The new position within the current target.
          */
-        onSeek: (offset: bigint, whence: number) => bigint;
+        onSeek: (offset: number, whence: number) => number;
 
         /**
          * Attach an end handler.

diff --git a/lib/vips.d.ts b/lib/vips.d.ts
diff --git a/src/bindings/connection.cpp b/src/bindings/connection.cpp
@@ -24,7 +24,7 @@ Source Source::new_from_memory(const std::string &memory) {
     return Source(input);
 }
 
-int64_t SourceCustom::read_handler(VipsSourceCustom *source, void *buffer,
+int64_t SourceCustom::read_handler(VipsSourceCustom *source, void *data,
                                    int64_t length, void *user) {
     if (length <= 0)
         return 0;
@@ -33,9 +33,18 @@ int64_t SourceCustom::read_handler(VipsSourceCustom *source, void *buffer,
     if (self->read_callback == nullptr)
         return -1;
 
-    int64_t bytes_read;
+    int64_t bytes_read = 0;
     proxy_sync([&]() {
-        bytes_read = self->read_callback(buffer, length);
+        emscripten::val val = emscripten::val::take_ownership(
+            self->read_callback(static_cast<int>(length)));
+        if (val.isUndefined())
+            return;
+
+        std::string buffer = val.as<std::string>();
+        bytes_read = buffer.size();
+
+        if (bytes_read > 0)
+            memcpy(data, buffer.data(), bytes_read);
     });
 
     return bytes_read;
@@ -49,23 +58,22 @@ int64_t SourceCustom::seek_handler(VipsSourceCustom *source, int64_t offset,
 
     int64_t new_pos;
     proxy_sync([&]() {
-        new_pos = self->seek_callback(offset, whence);
+        new_pos = self->seek_callback(static_cast<int>(offset), whence);
     });
 
     return new_pos;
 }
 
 void SourceCustom::set_read_callback(emscripten::val js_func) {
     emscripten::val ptr = emscripten::val::module_property("addFunction")(
-        js_func, emscripten::val("jpj"));
-    read_callback =
-        reinterpret_cast<int64_t (*)(void *, int64_t)>(ptr.as<int>());
+        js_func, emscripten::val("ii"));
+    read_callback = reinterpret_cast<ReadCallback>(ptr.as<int>());
 }
 
 void SourceCustom::set_seek_callback(emscripten::val js_func) {
     emscripten::val ptr = emscripten::val::module_property("addFunction")(
-        js_func, emscripten::val("jji"));
-    seek_callback = reinterpret_cast<int64_t (*)(int64_t, int)>(ptr.as<int>());
+        js_func, emscripten::val("iii"));
+    seek_callback = reinterpret_cast<SeekCallback>(ptr.as<int>());
 }
 
 Target Target::new_to_file(const std::string &filename) {
@@ -86,22 +94,23 @@ Target Target::new_to_memory() {
     return Target(output);
 }
 
-int64_t TargetCustom::write_handler(VipsTargetCustom *target,
-                                    const void *buffer, int64_t length,
-                                    void *user) {
+int64_t TargetCustom::write_handler(VipsTargetCustom *target, const void *data,
+                                    int64_t length, void *user) {
     TargetCustom *self = reinterpret_cast<TargetCustom *>(user);
     if (self->write_callback == nullptr)
         return -1;
 
     int64_t bytes_written;
     proxy_sync([&]() {
-        bytes_written = self->write_callback(buffer, length);
+        emscripten::val buffer = BlobVal.new_(emscripten::typed_memory_view(
+            length, static_cast<const uint8_t *>(data)));
+        bytes_written = self->write_callback(buffer.as_handle());
     });
 
     return bytes_written;
 }
 
-int64_t TargetCustom::read_handler(VipsTargetCustom *target, void *buffer,
+int64_t TargetCustom::read_handler(VipsTargetCustom *target, void *data,
                                    int64_t length, void *user) {
     if (length <= 0)
         return 0;
@@ -110,9 +119,18 @@ int64_t TargetCustom::read_handler(VipsTargetCustom *target, void *buffer,
     if (self->read_callback == nullptr)
         return -1;
 
-    int64_t bytes_read;
+    int64_t bytes_read = 0;
     proxy_sync([&]() {
-        bytes_read = self->read_callback(buffer, length);
+        emscripten::val val = emscripten::val::take_ownership(
+            self->read_callback(static_cast<int>(length)));
+        if (val.isUndefined())
+            return;
+
+        std::string buffer = val.as<std::string>();
+        bytes_read = buffer.size();
+
+        if (bytes_read > 0)
+            memcpy(data, buffer.data(), bytes_read);
     });
 
     return bytes_read;
@@ -126,7 +144,7 @@ int64_t TargetCustom::seek_handler(VipsTargetCustom *target, int64_t offset,
 
     int64_t new_pos;
     proxy_sync([&]() {
-        new_pos = self->seek_callback(offset, whence);
+        new_pos = self->seek_callback(static_cast<int>(offset), whence);
     });
 
     return new_pos;
@@ -147,28 +165,26 @@ int TargetCustom::end_handler(VipsTargetCustom *target, void *user) {
 
 void TargetCustom::set_write_callback(emscripten::val js_func) {
     emscripten::val ptr = emscripten::val::module_property("addFunction")(
-        js_func, emscripten::val("jpj"));
-    write_callback =
-        reinterpret_cast<int64_t (*)(const void *, int64_t)>(ptr.as<int>());
+        js_func, emscripten::val("ii"));
+    write_callback = reinterpret_cast<WriteCallback>(ptr.as<int>());
 }
 
 void TargetCustom::set_read_callback(emscripten::val js_func) {
     emscripten::val ptr = emscripten::val::module_property("addFunction")(
-        js_func, emscripten::val("jpj"));
-    read_callback =
-        reinterpret_cast<int64_t (*)(void *, int64_t)>(ptr.as<int>());
+        js_func, emscripten::val("ii"));
+    read_callback = reinterpret_cast<ReadCallback>(ptr.as<int>());
 }
 
 void TargetCustom::set_seek_callback(emscripten::val js_func) {
     emscripten::val ptr = emscripten::val::module_property("addFunction")(
-        js_func, emscripten::val("jji"));
-    seek_callback = reinterpret_cast<int64_t (*)(int64_t, int)>(ptr.as<int>());
+        js_func, emscripten::val("iii"));
+    seek_callback = reinterpret_cast<SeekCallback>(ptr.as<int>());
 }
 
 void TargetCustom::set_end_callback(emscripten::val js_func) {
     emscripten::val ptr = emscripten::val::module_property("addFunction")(
         js_func, emscripten::val("i"));
-    end_callback = reinterpret_cast<int (*)()>(ptr.as<int>());
+    end_callback = reinterpret_cast<EndCallback>(ptr.as<int>());
 }
 
 }  // namespace vips
diff --git a/src/bindings/connection.h b/src/bindings/connection.h
@@ -9,6 +9,14 @@
 
 namespace vips {
 
+// sig = ii
+using ReadCallback = emscripten::EM_VAL (*)(int length);
+using WriteCallback = int (*)(emscripten::EM_VAL data);
+// sig = iii
+using SeekCallback = int (*)(int offset, int whence);
+// sig = i
+using EndCallback = int (*)();
+
 class Connection : public Object {
  public:
     explicit Connection(VipsConnection *connection)
@@ -74,10 +82,8 @@ class SourceCustom : public Source {
     }
 
  private:
-    // sig = jpj
-    int64_t (*read_callback)(void *data, int64_t length) = nullptr;
-    // sig = jji
-    int64_t (*seek_callback)(int64_t offset, int whence) = nullptr;
+    ReadCallback read_callback = nullptr;
+    SeekCallback seek_callback = nullptr;
 };
 
 class Target : public Connection {
@@ -149,13 +155,10 @@ class TargetCustom : public Target {
     }
 
  private:
-    // sig = jpj
-    int64_t (*write_callback)(const void *data, int64_t length) = nullptr;
-    int64_t (*read_callback)(void *data, int64_t length) = nullptr;
-    // sig = jji
-    int64_t (*seek_callback)(int64_t offset, int whence) = nullptr;
-    // sig = i
-    int (*end_callback)() = nullptr;
+    WriteCallback write_callback = nullptr;
+    ReadCallback read_callback = nullptr;
+    SeekCallback seek_callback = nullptr;
+    EndCallback end_callback = nullptr;
 };
 
 }  // namespace vips
diff --git a/src/vips-library.js b/src/vips-library.js
@@ -4,6 +4,7 @@ var LibraryVips = {
     '$ENV',
 #endif
     '$ClassHandle',
+    '$Emval',
     '$deletionQueue',
   ],
   $VIPS__postset: 'VIPS.init();',
@@ -20,6 +21,23 @@ var LibraryVips = {
         ENV['VIPS_CONCURRENCY'] = 1;
       });
 #endif
+      addOnInit(() => {
+        // SourceCustom.onRead marshaller 
+        const sourceCustom = Object.getOwnPropertyDescriptor(Module['SourceCustom'].prototype, 'onRead');
+        Object.defineProperty(Module['SourceCustom'].prototype, 'onRead', {
+          set(cb) {
+            return sourceCustom.set.call(this, (length) => Emval.toHandle(cb(length)));
+          }
+        });
+
+        // TargetCustom.onWrite marshaller 
+        const targetCustom = Object.getOwnPropertyDescriptor(Module['TargetCustom'].prototype, 'onWrite');
+        Object.defineProperty(Module['TargetCustom'].prototype, 'onWrite', {
+          set(cb) {
+            return targetCustom.set.call(this, (data) => cb(Emval.toValue(data)));
+          }
+        });
+      });
 
       // Add preventAutoDelete method to ClassHandle
       Object.assign(ClassHandle.prototype, {