fix(image): decode+downsample in one step on iOS/Android to avoid OOM on large photos#415
Open
ceres-quangd wants to merge 1 commit into
Open
Conversation
… on large photos Both manualCompressHandler and autoCompressHandler on iOS decode the source image at its full native resolution (UIImage(contentsOfFile:) /UIImage(data:)) before scaleAndRotateImage() redraws it into a second full-resolution CGContext for orientation, and only THEN resize. For a 12MP HEIC photo that's ~2x a ~49MB RGBA buffer in flight per image, before any downsampling happens. Apps that compress several images concurrently (e.g. a small upload queue) can hit iOS's jetsam limit on lower-RAM devices (reproduced crashing on iPhone 11/13, 4GB RAM) purely from this decode step — before a single byte of the resize/compress logic runs. Fix: decode via CGImageSourceCreateThumbnailAtIndex with kCGImageSourceThumbnailMaxPixelSize + kCGImageSourceCreateThumbnailWithTransform, which downsamples AND applies EXIF orientation in a single native decode, never materializing the full-resolution bitmap. Native pixel size is read from metadata first (no decode) to clamp the target size and avoid upscaling. manualCompressHandler/autoCompressHandler now share this decode step; loadImage/scaleAndRotateImage/manualResize are left in place as a safety-net path for the (currently unused) maxWidth != maxHeight case. Also fixes a latent bug in copyExifInfo(): it copies the ORIGINAL file's EXIF Orientation tag onto the newly-encoded (already-upright) image whenever the destination doesn't already have that tag, which un-rotates images a second time in any viewer that respects EXIF orientation. This was masked before because nothing exercised manualCompressHandler with a rotated source through this exact path in our testing, but it's real regardless of the decode change above. Android: manualCompressImage() had the same full-decode-before-resize issue that autoCompressImage() already avoids via BitmapFactory's inSampleSize downsampling — reuse that same technique. Also removed manualCompressImage()'s fallback to the original (untouched) file when the compressed output isn't smaller, since callers using 'manual' specifically to force a format conversion (e.g. HEIC/HEIF -> JPEG) need a guarantee that the returned file is actually in the requested format, regardless of resulting size. autoCompressImage() keeps that size-comparison fallback as before. Verified by patching react-native-compressor@2.0.2 in a downstream app via patch-package: compiles cleanly on both platforms (xcodebuild -scheme react-native-compressor, and ./gradlew :react-native-compressor:compileDebugKotlin), and resolves the OOM crash in that app's own upload flow.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #414.
What
ImageCompressor.swift(iOS) andImageCompressor.kt(Android) decode the source image at full native resolution before resizing, which can OOM-crash apps on lower-RAM devices when compressing several photos concurrently (reproduced on iPhone 11/13, 4GB RAM). Full writeup with root cause in #414.iOS changes
makeImageSource,decodeDownsampledImage,fitExactBoxIfNeeded,decodeAndPrepareImage— decode viaCGImageSourceCreateThumbnailAtIndexwithkCGImageSourceThumbnailMaxPixelSize+kCGImageSourceCreateThumbnailWithTransform: true, so downsampling and EXIF orientation correction happen in a single native decode step instead ofUIImage(contentsOfFile:)→ full-resolution redraw inscaleAndRotateImage()→ resize.manualCompressHandlerandautoCompressHandlernow sharedecodeAndPrepareImage.loadImage,scaleAndRotateImage,manualResize,findTargetSizeare left in place —manualResizeis still used as a safety net byfitExactBoxIfNeededfor the (currently unused)maxWidth != maxHeightcase, so this is intentionally additive rather than a rewrite.copyExifInfo(): it copies the original file's EXIFOrientationtag onto the newly-encoded image whenever the destination doesn't already have that tag. Since the pixel data has already been rotated to.upby the decode step, leaving the original tag in place causes a second, incorrect rotation in any viewer that respects EXIF orientation. Fixed by forcingdataMetadata?[kCGImagePropertyOrientation] = 1after the merge loop. This is independent of the OOM fix but became reachable once I started exercisingmanualCompressHandler/autoCompressHandlerwith real rotated photos while testing the fix above, so I'm including it in the same PR.Android changes
loadImageDownsampled, reusing theinSampleSizetechniqueautoCompressImage()already uses (bounds-only decode pass, then a real decode withinSampleSizeset) —manualCompressImage()now uses it instead of the non-downsamplingloadImage().manualCompressImage()no longer falls back to the original untouched file when the compressed output isn't smaller (that behavior is unchanged inautoCompressImage()). A caller using'manual'specifically to force a format conversion (e.g. HEIC/HEIF → JPEG, since HEVC compresses better than JPEG so the JPEG re-encode is very often larger in bytes) needs a guarantee that the returned file is actually in the requested output format — not silently the original file/format.Testing
xcodebuild -workspace <app>.xcworkspace -scheme react-native-compressor -sdk iphonesimulator build→ succeeds../gradlew :react-native-compressor:compileDebugKotlin→ succeeds (same pre-existing nullability/deprecation warnings only, no new ones).patch-package: the OOM crash no longer reproduces when compressing a batch of HEIC photos concurrently on an iPhone 11 (4GB RAM), and rotation is correct for photos captured in portrait/landscape/upside-down orientations.Happy to split the
copyExifInfofix into a separate PR if you'd prefer to keep this one scoped to just the OOM fix — let me know.