From ae284a6c3002c3d21bb4f8a6e4d6d134bd3e43f3 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Sat, 11 Jul 2026 14:39:42 +0600 Subject: [PATCH] refactor(mobile): consolidate thumbhash into the rust core and clean up the ffi layer --- .../app/alextran/immich/NativeBuffer.kt | 3 - .../kotlin/app/alextran/immich/NativeImage.kt | 8 ++ .../alextran/immich/images/LocalImagesImpl.kt | 12 ++- .../app/alextran/immich/images/ThumbHash.java | 39 -------- mobile/integration_test/native_core_test.dart | 43 ++++---- mobile/ios/Runner.xcodeproj/project.pbxproj | 4 - mobile/ios/Runner/Core/NativeCore.swift | 12 +-- .../ios/Runner/Images/LocalImagesImpl.swift | 16 +-- mobile/ios/Runner/Images/Thumbhash.swift | 31 ------ native/.gitignore | 4 +- native/Cargo.toml | 3 +- native/README.md | 33 ++++--- native/crates/immich_core/src/lib.rs | 2 +- native/crates/immich_core_ffi/Cargo.toml | 5 + native/crates/immich_core_ffi/build.rs | 4 +- .../immich_core_ffi/include/immich_core.h | 35 +++---- .../immich_core_ffi/src/android/buffer.rs | 33 +------ .../immich_core_ffi/src/android/image.rs | 50 ++++++++-- .../crates/immich_core_ffi/src/android/mod.rs | 1 - .../immich_core_ffi/src/android/thumbhash.rs | 51 ---------- .../crates/immich_core_ffi/src/capi/image.rs | 9 +- .../immich_core_ffi/src/capi/thumbhash.rs | 98 ++++++++++--------- native/crates/immich_core_ffi/src/lib.rs | 2 +- native/crates/immich_core_ffi/src/runtime.rs | 4 +- native/crates/immich_core_ffi/tests/c_abi.rs | 64 +++++------- native/immich_native_core/README.md | 6 +- native/immich_native_core/hook/build.dart | 10 ++ .../lib/src/ffi/bindings.g.dart | 46 +++------ .../test/native_core_test.dart | 36 +++---- native/mise.toml | 2 +- 30 files changed, 261 insertions(+), 405 deletions(-) delete mode 100644 mobile/android/app/src/main/kotlin/app/alextran/immich/images/ThumbHash.java delete mode 100644 mobile/ios/Runner/Images/Thumbhash.swift delete mode 100644 native/crates/immich_core_ffi/src/android/thumbhash.rs diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt index 713aef909a..9c0f7000b4 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeBuffer.kt @@ -22,9 +22,6 @@ object NativeBuffer { @JvmStatic external fun wrap(address: Long, capacity: Int): ByteBuffer - @JvmStatic - external fun copy(buffer: ByteBuffer, destAddress: Long, offset: Int, length: Int) - @JvmStatic external fun createGlobalRef(obj: Any): Long } diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt index c90a1216a4..4c56f0be5e 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/NativeImage.kt @@ -26,4 +26,12 @@ object NativeImage { */ @JvmStatic external fun convert1010102(bitmap: Bitmap, outInfo: IntArray): Long + + /** + * Decodes a ThumbHash placeholder into a freshly malloc'd RGBA_8888 native buffer. Returns the + * buffer address (free it with [NativeBuffer.free]) and fills [outInfo] with + * {width, height, rowBytes}. Returns 0 when the hash is malformed. + */ + @JvmStatic + external fun thumbhash(hash: ByteArray, outInfo: IntArray): Long } diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt index 2f5db1ca0e..88bb87c1d3 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt @@ -107,12 +107,14 @@ class LocalImagesImpl(context: Context) : LocalImageApi { threadPool.execute { try { val bytes = Base64.getDecoder().decode(thumbhash) - val image = ThumbHash.thumbHashToRGBA(bytes) + val info = IntArray(3) + val pointer = NativeImage.thumbhash(bytes, info) + require(pointer != 0L) { "Invalid thumbhash" } val res = mapOf( - "pointer" to image.pointer, - "width" to image.width.toLong(), - "height" to image.height.toLong(), - "rowBytes" to (image.width * 4).toLong() + "pointer" to pointer, + "width" to info[0].toLong(), + "height" to info[1].toLong(), + "rowBytes" to info[2].toLong() ) callback(Result.success(res)) } catch (e: Exception) { diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/ThumbHash.java b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/ThumbHash.java deleted file mode 100644 index 5541f59426..0000000000 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/ThumbHash.java +++ /dev/null @@ -1,39 +0,0 @@ -package app.alextran.immich.images; - -public final class ThumbHash { - static { - // The decode lives in the shared Rust core (immich_core::thumbhash). - System.loadLibrary("immich_core_ffi"); - } - - /** - * Decodes a ThumbHash to an RGBA image. RGB is not be premultiplied by A. - * The pixels live in a native buffer the caller owns (freed via - * NativeBuffer.free or dart's malloc.free). - * - * @param hash The bytes of the ThumbHash. - * @return The width, height, and pixels of the rendered placeholder image. - */ - public static Image thumbHashToRGBA(byte[] hash) { - int[] info = new int[3]; - long pointer = nativeDecode(hash, info); - if (pointer == 0) { - throw new IllegalArgumentException("Invalid thumbhash"); - } - return new Image(info[0], info[1], pointer); - } - - private static native long nativeDecode(byte[] hash, int[] outInfo); - - public static final class Image { - public int width; - public int height; - public long pointer; - - public Image(int width, int height, long pointer) { - this.width = width; - this.height = height; - this.pointer = pointer; - } - } -} diff --git a/mobile/integration_test/native_core_test.dart b/mobile/integration_test/native_core_test.dart index a41a28effe..f69e7cafc7 100644 --- a/mobile/integration_test/native_core_test.dart +++ b/mobile/integration_test/native_core_test.dart @@ -1,10 +1,10 @@ // Plumbing check: proves immich_native_core is usable from the real immich app on // a real device — the build hook compiled the Rust for this target, the code asset -// bundled into the app, and the @Native symbols resolve at runtime. The two -// payloads run against their device-verified ground truth: the EXIF rotate ported -// from native_image.c (#29337) and the 10-bit convert matching Skia's Bitmap.copy -// (#29631). Calls the generated bindings directly — dart is the test harness here; -// the production callers are the platform decode pipelines. +// bundled into the app, and the @Native symbols resolve at runtime. The payloads +// run against their device-verified ground truth: the EXIF rotate ported from +// native_image.c (#29337), the 10-bit convert matching Skia's Bitmap.copy (#29631), +// and the thumbhash decode. Calls the generated bindings directly — dart is the +// test harness here; the production callers are the platform decode pipelines. // Self-contained: does NOT boot the immich app or need a server. // // Run: flutter test integration_test/native_core_test.dart -d @@ -82,34 +82,29 @@ void main() { expect(out.sublist(4, 8), [45, 28, 0, 255]); }); - test('thumbhash decodes via the core: dims then fill', () { + test('thumbhash decodes via the core into a malloc buffer', () { final hash = base64Decode('1QcSHQRnh493V4dIh4eXh1h4kJUI'); final hashPtr = malloc(hash.length); - final w = malloc(); - final h = malloc(); + final info = malloc(3); try { hashPtr.asTypedList(hash.length).setAll(0, hash); - expect(immich_core_thumbhash_dims(hashPtr, hash.length, w, h), isTrue); - expect((w.value, h.value), (23, 32)); + final ptr = immich_core_thumbhash_decode(hashPtr, hash.length, info); + expect(ptr, isNot(equals(nullptr))); + expect((info[0], info[1], info[2]), (23, 32, 23 * 4)); - final len = w.value * h.value * 4; - final dst = malloc(len); - try { - expect(immich_core_thumbhash_to_rgba(hashPtr, hash.length, dst, len), isTrue); - final pixels = dst.asTypedList(len); - for (var i = 3; i < len; i += 4) { - expect(pixels[i], 255, reason: 'alpha at $i'); - } - expect(pixels.toSet().length, greaterThan(2)); - } finally { - malloc.free(dst); + final len = info[0] * info[1] * 4; + final pixels = ptr.asTypedList(len); + for (var i = 3; i < len; i += 4) { + expect(pixels[i], 255, reason: 'alpha at $i'); } + expect(pixels.toSet().length, greaterThan(2)); + malloc.free(ptr); - expect(immich_core_thumbhash_dims(hashPtr, 4, w, h), isFalse); + // malformed hash: null return, info untouched + expect(immich_core_thumbhash_decode(hashPtr, 4, info), equals(nullptr)); } finally { malloc.free(hashPtr); - malloc.free(w); - malloc.free(h); + malloc.free(info); } }); } diff --git a/mobile/ios/Runner.xcodeproj/project.pbxproj b/mobile/ios/Runner.xcodeproj/project.pbxproj index 96d304f3d7..f198e30236 100644 --- a/mobile/ios/Runner.xcodeproj/project.pbxproj +++ b/mobile/ios/Runner.xcodeproj/project.pbxproj @@ -36,7 +36,6 @@ FE5499F62F11980E006016CB /* LocalImagesImpl.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE5499F52F11980E006016CB /* LocalImagesImpl.swift */; }; FE5499F82F1198E2006016CB /* RemoteImagesImpl.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE5499F72F1198DE006016CB /* RemoteImagesImpl.swift */; }; FE5FE4AE2F30FBC000A71243 /* ImageProcessing.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE5FE4AD2F30FBC000A71243 /* ImageProcessing.swift */; }; - FEAFA8732E4D42F4001E47FE /* Thumbhash.swift in Sources */ = {isa = PBXBuildFile; fileRef = FEAFA8722E4D42F4001E47FE /* Thumbhash.swift */; }; FEE084F82EC172460045228E /* SQLiteData in Frameworks */ = {isa = PBXBuildFile; productRef = FEE084F72EC172460045228E /* SQLiteData */; }; FEE084FB2EC1725A0045228E /* RawStructuredFieldValues in Frameworks */ = {isa = PBXBuildFile; productRef = FEE084FA2EC1725A0045228E /* RawStructuredFieldValues */; }; FEE084FD2EC1725A0045228E /* StructuredFieldValues in Frameworks */ = {isa = PBXBuildFile; productRef = FEE084FC2EC1725A0045228E /* StructuredFieldValues */; }; @@ -130,7 +129,6 @@ FE5499F52F11980E006016CB /* LocalImagesImpl.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocalImagesImpl.swift; sourceTree = ""; }; FE5499F72F1198DE006016CB /* RemoteImagesImpl.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RemoteImagesImpl.swift; sourceTree = ""; }; FE5FE4AD2F30FBC000A71243 /* ImageProcessing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageProcessing.swift; sourceTree = ""; }; - FEAFA8722E4D42F4001E47FE /* Thumbhash.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Thumbhash.swift; sourceTree = ""; }; /* End PBXFileReference section */ /* Begin PBXFileSystemSynchronizedBuildFileExceptionSet section */ @@ -349,7 +347,6 @@ FE5499F52F11980E006016CB /* LocalImagesImpl.swift */, FE5499F12F1197D8006016CB /* LocalImages.g.swift */, FE5499F22F1197D8006016CB /* RemoteImages.g.swift */, - FEAFA8722E4D42F4001E47FE /* Thumbhash.swift */, ); path = Images; sourceTree = ""; @@ -646,7 +643,6 @@ B2EE00022E72CA15008B6CA7 /* PermissionApi.g.swift in Sources */, B2EE00042E72CA15008B6CA7 /* PermissionApiImpl.swift in Sources */, FE5499F82F1198E2006016CB /* RemoteImagesImpl.swift in Sources */, - FEAFA8732E4D42F4001E47FE /* Thumbhash.swift in Sources */, B25D377C2E72CA26008B6CA7 /* ConnectivityApiImpl.swift in Sources */, B21E34AA2E5AFD2B0031FDB9 /* BackgroundWorkerApiImpl.swift in Sources */, 1498D2341E8E89220040F4C2 /* GeneratedPluginRegistrant.m in Sources */, diff --git a/mobile/ios/Runner/Core/NativeCore.swift b/mobile/ios/Runner/Core/NativeCore.swift index 8f485d15ed..aafd1f8a5b 100644 --- a/mobile/ios/Runner/Core/NativeCore.swift +++ b/mobile/ios/Runner/Core/NativeCore.swift @@ -5,15 +5,11 @@ import Foundation // framework in the bundle without linking Runner against it, so symbols are // resolved at runtime. Signatures mirror native/crates/immich_core_ffi/include/immich_core.h. enum NativeCore { - typealias ThumbhashDims = @convention(c) ( - UnsafePointer?, UInt, UnsafeMutablePointer?, UnsafeMutablePointer? - ) -> Bool - typealias ThumbhashToRgba = @convention(c) ( - UnsafePointer?, UInt, UnsafeMutablePointer?, UInt - ) -> Bool + typealias ThumbhashDecode = @convention(c) ( + UnsafePointer?, UInt, UnsafeMutablePointer? + ) -> UnsafeMutablePointer? - static let thumbhashDims: ThumbhashDims? = symbol("immich_core_thumbhash_dims") - static let thumbhashToRgba: ThumbhashToRgba? = symbol("immich_core_thumbhash_to_rgba") + static let thumbhashDecode: ThumbhashDecode? = symbol("immich_core_thumbhash_decode") private static let handle: UnsafeMutableRawPointer? = { if let frameworks = Bundle.main.privateFrameworksPath { diff --git a/mobile/ios/Runner/Images/LocalImagesImpl.swift b/mobile/ios/Runner/Images/LocalImagesImpl.swift index d06e4ccb81..912f2a25d2 100644 --- a/mobile/ios/Runner/Images/LocalImagesImpl.swift +++ b/mobile/ios/Runner/Images/LocalImagesImpl.swift @@ -38,17 +38,21 @@ class LocalImageApiImpl: LocalImageApi { func getThumbhash(thumbhash: String, completion: @escaping (Result<[String : Int64], any Error>) -> Void) { ImageProcessing.queue.addOperation { - guard let data = Data(base64Encoded: thumbhash) + guard let data = Data(base64Encoded: thumbhash), let decode = NativeCore.thumbhashDecode else { return completion(.failure(PigeonError(code: "", message: "Invalid base64 string: \(thumbhash)", details: nil)))} - guard let (width, height, pointer) = thumbHashToRGBA(hash: data) + var info = [UInt32](repeating: 0, count: 3) + let pointer = data.withUnsafeBytes { bytes in + decode(bytes.bindMemory(to: UInt8.self).baseAddress, UInt(bytes.count), &info) + } + guard let pointer else { return completion(.failure(PigeonError(code: "", message: "Invalid thumbhash: \(thumbhash)", details: nil)))} completion(.success([ - "pointer": Int64(Int(bitPattern: pointer.baseAddress)), - "width": Int64(width), - "height": Int64(height), - "rowBytes": Int64(width * 4) + "pointer": Int64(Int(bitPattern: pointer)), + "width": Int64(info[0]), + "height": Int64(info[1]), + "rowBytes": Int64(info[2]) ])) } } diff --git a/mobile/ios/Runner/Images/Thumbhash.swift b/mobile/ios/Runner/Images/Thumbhash.swift deleted file mode 100644 index d132685f6c..0000000000 --- a/mobile/ios/Runner/Images/Thumbhash.swift +++ /dev/null @@ -1,31 +0,0 @@ -import Foundation - -// The decode lives in the shared Rust core (immich_core::thumbhash); this is the -// malloc-and-call glue. The buffer is libc-allocated so dart frees the exact -// address with malloc.free. Returns nil on a malformed hash instead of trapping. -func thumbHashToRGBA(hash: Data) -> (Int, Int, UnsafeMutableRawBufferPointer)? { - guard let dims = NativeCore.thumbhashDims, let toRgba = NativeCore.thumbhashToRgba else { - return nil - } - - var w: UInt32 = 0 - var h: UInt32 = 0 - let parsed = hash.withUnsafeBytes { bytes in - dims(bytes.bindMemory(to: UInt8.self).baseAddress, UInt(bytes.count), &w, &h) - } - guard parsed, w > 0, h > 0 else { return nil } - - let byteCount = Int(w) * Int(h) * 4 - guard let raw = malloc(byteCount) else { return nil } - let filled = hash.withUnsafeBytes { bytes in - toRgba( - bytes.bindMemory(to: UInt8.self).baseAddress, UInt(bytes.count), - raw.assumingMemoryBound(to: UInt8.self), UInt(byteCount) - ) - } - guard filled else { - free(raw) - return nil - } - return (Int(w), Int(h), UnsafeMutableRawBufferPointer(start: raw, count: byteCount)) -} diff --git a/native/.gitignore b/native/.gitignore index 76ce80b49a..4ae97b8ece 100644 --- a/native/.gitignore +++ b/native/.gitignore @@ -1,5 +1,5 @@ /target smoke/*.node # generated + committed (regen via `mise run codegen`): -# crates/immich_core_dart/include/immich_core.h (cbindgen) -# immich_native_core/lib/immich_native_core_bindings_generated.dart (ffigen) +# crates/immich_core_ffi/include/immich_core.h (cbindgen) +# immich_native_core/lib/src/ffi/bindings.g.dart (ffigen) diff --git a/native/Cargo.toml b/native/Cargo.toml index ed40933d1c..0d749cd3c0 100644 --- a/native/Cargo.toml +++ b/native/Cargo.toml @@ -35,7 +35,8 @@ napi-derive = "3" napi-build = "2" cbindgen = { version = "0.29", default-features = false } -# CI-enforced (not review-hoped): the boundary crate also #![deny]s unwrap/expect. +# enforced by `mise run lint` (clippy -D warnings); the boundary crate also +# #![deny]s unwrap/expect. [workspace.lints.clippy] undocumented_unsafe_blocks = "deny" diff --git a/native/README.md b/native/README.md index d13e01801b..bb8492a4fe 100644 --- a/native/README.md +++ b/native/README.md @@ -1,27 +1,30 @@ -# immich_native_core (PoC) +# immich_native_core Shared Rust core for the pixel work the platforms can't do without native code, -built from source into the app via Flutter native assets. - -The two capabilities are ports of native code we already ship or have in review, -so the same logic exists once, tested, instead of per-platform C: -- **EXIF-orientation rotate** — from `mobile/android/app/src/main/cpp/native_image.c` - (#29337, merged; fixes #24796, sideways RAW photos). Byte-for-byte the same affine - + tiled copy, plus bounds checks the raw C can't have. +built from source into the app via Flutter native assets. It replaces the android +C layer (`native_buffer.c`/`native_image.c`) and the per-platform thumbhash ports +(`ThumbHash.java`/`Thumbhash.swift`), so the same logic exists once, tested: +- **EXIF-orientation rotate** — from `native_image.c` (#29337; fixes #24796, + sideways RAW photos). Byte-for-byte the same affine + tiled copy, plus bounds + checks the raw C can't have. - **RGBA_1010102 → RGBA8888 convert** — the 10-bit HEIC/AVIF color fix (#29631, fixes #24906). Same `round(v*255/1023)` LUT + packing as the C, proven on-device against Skia's `Bitmap.copy(ARGB_8888)`. +- **ThumbHash decode** — one bounds-checked implementation instead of the two + platform ports, which disagreed on rounding and crashed on truncated hashes. + Both platforms now render identical placeholders. -Both fill a caller-owned output buffer (no allocation at the boundary) because the -production callers hold JNI-locked bitmaps. Nothing in the app calls the core yet; -wiring `native_image.c`'s JNI shims to it is the follow-up. +The image ops fill a caller-owned output buffer (no allocation at the boundary) +because the production callers hold JNI-locked bitmaps; the thumbhash decode +returns a libc allocation the consumer frees with plain `free`. ## Layout ``` crates/ - immich_core pure logic, no binding deps. capabilities = cargo features (image). - immich_core_ffi the hand-written C ABI + cbindgen header — consumed by dart (ffigen), - swift (native C interop) and kotlin (JNI shim) + immich_core pure logic, no binding deps. capabilities = cargo features + (image, thumbhash). + immich_core_ffi the hand-written C ABI + cbindgen header — consumed by dart + (ffigen), swift (C interop) and kotlin (JNI shims in src/android/) immich_core_napi cdylib (.node) via napi-rs (server, unwired) immich_native_core/ the Flutter package mobile depends on. build hook + ffigen @Native bindings. smoke/ host dart + node roundtrip scripts (no device) @@ -50,7 +53,7 @@ mise run smoke Rust tests + host dart:ffi + host napi roundtrips ## Add a capability (end to end) 1. add the logic to `crates/immich_core` (behind a cargo feature if it pulls a dep). -2. expose a C entry in `crates/immich_core_ffi/src/lib.rs` — `#[no_mangle] pub extern "C"`, +2. expose a C entry in `crates/immich_core_ffi/src/capi/` — `#[no_mangle] pub extern "C"`, wrap the body in `guard(...)`/`catch_unwind` (panic at the boundary → sentinel, never unwind into the host), validate pointers, and either fill a caller-owned buffer or return Rust-owned memory freed via `immich_core_free_string`. diff --git a/native/crates/immich_core/src/lib.rs b/native/crates/immich_core/src/lib.rs index cfd8fe9960..c9dcc83107 100644 --- a/native/crates/immich_core/src/lib.rs +++ b/native/crates/immich_core/src/lib.rs @@ -1,4 +1,4 @@ -//! immich_native_core — shared Rust core for the immich server (napi) and mobile (dart:ffi). +//! immich_core — shared Rust core for the immich server (napi) and mobile (dart:ffi). //! //! Pure logic only: no binding or platform deps live here. Each binding crate //! (`immich_core_ffi`, `immich_core_napi`) is a thin wrapper. Capabilities are diff --git a/native/crates/immich_core_ffi/Cargo.toml b/native/crates/immich_core_ffi/Cargo.toml index 524d5073a0..9604bc4640 100644 --- a/native/crates/immich_core_ffi/Cargo.toml +++ b/native/crates/immich_core_ffi/Cargo.toml @@ -14,10 +14,15 @@ crate-type = ["lib", "cdylib", "staticlib"] # match the exported symbols regardless of default-feature drift. [dependencies] immich_core = { path = "../immich_core", default-features = false, features = ["image", "thumbhash"] } +# libc everywhere: returned buffers live on the libc heap so every consumer frees +# them with plain free (dart malloc.free / kotlin NativeBuffer.free / swift free). +libc = { workspace = true } tokio = { workspace = true } [target.'cfg(target_os = "android")'.dependencies] jni = { workspace = true } + +[dev-dependencies] libc = { workspace = true } [build-dependencies] diff --git a/native/crates/immich_core_ffi/build.rs b/native/crates/immich_core_ffi/build.rs index b5a8402950..80899f01c2 100644 --- a/native/crates/immich_core_ffi/build.rs +++ b/native/crates/immich_core_ffi/build.rs @@ -8,8 +8,8 @@ fn main() { let out = Path::new(&crate_dir).join("include").join("immich_core.h"); std::fs::create_dir_all(out.parent().unwrap()).ok(); - // Hard-fail, not a warning: the CI drift gate diffs this header, so a silent - // codegen failure would let a stale header sail through green. + // Hard-fail, not a warning: the header is committed and consumed by ffigen + + // Swift, so a silent codegen failure would leave a stale header in the tree. match cbindgen::generate(&crate_dir) { Ok(bindings) => { bindings.write_to_file(&out); diff --git a/native/crates/immich_core_ffi/include/immich_core.h b/native/crates/immich_core_ffi/include/immich_core.h index 2bb102e1f6..2e6fe184bc 100644 --- a/native/crates/immich_core_ffi/include/immich_core.h +++ b/native/crates/immich_core_ffi/include/immich_core.h @@ -35,7 +35,8 @@ bool immich_core_orientation_swaps_dims(int32_t orientation); * bitmap lock + the dst allocation; this only fills dst. * * # Safety - * `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. + * `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, + * and the two ranges must not overlap. */ bool immich_core_rotate_rgba8888(const uint8_t *src, uintptr_t src_len, @@ -54,7 +55,8 @@ bool immich_core_rotate_rgba8888(const uint8_t *src, * bitmap lock + the dst allocation; this only fills dst. * * # Safety - * `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. + * `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, + * and the two ranges must not overlap. */ bool immich_core_rgba1010102_to_rgba8888(const uint8_t *src, uintptr_t src_len, @@ -65,28 +67,13 @@ bool immich_core_rgba1010102_to_rgba8888(const uint8_t *src, uintptr_t dst_len); /** - * Placeholder size for a ThumbHash. Returns false (leaving the out params - * untouched) if the hash is malformed. + * Decode a ThumbHash into a freshly malloc'd RGBA8888 buffer (not premultiplied + * by alpha) and fill `out_info` with {width, height, rowBytes}. The caller owns + * the buffer and releases it with `free`. Returns null on a malformed hash, + * leaving `out_info` untouched. * * # Safety - * `hash` must be valid for reads of `hash_len` bytes; `out_width`/`out_height` - * must be valid for writes. + * `hash` must be valid for reads of `hash_len` bytes and `out_info` for writes + * of three u32 values. */ -bool immich_core_thumbhash_dims(const uint8_t *hash, - uintptr_t hash_len, - uint32_t *out_width, - uint32_t *out_height); - -/** - * Render a ThumbHash as RGBA8888 (not premultiplied) into the caller's - * densely-packed `w*h*4` `dst`, sized via [`immich_core_thumbhash_dims`]. - * Returns false (a safe no-op) on a malformed hash or short buffer. - * - * # Safety - * `hash` must be valid for reads of `hash_len` bytes and `dst` for writes of - * `dst_len`. - */ -bool immich_core_thumbhash_to_rgba(const uint8_t *hash, - uintptr_t hash_len, - uint8_t *dst, - uintptr_t dst_len); +uint8_t *immich_core_thumbhash_decode(const uint8_t *hash, uintptr_t hash_len, uint32_t *out_info); diff --git a/native/crates/immich_core_ffi/src/android/buffer.rs b/native/crates/immich_core_ffi/src/android/buffer.rs index d60aea62ac..175542cd77 100644 --- a/native/crates/immich_core_ffi/src/android/buffer.rs +++ b/native/crates/immich_core_ffi/src/android/buffer.rs @@ -11,7 +11,7 @@ //! the caller falls back, never unwinding into the JVM. The pure allocator calls //! don't touch the env and contain no panicking code, so they run directly. -use jni::objects::{JByteBuffer, JClass, JObject}; +use jni::objects::{JClass, JObject}; use jni::sys::{jint, jlong, jobject}; use jni::{EnvUnowned, Outcome}; @@ -78,37 +78,6 @@ pub extern "system" fn Java_app_alextran_immich_NativeBuffer_wrap<'local>( } } -#[no_mangle] -pub extern "system" fn Java_app_alextran_immich_NativeBuffer_copy<'local>( - mut env: EnvUnowned<'local>, - _class: JClass<'local>, - buffer: JByteBuffer<'local>, - dest_address: jlong, - offset: jint, - length: jint, -) { - // A non-direct buffer makes get_direct_buffer_address return Err, which becomes - // Outcome::Err here: a silent no-op, exactly like the C's NULL check. - crate::log::ensure_panic_hook(); - let _ = env - .with_env(|env| -> jni::errors::Result<()> { - let src = env.get_direct_buffer_address(&buffer)?; - if !src.is_null() { - // SAFETY: `src` is the direct buffer's backing store and `dest` is a - // live libc allocation sized by the caller. - unsafe { - libc::memcpy( - dest_address as usize as *mut libc::c_void, - src.add(offset as usize) as *const libc::c_void, - length as usize, - ); - } - } - Ok(()) - }) - .into_outcome(); -} - #[no_mangle] pub extern "system" fn Java_app_alextran_immich_NativeBuffer_createGlobalRef<'local>( mut env: EnvUnowned<'local>, diff --git a/native/crates/immich_core_ffi/src/android/image.rs b/native/crates/immich_core_ffi/src/android/image.rs index 3393661901..31a1c1c44b 100644 --- a/native/crates/immich_core_ffi/src/android/image.rs +++ b/native/crates/immich_core_ffi/src/android/image.rs @@ -1,8 +1,10 @@ -//! `NativeImage` — the bitmap pixel ops. Locks an Android bitmap, runs the shared -//! `immich_core::image` math into a fresh libc buffer, hands it back to Kotlin. -//! Ports of the former native_image.c. +//! `NativeImage` — the pixel exports Kotlin calls: the bitmap ops ported from the +//! former native_image.c (lock the bitmap, run the shared `immich_core::image` +//! math into a fresh libc buffer, hand it back) and the thumbhash decode. -use jni::objects::{JClass, JIntArray, JObject}; +use std::panic::{catch_unwind, AssertUnwindSafe}; + +use jni::objects::{JByteArray, JClass, JIntArray, JObject}; use jni::sys::{jint, jlong}; use jni::{Env, EnvUnowned, Outcome}; @@ -62,13 +64,16 @@ fn with_bitmap_into_buffer( } let src_len = info.stride as usize * info.height as usize; - let ok = { + // AssertUnwindSafe: catching here keeps the unlock + free below on the panic + // path — otherwise an unwind would leak dst and leave the bitmap locked. + let ok = catch_unwind(AssertUnwindSafe(|| { // SAFETY: the locked bitmap is valid for `stride * height` bytes. let src = unsafe { std::slice::from_raw_parts(src_pixels as *const u8, src_len) }; // SAFETY: dst was allocated with dst_len bytes above and never escaped. let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst, dst_len) }; work(&info, src, dst_slice) - }; + })) + .unwrap_or(false); // SAFETY: paired with the successful lock above. unsafe { AndroidBitmap_unlockPixels(raw_env, raw_bitmap) }; if !ok { @@ -170,3 +175,36 @@ pub extern "system" fn Java_app_alextran_immich_NativeImage_convert1010102<'loca Outcome::Panic(_) => 0, } } + +#[no_mangle] +pub extern "system" fn Java_app_alextran_immich_NativeImage_thumbhash<'local>( + mut env: EnvUnowned<'local>, + _class: JClass<'local>, + hash: JByteArray<'local>, + out_info: JIntArray<'local>, +) -> jlong { + crate::log::ensure_panic_hook(); + let outcome = env + .with_env(|env| -> jni::errors::Result { + let hash = env.convert_byte_array(&hash)?; + let Some((dst, w, h)) = crate::capi::thumbhash::decode_malloc(&hash) else { + return Ok(0); + }; + let dims = [w as i32, h as i32, w as i32 * 4]; + if out_info.set_region(env, 0, &dims).is_err() { + // SAFETY: dst never escaped; free before reporting failure. + unsafe { libc::free(dst as *mut libc::c_void) }; + return Ok(0); + } + Ok(dst as jlong) + }) + .into_outcome(); + match outcome { + Outcome::Ok(ptr) => ptr, + Outcome::Err(e) => { + super::log_error(&format!("thumbhash decode failed: {e}")); + 0 + } + Outcome::Panic(_) => 0, + } +} diff --git a/native/crates/immich_core_ffi/src/android/mod.rs b/native/crates/immich_core_ffi/src/android/mod.rs index 7871a1e16e..4d0fd6e7e3 100644 --- a/native/crates/immich_core_ffi/src/android/mod.rs +++ b/native/crates/immich_core_ffi/src/android/mod.rs @@ -18,6 +18,5 @@ mod buffer; mod image; mod jnigraphics; mod log; -mod thumbhash; pub(crate) use log::log_error; diff --git a/native/crates/immich_core_ffi/src/android/thumbhash.rs b/native/crates/immich_core_ffi/src/android/thumbhash.rs deleted file mode 100644 index f4357649a1..0000000000 --- a/native/crates/immich_core_ffi/src/android/thumbhash.rs +++ /dev/null @@ -1,51 +0,0 @@ -//! `ThumbHash` — decodes a placeholder hash into a fresh libc RGBA buffer, same -//! ownership contract as the `NativeImage` ops (freed via NativeBuffer.free from -//! Kotlin or malloc.free from Dart). Returns 0 on a malformed hash. - -use jni::objects::{JByteArray, JClass, JIntArray}; -use jni::sys::jlong; -use jni::{EnvUnowned, Outcome}; - -#[no_mangle] -pub extern "system" fn Java_app_alextran_immich_images_ThumbHash_nativeDecode<'local>( - mut env: EnvUnowned<'local>, - _class: JClass<'local>, - hash: JByteArray<'local>, - out_info: JIntArray<'local>, -) -> jlong { - crate::log::ensure_panic_hook(); - let outcome = env - .with_env(|env| -> jni::errors::Result { - let hash = env.convert_byte_array(&hash)?; - let Some((w, h)) = immich_core::thumbhash::dims(&hash) else { - return Ok(0); - }; - let dst_len = w as usize * h as usize * 4; - // SAFETY: libc heap — this exact address is later freed via - // NativeBuffer.free (Kotlin) or malloc.free (Dart). - let dst = unsafe { libc::malloc(dst_len) } as *mut u8; - if dst.is_null() { - return Ok(0); - } - // SAFETY: dst was allocated with dst_len bytes above and never escaped. - let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst, dst_len) }; - let dims = [w as i32, h as i32, w as i32 * 4]; - if !immich_core::thumbhash::to_rgba(&hash, dst_slice) - || out_info.set_region(env, 0, &dims).is_err() - { - // SAFETY: dst never escaped; free before reporting failure. - unsafe { libc::free(dst as *mut libc::c_void) }; - return Ok(0); - } - Ok(dst as jlong) - }) - .into_outcome(); - match outcome { - Outcome::Ok(ptr) => ptr, - Outcome::Err(e) => { - super::log_error(&format!("thumbhash decode failed: {e}")); - 0 - } - Outcome::Panic(_) => 0, - } -} diff --git a/native/crates/immich_core_ffi/src/capi/image.rs b/native/crates/immich_core_ffi/src/capi/image.rs index 59e1903ca9..597513147a 100644 --- a/native/crates/immich_core_ffi/src/capi/image.rs +++ b/native/crates/immich_core_ffi/src/capi/image.rs @@ -7,7 +7,8 @@ /// discard it. Nothing outside `src_len`/`dst_len` is touched. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. +/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, +/// and the two ranges must not overlap. pub(super) unsafe fn fill_dst( src: *const u8, src_len: usize, @@ -43,7 +44,8 @@ pub extern "C" fn immich_core_orientation_swaps_dims(orientation: i32) -> bool { /// bitmap lock + the dst allocation; this only fills dst. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. +/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, +/// and the two ranges must not overlap. #[no_mangle] pub unsafe extern "C" fn immich_core_rotate_rgba8888( src: *const u8, @@ -77,7 +79,8 @@ pub unsafe extern "C" fn immich_core_rotate_rgba8888( /// bitmap lock + the dst allocation; this only fills dst. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. +/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, +/// and the two ranges must not overlap. #[no_mangle] pub unsafe extern "C" fn immich_core_rgba1010102_to_rgba8888( src: *const u8, diff --git a/native/crates/immich_core_ffi/src/capi/thumbhash.rs b/native/crates/immich_core_ffi/src/capi/thumbhash.rs index 452f9b5115..d8ab9d0018 100644 --- a/native/crates/immich_core_ffi/src/capi/thumbhash.rs +++ b/native/crates/immich_core_ffi/src/capi/thumbhash.rs @@ -1,59 +1,69 @@ -//! C-ABI thumbhash decode. `dims` first to size the output, then `to_rgba` into -//! the caller's buffer — same caller-owns-dst contract as the image ops. +//! C-ABI thumbhash decode. One call: parse, malloc, fill. Every consumer (dart, +//! swift, and kotlin via the JNI wrapper) wants an allocated RGBA buffer, so +//! unlike the image ops there is no caller-owned dst here — the buffer comes back +//! on the libc heap and the caller releases it with plain `free` (dart: +//! `malloc.free`, kotlin: `NativeBuffer.free`). -/// Placeholder size for a ThumbHash. Returns false (leaving the out params -/// untouched) if the hash is malformed. +use std::panic::{catch_unwind, AssertUnwindSafe}; + +/// Decodes `hash` into a fresh libc allocation. Shared by the C export below and +/// the android JNI wrapper. Returns the buffer with its (width, height), or None +/// on a malformed hash — never leaking the allocation on any failure path. +pub(crate) fn decode_malloc(hash: &[u8]) -> Option<(*mut u8, u32, u32)> { + let (w, h) = immich_core::thumbhash::dims(hash)?; + let len = w as usize * h as usize * 4; + // SAFETY: libc heap — this exact address is later freed by the consumer via + // free()/malloc.free/NativeBuffer.free. + let dst = unsafe { libc::malloc(len) } as *mut u8; + if dst.is_null() { + return None; + } + // SAFETY: dst was allocated with len bytes above and never escaped. + let dst_slice = unsafe { std::slice::from_raw_parts_mut(dst, len) }; + // AssertUnwindSafe: a panic mid-fill only leaves dst partially written, and + // dst is freed right here on that path. + let ok = catch_unwind(AssertUnwindSafe(|| { + immich_core::thumbhash::to_rgba(hash, dst_slice) + })) + .unwrap_or(false); + if !ok { + // SAFETY: dst never escaped. + unsafe { libc::free(dst as *mut libc::c_void) }; + return None; + } + Some((dst, w, h)) +} + +/// Decode a ThumbHash into a freshly malloc'd RGBA8888 buffer (not premultiplied +/// by alpha) and fill `out_info` with {width, height, rowBytes}. The caller owns +/// the buffer and releases it with `free`. Returns null on a malformed hash, +/// leaving `out_info` untouched. /// /// # Safety -/// `hash` must be valid for reads of `hash_len` bytes; `out_width`/`out_height` -/// must be valid for writes. +/// `hash` must be valid for reads of `hash_len` bytes and `out_info` for writes +/// of three u32 values. #[no_mangle] -pub unsafe extern "C" fn immich_core_thumbhash_dims( +pub unsafe extern "C" fn immich_core_thumbhash_decode( hash: *const u8, hash_len: usize, - out_width: *mut u32, - out_height: *mut u32, -) -> bool { + out_info: *mut u32, +) -> *mut u8 { crate::log::ensure_panic_hook(); - if hash.is_null() || out_width.is_null() || out_height.is_null() { - return false; + if hash.is_null() || out_info.is_null() { + return std::ptr::null_mut(); } // SAFETY: caller guarantees `hash` is valid for reads of `hash_len` bytes (see # Safety). let hash_slice = unsafe { std::slice::from_raw_parts(hash, hash_len) }; - let dims = std::panic::catch_unwind(|| immich_core::thumbhash::dims(hash_slice)) - .ok() - .flatten(); - match dims { - Some((w, h)) => { - // SAFETY: caller guarantees the out pointers are valid for writes (see # Safety). + match catch_unwind(|| decode_malloc(hash_slice)) { + Ok(Some((dst, w, h))) => { + // SAFETY: caller guarantees `out_info` is valid for three u32 writes (see # Safety). unsafe { - *out_width = w; - *out_height = h; + *out_info = w; + *out_info.add(1) = h; + *out_info.add(2) = w * 4; } - true + dst } - None => false, - } -} - -/// Render a ThumbHash as RGBA8888 (not premultiplied) into the caller's -/// densely-packed `w*h*4` `dst`, sized via [`immich_core_thumbhash_dims`]. -/// Returns false (a safe no-op) on a malformed hash or short buffer. -/// -/// # Safety -/// `hash` must be valid for reads of `hash_len` bytes and `dst` for writes of -/// `dst_len`. -#[no_mangle] -pub unsafe extern "C" fn immich_core_thumbhash_to_rgba( - hash: *const u8, - hash_len: usize, - dst: *mut u8, - dst_len: usize, -) -> bool { - // SAFETY: pointers/lengths forwarded verbatim to fill_dst (see # Safety). - unsafe { - super::image::fill_dst(hash, hash_len, dst, dst_len, |h, d| { - immich_core::thumbhash::to_rgba(h, d) - }) + _ => std::ptr::null_mut(), } } diff --git a/native/crates/immich_core_ffi/src/lib.rs b/native/crates/immich_core_ffi/src/lib.rs index ebe43f2c15..0a9f45e397 100644 --- a/native/crates/immich_core_ffi/src/lib.rs +++ b/native/crates/immich_core_ffi/src/lib.rs @@ -28,5 +28,5 @@ pub use capi::image::{ immich_core_orientation_swaps_dims, immich_core_rgba1010102_to_rgba8888, immich_core_rotate_rgba8888, }; -pub use capi::thumbhash::{immich_core_thumbhash_dims, immich_core_thumbhash_to_rgba}; +pub use capi::thumbhash::immich_core_thumbhash_decode; pub use capi::{immich_core_free_string, immich_core_version}; diff --git a/native/crates/immich_core_ffi/src/runtime.rs b/native/crates/immich_core_ffi/src/runtime.rs index e8b7a16849..f2013f5cda 100644 --- a/native/crates/immich_core_ffi/src/runtime.rs +++ b/native/crates/immich_core_ffi/src/runtime.rs @@ -1,8 +1,8 @@ //! The shared tokio runtime for every async task the core ever runs. One static //! multi-thread runtime, created lazily on first use and reused for the process //! lifetime — FFI entry points must never build per-call or scoped runtimes. -//! When the first async capability gets an FFI surface, this graduates to the -//! explicit init/shutdown AppRuntime shape (isync / sha4x pattern). +//! When the first async capability gets an FFI surface, this can graduate to an +//! explicit init/shutdown lifecycle. use std::sync::LazyLock; use tokio::runtime::{Builder, Runtime}; diff --git a/native/crates/immich_core_ffi/tests/c_abi.rs b/native/crates/immich_core_ffi/tests/c_abi.rs index ff64ed35c0..75d8b6e88e 100644 --- a/native/crates/immich_core_ffi/tests/c_abi.rs +++ b/native/crates/immich_core_ffi/tests/c_abi.rs @@ -11,8 +11,8 @@ use std::ptr; use immich_core_ffi::{ immich_core_free_string, immich_core_orientation_swaps_dims, - immich_core_rgba1010102_to_rgba8888, immich_core_rotate_rgba8888, immich_core_thumbhash_dims, - immich_core_thumbhash_to_rgba, immich_core_version, + immich_core_rgba1010102_to_rgba8888, immich_core_rotate_rgba8888, immich_core_thumbhash_decode, + immich_core_version, }; // "1QcSHQRnh493V4dIh4eXh1h4kJUI" decoded — the classic 23x32 opaque sample. @@ -185,53 +185,35 @@ fn convert_rejects_bad_input_without_touching_dst() { } #[test] -fn thumbhash_dims_then_decode() { - let (mut w, mut h) = (0u32, 0u32); - let ok = - unsafe { immich_core_thumbhash_dims(THUMBHASH.as_ptr(), THUMBHASH.len(), &mut w, &mut h) }; - assert!(ok); - assert_eq!((w, h), (23, 32)); - - let mut dst = vec![0u8; (w * h * 4) as usize]; - let ok = unsafe { - immich_core_thumbhash_to_rgba( - THUMBHASH.as_ptr(), - THUMBHASH.len(), - dst.as_mut_ptr(), - dst.len(), - ) +fn thumbhash_decodes_into_a_malloc_buffer() { + let mut info = [0u32; 3]; + let ptr = unsafe { + immich_core_thumbhash_decode(THUMBHASH.as_ptr(), THUMBHASH.len(), info.as_mut_ptr()) }; - assert!(ok); + assert!(!ptr.is_null()); + assert_eq!(info, [23, 32, 92]); + + let len = (info[0] * info[1] * 4) as usize; + let pixels = unsafe { std::slice::from_raw_parts(ptr, len) }; // opaque hash: every pixel fully opaque, image not a single flat color - assert!(dst.chunks_exact(4).all(|px| px[3] == 255)); - assert!(dst.chunks_exact(4).any(|px| px[0] != dst[0])); + assert!(pixels.chunks_exact(4).all(|px| px[3] == 255)); + assert!(pixels.chunks_exact(4).any(|px| px[0] != pixels[0])); + unsafe { libc::free(ptr as *mut libc::c_void) }; } #[test] fn thumbhash_rejects_bad_input() { - let (mut w, mut h) = (0u32, 0u32); + let mut info = [7u32; 3]; unsafe { - assert!(!immich_core_thumbhash_dims(ptr::null(), 21, &mut w, &mut h)); - assert!(!immich_core_thumbhash_dims( - THUMBHASH.as_ptr(), - 4, - &mut w, - &mut h - )); - let mut dst = [0u8; 16]; - assert!(!immich_core_thumbhash_to_rgba( - THUMBHASH.as_ptr(), - THUMBHASH.len(), - dst.as_mut_ptr(), - dst.len() - )); - assert!(!immich_core_thumbhash_to_rgba( - ptr::null(), - 21, - dst.as_mut_ptr(), - dst.len() - )); + assert!(immich_core_thumbhash_decode(ptr::null(), 21, info.as_mut_ptr()).is_null()); + assert!(immich_core_thumbhash_decode(THUMBHASH.as_ptr(), 4, info.as_mut_ptr()).is_null()); + assert!( + immich_core_thumbhash_decode(THUMBHASH.as_ptr(), THUMBHASH.len(), ptr::null_mut()) + .is_null() + ); } + // failed decodes never touched the out params + assert_eq!(info, [7, 7, 7]); } #[test] diff --git a/native/immich_native_core/README.md b/native/immich_native_core/README.md index 88afb8fb64..d91a40284e 100644 --- a/native/immich_native_core/README.md +++ b/native/immich_native_core/README.md @@ -14,9 +14,9 @@ dependencies: ``` `dart pub get`. The dart surface is exactly the ffigen output of the C header — -the current capabilities (EXIF rotate, 10-bit convert) are called by the platform -decode pipelines, not by dart, so there are no hand-written dart wrappers. Add one -only when a dart feature actually consumes a function. +the current capabilities (EXIF rotate, 10-bit convert, thumbhash decode) are called +by the platform decode pipelines, not by dart, so there are no hand-written dart +wrappers. Add one only when a dart feature actually consumes a function. No app-level Gradle/Podfile edits. `hook/build.dart` compiles the Rust crate and Flutter bundles it as a code asset; the `@Native` bindings resolve against it. diff --git a/native/immich_native_core/hook/build.dart b/native/immich_native_core/hook/build.dart index 4c956a5f16..2824e17996 100644 --- a/native/immich_native_core/hook/build.dart +++ b/native/immich_native_core/hook/build.dart @@ -13,6 +13,16 @@ void main(List args) async { await RustBuilder( assetName: 'src/ffi/bindings.g.dart', cratePath: '../crates/immich_core_ffi', + // Android requires 16 KB-aligned load segments for Play (apps targeting + // Android 15+); the NDK linker still defaults to 4 KB, so force the page + // size for the .so. iOS/macOS/host use their own alignment and must not + // get this ELF-only flag. Set via env (not .cargo/config.toml) because the + // hook runs cargo from Flutter's cwd, where a crate-local config wouldn't + // be discovered. + extraCargoEnvironmentVariables: { + if (input.config.code.targetOS == OS.android) + 'RUSTFLAGS': '-C link-arg=-Wl,-z,max-page-size=16384', + }, ).run(input: input, output: output); }); } diff --git a/native/immich_native_core/lib/src/ffi/bindings.g.dart b/native/immich_native_core/lib/src/ffi/bindings.g.dart index 5b058114e3..747b73665a 100644 --- a/native/immich_native_core/lib/src/ffi/bindings.g.dart +++ b/native/immich_native_core/lib/src/ffi/bindings.g.dart @@ -31,7 +31,8 @@ external bool immich_core_orientation_swaps_dims(int orientation); /// bitmap lock + the dst allocation; this only fills dst. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. +/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, +/// and the two ranges must not overlap. @ffi.Native< ffi.Bool Function( ffi.Pointer, @@ -62,7 +63,8 @@ external bool immich_core_rotate_rgba8888( /// bitmap lock + the dst allocation; this only fills dst. /// /// # Safety -/// `src` must be valid for reads of `src_len` bytes and `dst` for writes of `dst_len`. +/// `src` must be valid for reads of `src_len` bytes, `dst` for writes of `dst_len`, +/// and the two ranges must not overlap. @ffi.Native< ffi.Bool Function( ffi.Pointer, @@ -84,45 +86,23 @@ external bool immich_core_rgba1010102_to_rgba8888( int dst_len, ); -/// Placeholder size for a ThumbHash. Returns false (leaving the out params -/// untouched) if the hash is malformed. +/// Decode a ThumbHash into a freshly malloc'd RGBA8888 buffer (not premultiplied +/// by alpha) and fill `out_info` with {width, height, rowBytes}. The caller owns +/// the buffer and releases it with `free`. Returns null on a malformed hash, +/// leaving `out_info` untouched. /// /// # Safety -/// `hash` must be valid for reads of `hash_len` bytes; `out_width`/`out_height` -/// must be valid for writes. +/// `hash` must be valid for reads of `hash_len` bytes and `out_info` for writes +/// of three u32 values. @ffi.Native< - ffi.Bool Function( + ffi.Pointer Function( ffi.Pointer, ffi.UintPtr, ffi.Pointer, - ffi.Pointer, ) >() -external bool immich_core_thumbhash_dims( +external ffi.Pointer immich_core_thumbhash_decode( ffi.Pointer hash, int hash_len, - ffi.Pointer out_width, - ffi.Pointer out_height, -); - -/// Render a ThumbHash as RGBA8888 (not premultiplied) into the caller's -/// densely-packed `w*h*4` `dst`, sized via [`immich_core_thumbhash_dims`]. -/// Returns false (a safe no-op) on a malformed hash or short buffer. -/// -/// # Safety -/// `hash` must be valid for reads of `hash_len` bytes and `dst` for writes of -/// `dst_len`. -@ffi.Native< - ffi.Bool Function( - ffi.Pointer, - ffi.UintPtr, - ffi.Pointer, - ffi.UintPtr, - ) ->() -external bool immich_core_thumbhash_to_rgba( - ffi.Pointer hash, - int hash_len, - ffi.Pointer dst, - int dst_len, + ffi.Pointer out_info, ); diff --git a/native/immich_native_core/test/native_core_test.dart b/native/immich_native_core/test/native_core_test.dart index 141076b382..f94cb5ad9e 100644 --- a/native/immich_native_core/test/native_core_test.dart +++ b/native/immich_native_core/test/native_core_test.dart @@ -114,37 +114,29 @@ void main() { expect(badStride, isNull); }); - test('thumbhash decodes via the core: dims then fill', () { + test('thumbhash decodes via the core into a malloc buffer', () { final hash = base64Decode('1QcSHQRnh493V4dIh4eXh1h4kJUI'); final hashPtr = malloc(hash.length); - final w = malloc(); - final h = malloc(); + final info = malloc(3); try { hashPtr.asTypedList(hash.length).setAll(0, hash); - expect(immich_core_thumbhash_dims(hashPtr, hash.length, w, h), isTrue); - expect((w.value, h.value), (23, 32)); + final ptr = immich_core_thumbhash_decode(hashPtr, hash.length, info); + expect(ptr, isNot(equals(nullptr))); + expect((info[0], info[1], info[2]), (23, 32, 23 * 4)); - final len = w.value * h.value * 4; - final dst = malloc(len); - try { - expect( - immich_core_thumbhash_to_rgba(hashPtr, hash.length, dst, len), - isTrue, - ); - final pixels = dst.asTypedList(len); - for (var i = 3; i < len; i += 4) { - expect(pixels[i], 255, reason: 'alpha at $i'); - } - expect(pixels.toSet().length, greaterThan(2)); - } finally { - malloc.free(dst); + final len = info[0] * info[1] * 4; + final pixels = ptr.asTypedList(len); + for (var i = 3; i < len; i += 4) { + expect(pixels[i], 255, reason: 'alpha at $i'); } + expect(pixels.toSet().length, greaterThan(2)); + malloc.free(ptr); - expect(immich_core_thumbhash_dims(hashPtr, 4, w, h), isFalse); + // malformed hash: null return, info untouched + expect(immich_core_thumbhash_decode(hashPtr, 4, info), equals(nullptr)); } finally { malloc.free(hashPtr); - malloc.free(w); - malloc.free(h); + malloc.free(info); } }); } diff --git a/native/mise.toml b/native/mise.toml index 39b4cd81e8..3b95d6e198 100644 --- a/native/mise.toml +++ b/native/mise.toml @@ -22,7 +22,7 @@ run = "cargo clippy --workspace --all-targets -- -D warnings" alias = "codegen" description = "Generate the C header (cbindgen) + Dart @Native bindings (ffigen)" sources = [ - "crates/immich_core_ffi/src/lib.rs", + "crates/immich_core_ffi/src/**/*.rs", "crates/immich_core_ffi/cbindgen.toml", "immich_native_core/ffigen.yaml", ]