From cef5a7cf501133674480ad4ebff5eea616a47270 Mon Sep 17 00:00:00 2001 From: mertalev <101130780+mertalev@users.noreply.github.com> Date: Wed, 21 Jan 2026 02:46:21 -0500 Subject: [PATCH] refactor, fix capacity handling --- .../app/alextran/immich/NativeBuffer.kt | 12 +-- .../alextran/immich/images/LocalImages.g.kt | 4 +- .../alextran/immich/images/LocalImagesImpl.kt | 8 +- .../alextran/immich/images/RemoteImages.g.kt | 4 +- .../immich/images/RemoteImagesImpl.kt | 73 +++++++++---------- mobile/ios/Runner/Images/LocalImages.g.swift | 2 +- .../ios/Runner/Images/LocalImagesImpl.swift | 4 +- mobile/ios/Runner/Images/RemoteImages.g.swift | 2 +- .../loaders/local_image_request.dart | 5 +- .../loaders/remote_image_request.dart | 9 ++- mobile/lib/platform/local_image_api.g.dart | 9 +-- mobile/lib/platform/remote_image_api.g.dart | 9 +-- mobile/pigeon/local_image_api.dart | 2 +- mobile/pigeon/remote_image_api.dart | 2 +- 14 files changed, 68 insertions(+), 77 deletions(-) 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 9afd254709..a9011f3047 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 @@ -30,20 +30,20 @@ class NativeByteBuffer(initialCapacity: Int) { var capacity = initialCapacity var offset = 0 - fun ensureHeadroom(needed: Int = INITIAL_BUFFER_SIZE) { - if (offset + needed > capacity) { - capacity = (capacity * 2).coerceAtLeast(offset + needed) + inline fun ensureHeadroom() { + if (offset == capacity) { + capacity *= 2 pointer = NativeBuffer.realloc(pointer, capacity) } } - fun wrapRemaining() = NativeBuffer.wrap(pointer + offset, capacity - offset) + inline fun wrapRemaining() = NativeBuffer.wrap(pointer + offset, capacity - offset) - fun advance(bytesRead: Int) { + inline fun advance(bytesRead: Int) { offset += bytesRead } - fun free() { + inline fun free() { if (pointer != 0L) { NativeBuffer.free(pointer) pointer = 0L diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImages.g.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImages.g.kt index 1a965ac1b3..5b95daf38b 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImages.g.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImages.g.kt @@ -59,7 +59,7 @@ private open class LocalImagesPigeonCodec : StandardMessageCodec() { /** Generated interface from Pigeon that represents a handler of messages from Flutter. */ interface LocalImageApi { - fun requestImage(assetId: String, requestId: Long, width: Long, height: Long, isVideo: Boolean, callback: (Result>) -> Unit) + fun requestImage(assetId: String, requestId: Long, width: Long, height: Long, isVideo: Boolean, callback: (Result?>) -> Unit) fun cancelRequest(requestId: Long) fun getThumbhash(thumbhash: String, callback: (Result>) -> Unit) @@ -82,7 +82,7 @@ interface LocalImageApi { val widthArg = args[2] as Long val heightArg = args[3] as Long val isVideoArg = args[4] as Boolean - api.requestImage(assetIdArg, requestIdArg, widthArg, heightArg, isVideoArg) { result: Result> -> + api.requestImage(assetIdArg, requestIdArg, widthArg, heightArg, isVideoArg) { result: Result?> -> val error = result.exceptionOrNull() if (error != null) { reply.reply(LocalImagesPigeonUtils.wrapError(error)) 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 02938ef329..50ff11b0c2 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 @@ -27,7 +27,7 @@ import java.util.concurrent.Future data class Request( val taskFuture: Future<*>, val cancellationSignal: CancellationSignal, - val callback: (Result>) -> Unit + val callback: (Result?>) -> Unit ) @RequiresApi(Build.VERSION_CODES.Q) @@ -71,7 +71,7 @@ class LocalImagesImpl(context: Context) : LocalImageApi { private val requestMap = ConcurrentHashMap() companion object { - val CANCELLED = Result.success>(mapOf()) + val CANCELLED = Result.success?>(null) val OPTIONS = BitmapFactory.Options().apply { inPreferredConfig = Bitmap.Config.ARGB_8888 } } @@ -99,7 +99,7 @@ class LocalImagesImpl(context: Context) : LocalImageApi { width: Long, height: Long, isVideo: Boolean, - callback: (Result>) -> Unit + callback: (Result?>) -> Unit ) { val signal = CancellationSignal() val task = threadPool.submit { @@ -138,7 +138,7 @@ class LocalImagesImpl(context: Context) : LocalImageApi { width: Long, height: Long, isVideo: Boolean, - callback: (Result>) -> Unit, + callback: (Result?>) -> Unit, signal: CancellationSignal ) { signal.throwIfCanceled() diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImages.g.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImages.g.kt index d98676ec67..ec60530f86 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImages.g.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImages.g.kt @@ -47,7 +47,7 @@ private open class RemoteImagesPigeonCodec : StandardMessageCodec() { /** Generated interface from Pigeon that represents a handler of messages from Flutter. */ interface RemoteImageApi { - fun requestImage(url: String, headers: Map, requestId: Long, callback: (Result>) -> Unit) + fun requestImage(url: String, headers: Map, requestId: Long, callback: (Result?>) -> Unit) fun cancelRequest(requestId: Long) companion object { @@ -67,7 +67,7 @@ interface RemoteImageApi { val urlArg = args[0] as String val headersArg = args[1] as Map val requestIdArg = args[2] as Long - api.requestImage(urlArg, headersArg, requestIdArg) { result: Result> -> + api.requestImage(urlArg, headersArg, requestIdArg) { result: Result?> -> val error = result.exceptionOrNull() if (error != null) { reply.reply(RemoteImagesPigeonUtils.wrapError(error)) diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt index 30ce95274e..bd461c25b7 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt @@ -46,14 +46,14 @@ class RemoteImagesImpl(context: Context) : RemoteImageApi { } companion object { - val CANCELLED = Result.success>(emptyMap()) + val CANCELLED = Result.success?>(null) } override fun requestImage( url: String, headers: Map, requestId: Long, - callback: (Result>) -> Unit + callback: (Result?>) -> Unit ) { val signal = CancellationSignal() requestMap[requestId] = RemoteRequest(signal) @@ -167,7 +167,7 @@ private class CronetImageFetcher(context: Context, cacheDir: File) : ImageFetche .enableBrotli(true) .setStoragePath(storageDir.absolutePath) .setUserAgent(USER_AGENT) - .enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISK, CACHE_SIZE_BYTES) +// .enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISK, CACHE_SIZE_BYTES) .build() } @@ -190,7 +190,7 @@ private class CronetImageFetcher(context: Context, cacheDir: File) : ImageFetche val requestBuilder = engine.newUrlRequestBuilder(url, callback, executor) headers.forEach { (key, value) -> requestBuilder.addHeader(key, value) } val request = requestBuilder.build() - signal.setOnCancelListener { request.cancel() } + signal.setOnCancelListener(request::cancel) request.start() } @@ -222,6 +222,7 @@ private class CronetImageFetcher(context: Context, cacheDir: File) : ImageFetche private val onFailure: (Exception) -> Unit, private val onComplete: () -> Unit, ) : UrlRequest.Callback() { + private var contentLength: Int = 0 private var buffer: NativeByteBuffer? = null private var httpError: IOException? = null @@ -235,9 +236,9 @@ private class CronetImageFetcher(context: Context, cacheDir: File) : ImageFetche return request.cancel() } - // Pre-size from Content-Length when available, otherwise use reasonable default - val capacity = info.allHeaders["content-length"]?.firstOrNull()?.toIntOrNull() - ?.takeIf { it > 0 } ?: INITIAL_BUFFER_SIZE + contentLength = info.allHeaders["content-length"]?.firstOrNull()?.toIntOrNull() ?: 0 + // Cronet wants the buffer to always have free space, so increment by 1 + val capacity = if (contentLength > 0) contentLength + 1 else INITIAL_BUFFER_SIZE buffer = NativeByteBuffer(capacity) request.read(buffer!!.wrapRemaining()) } @@ -248,7 +249,7 @@ private class CronetImageFetcher(context: Context, cacheDir: File) : ImageFetche byteBuffer: ByteBuffer ) { buffer!!.apply { - advance(byteBuffer.remaining()) + advance(byteBuffer.position()) ensureHeadroom() } request.read(buffer!!.wrapRemaining()) @@ -303,7 +304,7 @@ private class OkHttpImageFetcher private constructor( } .dispatcher(Dispatcher().apply { maxRequestsPerHost = MAX_REQUESTS_PER_HOST }) .connectionPool(connectionPool) - .cache(Cache(File(dir, "thumbnails"), CACHE_SIZE_BYTES)) +// .cache(Cache(File(dir, "thumbnails"), CACHE_SIZE_BYTES)) if (sslSocketFactory != null && trustManager != null) { builder.sslSocketFactory(sslSocketFactory, trustManager) @@ -345,8 +346,7 @@ private class OkHttpImageFetcher private constructor( ) { synchronized(stateLock) { if (draining) { - onFailure(IllegalStateException("Client is draining")) - return + return onFailure(IllegalStateException("Client is draining")) } activeCount++ } @@ -354,7 +354,7 @@ private class OkHttpImageFetcher private constructor( val requestBuilder = Request.Builder().url(url) headers.forEach { (key, value) -> requestBuilder.addHeader(key, value) } val call = client.newCall(requestBuilder.build()) - signal.setOnCancelListener { call.cancel() } + signal.setOnCancelListener(call::cancel) call.enqueue(object : Callback { override fun onFailure(call: Call, e: IOException) { @@ -363,45 +363,38 @@ private class OkHttpImageFetcher private constructor( } override fun onResponse(call: Call, response: Response) { - try { - response.use { - if (call.isCanceled()) { - return onFailure(OperationCanceledException()) - } + response.use { + if (!response.isSuccessful) { + return onFailure(IOException("HTTP ${response.code}: ${response.message}")).also { onComplete() } + } - if (!response.isSuccessful) { - return onFailure(IOException("HTTP ${response.code}: ${response.message}")) - } + val body = response.body + ?: return onFailure(IOException("Empty response body")).also { onComplete() } - val body = response.body ?: return onFailure(IOException("Empty response body")) - - val contentLength = body.contentLength() - val capacity = if (contentLength > 0 && contentLength <= Int.MAX_VALUE) { - contentLength.toInt() - } else { - INITIAL_BUFFER_SIZE - } - val buffer = NativeByteBuffer(capacity) + if (call.isCanceled()) { + onFailure(OperationCanceledException()) + return onComplete() + } + val contentLength = body.contentLength().toInt() + val capacity = if (contentLength > 0) contentLength + 1 else INITIAL_BUFFER_SIZE + val buffer = NativeByteBuffer(capacity) + body.source().use { source -> try { - body.source().use { source -> - while (true) { - signal.throwIfCanceled() - buffer.ensureHeadroom() - val bytesRead = source.read(buffer.wrapRemaining()) - if (bytesRead == -1) break - buffer.advance(bytesRead) - } + while (true) { + if (call.isCanceled()) throw OperationCanceledException() + val bytesRead = source.read(buffer.wrapRemaining()) + if (bytesRead == -1) break + buffer.ensureHeadroom() + buffer.advance(bytesRead) } - onSuccess(buffer) } catch (e: Exception) { buffer.free() onFailure(e) } + onComplete() } - } finally { - onComplete() } } }) diff --git a/mobile/ios/Runner/Images/LocalImages.g.swift b/mobile/ios/Runner/Images/LocalImages.g.swift index fd99f0e019..d417f10222 100644 --- a/mobile/ios/Runner/Images/LocalImages.g.swift +++ b/mobile/ios/Runner/Images/LocalImages.g.swift @@ -70,7 +70,7 @@ class LocalImagesPigeonCodec: FlutterStandardMessageCodec, @unchecked Sendable { /// Generated protocol from Pigeon that represents a handler of messages from Flutter. protocol LocalImageApi { - func requestImage(assetId: String, requestId: Int64, width: Int64, height: Int64, isVideo: Bool, completion: @escaping (Result<[String: Int64], Error>) -> Void) + func requestImage(assetId: String, requestId: Int64, width: Int64, height: Int64, isVideo: Bool, completion: @escaping (Result<[String: Int64]?, Error>) -> Void) func cancelRequest(requestId: Int64) throws func getThumbhash(thumbhash: String, completion: @escaping (Result<[String: Int64], Error>) -> Void) } diff --git a/mobile/ios/Runner/Images/LocalImagesImpl.swift b/mobile/ios/Runner/Images/LocalImagesImpl.swift index 0c2ffeb981..dc189fbbd4 100644 --- a/mobile/ios/Runner/Images/LocalImagesImpl.swift +++ b/mobile/ios/Runner/Images/LocalImagesImpl.swift @@ -44,7 +44,7 @@ class LocalImageApiImpl: LocalImageApi { renderingIntent: .defaultIntent )! private static var requests = [Int64: LocalImageRequest]() - private static let cancelledResult = Result<[String: Int64], any Error>.success([:]) + private static let cancelledResult = Result<[String: Int64]?, any Error>.success(nil) private static let concurrencySemaphore = DispatchSemaphore(value: ProcessInfo.processInfo.activeProcessorCount * 2) private static let assetCache = { let assetCache = NSCache() @@ -62,7 +62,7 @@ class LocalImageApiImpl: LocalImageApi { } } - func requestImage(assetId: String, requestId: Int64, width: Int64, height: Int64, isVideo: Bool, completion: @escaping (Result<[String: Int64], any Error>) -> Void) { + func requestImage(assetId: String, requestId: Int64, width: Int64, height: Int64, isVideo: Bool, completion: @escaping (Result<[String: Int64]?, any Error>) -> Void) { let request = LocalImageRequest(callback: completion) let item = DispatchWorkItem { if request.isCancelled { diff --git a/mobile/ios/Runner/Images/RemoteImages.g.swift b/mobile/ios/Runner/Images/RemoteImages.g.swift index c8af4d8772..e3e76dd58f 100644 --- a/mobile/ios/Runner/Images/RemoteImages.g.swift +++ b/mobile/ios/Runner/Images/RemoteImages.g.swift @@ -70,7 +70,7 @@ class RemoteImagesPigeonCodec: FlutterStandardMessageCodec, @unchecked Sendable /// Generated protocol from Pigeon that represents a handler of messages from Flutter. protocol RemoteImageApi { - func requestImage(url: String, headers: [String: String], requestId: Int64, completion: @escaping (Result<[String: Int64], Error>) -> Void) + func requestImage(url: String, headers: [String: String], requestId: Int64, completion: @escaping (Result<[String: Int64]?, Error>) -> Void) func cancelRequest(requestId: Int64) throws } diff --git a/mobile/lib/infrastructure/loaders/local_image_request.dart b/mobile/lib/infrastructure/loaders/local_image_request.dart index ce8c3debfd..c2e3165aad 100644 --- a/mobile/lib/infrastructure/loaders/local_image_request.dart +++ b/mobile/lib/infrastructure/loaders/local_image_request.dart @@ -16,13 +16,16 @@ class LocalImageRequest extends ImageRequest { return null; } - final Map info = await localImageApi.requestImage( + final info = await localImageApi.requestImage( localId, requestId: requestId, width: width, height: height, isVideo: assetType == AssetType.video, ); + if (info == null) { + return null; + } final frame = await _fromDecodedPlatformImage(info["pointer"]!, info["width"]!, info["height"]!, info["rowBytes"]!); return frame == null ? null : ImageInfo(image: frame.image, scale: scale); diff --git a/mobile/lib/infrastructure/loaders/remote_image_request.dart b/mobile/lib/infrastructure/loaders/remote_image_request.dart index 7840551e50..2da70c3ae1 100644 --- a/mobile/lib/infrastructure/loaders/remote_image_request.dart +++ b/mobile/lib/infrastructure/loaders/remote_image_request.dart @@ -12,8 +12,13 @@ class RemoteImageRequest extends ImageRequest { return null; } - final Map info = await remoteImageApi.requestImage(uri, headers: headers, requestId: requestId); - final frame = await _fromEncodedPlatformImage(info["pointer"]!, info["length"]!); + final info = await remoteImageApi.requestImage(uri, headers: headers, requestId: requestId); + final frame = switch (info) { + {'pointer': int pointer, 'length': int length} => await _fromEncodedPlatformImage(pointer, length), + {'pointer': int pointer, 'width': int width, 'height': int height, 'rowBytes': int rowBytes} => + await _fromDecodedPlatformImage(pointer, width, height, rowBytes), + _ => null, + }; return frame == null ? null : ImageInfo(image: frame.image, scale: scale); } diff --git a/mobile/lib/platform/local_image_api.g.dart b/mobile/lib/platform/local_image_api.g.dart index e91dd87206..8b7c82f15d 100644 --- a/mobile/lib/platform/local_image_api.g.dart +++ b/mobile/lib/platform/local_image_api.g.dart @@ -49,7 +49,7 @@ class LocalImageApi { final String pigeonVar_messageChannelSuffix; - Future> requestImage( + Future?> requestImage( String assetId, { required int requestId, required int width, @@ -79,13 +79,8 @@ class LocalImageApi { message: pigeonVar_replyList[1] as String?, details: pigeonVar_replyList[2], ); - } else if (pigeonVar_replyList[0] == null) { - throw PlatformException( - code: 'null-error', - message: 'Host platform returned null value for non-null return value.', - ); } else { - return (pigeonVar_replyList[0] as Map?)!.cast(); + return (pigeonVar_replyList[0] as Map?)?.cast(); } } diff --git a/mobile/lib/platform/remote_image_api.g.dart b/mobile/lib/platform/remote_image_api.g.dart index f9788ea979..5a36e6773c 100644 --- a/mobile/lib/platform/remote_image_api.g.dart +++ b/mobile/lib/platform/remote_image_api.g.dart @@ -49,7 +49,7 @@ class RemoteImageApi { final String pigeonVar_messageChannelSuffix; - Future> requestImage( + Future?> requestImage( String url, { required Map headers, required int requestId, @@ -71,13 +71,8 @@ class RemoteImageApi { message: pigeonVar_replyList[1] as String?, details: pigeonVar_replyList[2], ); - } else if (pigeonVar_replyList[0] == null) { - throw PlatformException( - code: 'null-error', - message: 'Host platform returned null value for non-null return value.', - ); } else { - return (pigeonVar_replyList[0] as Map?)!.cast(); + return (pigeonVar_replyList[0] as Map?)?.cast(); } } diff --git a/mobile/pigeon/local_image_api.dart b/mobile/pigeon/local_image_api.dart index 5c4ab50cc8..35b6734568 100644 --- a/mobile/pigeon/local_image_api.dart +++ b/mobile/pigeon/local_image_api.dart @@ -15,7 +15,7 @@ import 'package:pigeon/pigeon.dart'; @HostApi() abstract class LocalImageApi { @async - Map requestImage( + Map? requestImage( String assetId, { required int requestId, required int width, diff --git a/mobile/pigeon/remote_image_api.dart b/mobile/pigeon/remote_image_api.dart index 5ff73631f9..0d544f262e 100644 --- a/mobile/pigeon/remote_image_api.dart +++ b/mobile/pigeon/remote_image_api.dart @@ -15,7 +15,7 @@ import 'package:pigeon/pigeon.dart'; @HostApi() abstract class RemoteImageApi { @async - Map requestImage( + Map? requestImage( String url, { required Map headers, required int requestId,