feat(mobile): add support for encoded image requests in local/remote image APIs#26584
Conversation
|
@mertalev another part of the code which I could extract |
There was a problem hiding this comment.
Pull request overview
Adds an “encoded image” request mode to the local/remote image platform APIs and introduces a loadCodec() pathway in Flutter to enable future animated-image rendering (multi-frame codec support).
Changes:
- Extend Pigeon
LocalImageApi/RemoteImageApiwith a requiredencodedboolean flag and propagate it through generated bindings. - Add
ImageRequest.loadCodec()plus shared_codecFromEncodedPlatformImage(...)helper, and implement codec loading for local/remote requests. - Implement native encoded-return paths on iOS (local + remote) and Android (local), while Android remote keeps returning encoded bytes.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| mobile/pigeon/remote_image_api.dart | Adds encoded flag to remote request API definition. |
| mobile/pigeon/local_image_api.dart | Adds encoded flag to local request API definition. |
| mobile/lib/platform/remote_image_api.g.dart | Regenerates Flutter-side channel call to include encoded. |
| mobile/lib/platform/local_image_api.g.dart | Regenerates Flutter-side channel call to include encoded. |
| mobile/lib/presentation/widgets/images/image_provider.dart | Adds loadCodecRequest(...) helper to fetch a ui.Codec. |
| mobile/lib/infrastructure/loaders/image_request.dart | Adds loadCodec() to the abstraction and shared encoded->codec helper. |
| mobile/lib/infrastructure/loaders/remote_image_request.dart | Implements codec loading via encoded bytes for remote images. |
| mobile/lib/infrastructure/loaders/local_image_request.dart | Implements codec loading via encoded bytes for local assets. |
| mobile/lib/infrastructure/loaders/thumbhash_image_request.dart | Adds loadCodec() override (currently throws). |
| mobile/ios/Runner/Images/RemoteImagesImpl.swift | Adds encoded handling for remote requests (raw bytes vs decoded RGBA). |
| mobile/ios/Runner/Images/RemoteImages.g.swift | Regenerates iOS Pigeon protocol/signature to include encoded. |
| mobile/ios/Runner/Images/LocalImagesImpl.swift | Adds encoded handling for local requests (raw bytes vs decoded RGBA). |
| mobile/ios/Runner/Images/LocalImages.g.swift | Regenerates iOS Pigeon protocol/signature to include encoded. |
| mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt | Updates signature to accept encoded (ignored on Android remote). |
| mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImages.g.kt | Regenerates Android Pigeon interface to include encoded. |
| mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt | Adds encoded local-image code path returning pointer/length. |
| mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImages.g.kt | Regenerates Android Pigeon interface to include encoded. |
Comments suppressed due to low confidence (4)
mobile/lib/infrastructure/loaders/thumbhash_image_request.dart:21
ThumbhashImageRequest.loadCodec()throwsUnsupportedError, but theImageRequest.loadCodec()contract is already nullable. Throwing here will surface as an exception if a caller accidentally requests a codec for a thumbhash (e.g., via a shared code path for animated images). Prefer returningnull(or a no-op codec strategy) to keep callers from crashing unexpectedly.
@override
Future<ui.Codec?> loadCodec() => throw UnsupportedError('Thumbhash does not support codec loading');
mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt:164
- The
encodedpath reads the entire original asset into aByteArray(readBytes()), then copies it intoNativeBuffer, and then Dart copies again into anImmutableBuffer. For large images this can cause significant peak memory and jank/OOM. Consider streaming into the native buffer (chunked reads) or using anAssetFileDescriptor/file-channel mapping to avoid multiple full-size copies, and/or enforcing a size limit for this path.
signal.throwIfCanceled()
val bytes = resolver.openInputStream(uri)?.use { it.readBytes() }
?: throw IOException("Could not read image data for $assetId")
signal.throwIfCanceled()
val pointer = NativeBuffer.allocate(bytes.size)
try {
val buffer = NativeBuffer.wrap(pointer, bytes.size)
buffer.put(bytes)
signal.throwIfCanceled()
callback(Result.success(mapOf(
"pointer" to pointer,
"length" to bytes.size.toLong()
)))
mobile/ios/Runner/Images/RemoteImagesImpl.swift:102
- In the
encodedbranch,malloc(length)!can crash whendata.count == 0becausemalloc(0)is allowed to returnnil. Add an explicitlength > 0check (return an error for empty responses) or handle zero-length allocation safely before force-unwrapping.
// Return raw encoded bytes when requested (for animated images)
if encoded {
let length = data.count
let pointer = malloc(length)!
data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length)
if request.isCancelled {
free(pointer)
return request.completion(ImageProcessing.cancelledResult)
}
return request.completion(
.success([
"pointer": Int64(Int(bitPattern: pointer)),
"length": Int64(length),
]))
mobile/ios/Runner/Images/LocalImagesImpl.swift:132
- In the
encodedbranch,malloc(length)!can crash when the asset data is empty (length == 0) becausemalloc(0)may returnnil. Guard against zero-length data (fail the request) or handle zero-length allocation safely before force-unwrapping.
let length = data.count
let pointer = malloc(length)!
data.copyBytes(to: pointer.assumingMemoryBound(to: UInt8.self), count: length)
if request.isCancelled {
free(pointer)
Self.remove(requestId: requestId)
return completion(ImageProcessing.cancelledResult)
}
request.callback(.success([
"pointer": Int64(Int(bitPattern: pointer)),
"length": Int64(length),
]))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed the formatting issue |
Co-authored-by: Mert <101130780+mertalev@users.noreply.github.com>
…larity in image request APIs
…escriptor disposal in loadCodec request
|
@mertalev added the changes, loadCodec now doesn't destroy the Codec and lets GC handle it |
…image APIs (immich-app#26584) * feat(mobile): add support for encoded image requests in local and remote image APIs * fix(mobile): handle memory cleanup for cancelled image requests * refactor(mobile): simplify memory management and response handling for encoded image requests * fix(mobile): correct formatting in cancellation check for image requests * Apply suggestion from @mertalev Co-authored-by: Mert <101130780+mertalev@users.noreply.github.com> * refactor(mobile): rename 'encoded' parameter to 'preferEncoded' for clarity in image request APIs * fix(mobile): ensure proper resource cleanup for cancelled image requests * refactor(mobile): streamline codec handling by removing unnecessary descriptor disposal in loadCodec request --------- Co-authored-by: Mert <101130780+mertalev@users.noreply.github.com>
This PR adds support for loading encoded images. This is also part of #26148
The "Call" pipeline looks like that:
image_provider.dart - loadCodecRequestcallsloadCodedon ImageRequestsloadCodedis implemented byRemoteImageRequestandLocalImageRequestthese call ->_codecFromEncodedPlatformImagewhich is implemented in the baseImageRequest.dartThis PR adds the foundation of supporting animated images, because without the full coded we can't use the
MultiFrameImageStreamCompleter.when #26541 is merged, I then can:
And if this here gets merged we then can based on the playbackStyle decide if we want to show normal images like its currently implemented, or load the codec and show animated images.