Skip to content

Stream image materialization off disk and strip GPS losslessly - #25807

Open
jkmassel wants to merge 2 commits into
trunkfrom
task/media-v2-streaming-image-transforms
Open

Stream image materialization off disk and strip GPS losslessly#25807
jkmassel wants to merge 2 commits into
trunkfrom
task/media-v2-streaming-image-transforms

Conversation

@jkmassel

Copy link
Copy Markdown
Contributor

Stacked on #25620 (base branch task/media-v2-upload-materializer) — the memory/quality follow-up from that PR's review thread.

Summary

  • finalizeImage no longer pins the whole compressed image in RAM. Disk-backed sources stream through a URL-backed CGImageSourceCGImageDestination; the no-transform case is an APFS clone.
  • "Remove Location" on an in-cap, web-safe photo is now a lossless container rewrite — no decode, no recompress — instead of a full decode + JPEG re-encode.
  • Behavior-preserving: same transforms, same outputs, same validation. All 40 UploadSourceMaterializer tests pass, plus 4 new ones.

Background

The review thread on #25620 flagged (comment on line 551) that we can't just move the picked file — it has to be processed to honor MediaUploadPolicy (GPS strip, compression, resize). That's right. But processing the image does not require loading the whole file into memory. Every API below is back-deployed well under the iOS 17 floor: CGImageSourceCreateWithURL (4.0), CGImageDestinationCopyImageSource (7.0), kCGImageMetadataShouldExcludeGPS (8.0).

Changes

1. Stream off a URL source and destination

finalizeImage(data:) became finalizeImage(input:) over a small ImageInput { case url(URL); case data(Data) }. The disk-backed callers (.file, .imagePlayground, the downloaded .remoteURL temp) pass .url — ImageIO faults the compressed bytes in on demand and the encoder writes straight to CGImageDestinationCreateWithURL, so we never own a full-file Data or an NSMutableData output buffer. The photo-library (NSItemProviderData) and camera (UIImage → JPEG Data) sources have no URL, so they keep .data.

No transform needed → FileManager.copyItem (copy-on-write clone on APFS), zero bytes through RAM, replacing the old data.write(to:) round-trip.

2. Lossless GPS strip

The old .stripGPS-without-.resize branch did a full CGImageSourceCreateImageAtIndex decode and recompressed via kCGImageDestinationLossyCompressionQuality — just to delete a metadata block, silently degrading JPEG quality on every located photo. CGImageDestinationCopyImageSource copies the encoded image verbatim and rewrites only the metadata: no decode, no recompress, and the EXIF orientation tag stays paired with its pixels (the old "decode unrotated pixels, re-attach the orientation tag" hazard disappears).

One correctness wrinkle worth flagging. The obvious kCGImageMetadataShouldExcludeGPS: true on its own is wrong twice over — I verified both against ImageIO before settling on the approach:

  • It drops all EXIF (including DateTimeOriginal), not just GPS. ❌ The policy strips location only, and the resize path is tested to retain other EXIF, so a blanket exclude would be an inconsistent regression.
  • It silently leaves GPS in a PNG. ❌

So the strip copies the source CGImageMetadata, removes the GPS tags, writes it back via kCGImageDestinationMetadata (this keeps the capture date and camera make), and then re-reads the output and confirms the GPS block is actually gone before trusting it. PNG — and any future container that ignores the rewrite — fails that check and falls back to the existing decode strip. Fail-closed: a required strip never silently ships the location.

Format Lossless strip Result
JPEG GPS gone, EXIF kept, pixels byte-identical
HEIC GPS gone, EXIF kept, pixels byte-identical
PNG ❌ (self-detected) falls back to decode strip; GPS gone, stays PNG

3. Transform paths decode from the URL source

Resize (CGImageSourceCreateThumbnailAtIndex) and format conversion (CGImageDestinationAddImageFromSource) still decode — a resample or codec change must touch pixels — but now from a URL-backed source, so ImageIO DCT-scales / tiles instead of materializing the full-resolution bitmap, and writes straight to disk. The format-changing GPS strip (e.g. HEIC→JPEG + Remove Location) is the one strip case that still decodes, and it drops GPS from the carried-over properties as before.

Test plan

  • Modules-PackageWordPressMediaLibraryTests/UploadSourceMaterializer: 40 tests pass on the iOS 26.0 simulator.
  • New: JPEG GPS strip is lossless — decoded output pixels are byte-identical to the source and DateTimeOriginal is retained.
  • New: located HEIC converts to JPEG and strips GPS (format-change decode branch).
  • New: PNG-with-GPS strips via the decode fallback and stays PNG (exercises the fail-closed post-check).
  • New: remote in-cap image is clone-copied byte-for-byte.
  • Existing resize / convert / orientation / passthrough / validation tests unchanged and green.

Related

@dangermattic

dangermattic commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator
1 Error
🚫 PR requires at least one label.
4 Warnings
⚠️ Modules/Package.swift was changed without updating its corresponding Package.resolved.

If the change includes adding, removing, or editing a dependency please resolve the Swift packages as appropriate to your project setup (e.g. in Xcode or by running swift package resolve).

If the change to the Package.swift did not modify dependencies, ignoring this warning should be safe, but we recommend double checking and running the package resolution just in case.
.

⚠️ Package.swift was changed without updating its corresponding Package.resolved.

If the change includes adding, removing, or editing a dependency please resolve the Swift packages as appropriate to your project setup (e.g. in Xcode or by running swift package resolve).

If the change to the Package.swift did not modify dependencies, ignoring this warning should be safe, but we recommend double checking and running the package resolution just in case.
.

⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot

wpmobilebot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
App Icon📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress
ConfigurationRelease-Alpha
Build Number33311
VersionPR #25807
Bundle IDorg.wordpress.alpha
Commitdc911a4
Installation URL3g933bnlmmks0
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot

wpmobilebot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
App Icon📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack
ConfigurationRelease-Alpha
Build Number33311
VersionPR #25807
Bundle IDcom.jetpack.alpha
Commitdc911a4
Installation URL5a07s9o55pvcg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jkmassel
jkmassel force-pushed the task/media-v2-streaming-image-transforms branch 3 times, most recently from 2776520 to 5a85126 Compare July 22, 2026 02:15
@jkmassel
jkmassel requested a review from a team as a code owner July 22, 2026 02:15
@jkmassel
jkmassel force-pushed the task/media-v2-streaming-image-transforms branch from 5a85126 to eef1474 Compare July 22, 2026 02:42
@jkmassel
jkmassel changed the base branch from task/media-v2-upload-materializer to trunk July 22, 2026 02:42
jkmassel added 2 commits July 21, 2026 20:50
MediaTransformer applies the upload policy's transforms — HEIC→JPEG conversion, resize, EXIF-orientation flatten, GPS/location strip, video duration cap and passthrough remux — streaming each result to disk through pure ImageIO/AVFoundation. It depends on no app code, so it lands as a leaf module: `MediaUploadPolicy` moves in from `WordPressMediaLibrary` (nothing there referenced it yet) and `MediaTransformerError` carries its own localized strings, scoped to the failures the engine actually throws.

Nothing consumes it yet — the upload materializer that will comes separately — so `WordPressMediaLibrary` gains no dependency on it. `MediaTransformerTests` exercises the engine directly (47 tests) with UIKit-free CoreGraphics fixtures, registered in `WordPressUnitTests.xctestplan` so iOS CI runs it.
Add a `MediaTransformerTests` entry to the root `WordPressCrossPlatformModules` package so `swift test` builds and runs the transform engine on the macOS host — no Xcode, no simulator, no wordpress-rs.
@jkmassel
jkmassel force-pushed the task/media-v2-streaming-image-transforms branch from eef1474 to dc911a4 Compare July 22, 2026 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants