Add didCaptureMediaURL callback to bypass camera roll - #383

@Rasmussw Thanks for the PR — the didCaptureMediaURL callback is a nice addition for apps that want to bypass the photo library.

One thing I'd like your opinion on, around error handling:

try? FileManager.default.copyItem(at: videoURL, to: destURL)
bypass(destURL)

try? swallows the error but bypass(destURL) still fires regardless. In practice the failure surface is tiny here — the source is a temp file the picker just created, the destination is a UUID-based path under our own temporaryDirectory, so permissions/collisions aren't a concern. The realistic failure mode is basically "disk full," which is rare on iOS.

Still, when it does fail, the caller receives a URL pointing to a file that doesn't exist, with no signal. Two ways to address this — curious which you prefer:

A. Silently drop on failure — keep the API as-is, only invoke the callback when the write succeeded:

if (try? FileManager.default.copyItem(at: videoURL, to: destURL)) != nil {
    bypass(destURL)
}

B. Surface the error — change the signature to pass the error through:

var didCaptureMediaURL: ((Result<URL, Error>) -> Void)?

Given how rare the failure path is, A feels like the right tradeoff to me — preserves the simple API and just enforces the minimum contract that "if you got a URL, the file is there." But if you have a use case where the caller benefits from knowing about the failure, B is reasonable too.

What do you think?

One more thought, on the photo-encoding side — I looked at how the rest of the SDK handles this and there's a small mismatch worth flagging:

let data = image.jpegData(compressionQuality: 0.9)

A few concerns:

1. Original format and metadata are lost. info[.originalImage] is a decoded UIImage, so jpegData(compressionQuality:) re-compresses from a bitmap. EXIF, GPS, and color profile are stripped, and on devices that capture HEIF or ProRAW it's a lossy double-transcode.

2. Inconsistent with the SDK's existing convention. The rest of the SDK preserves original bytes via PHAssetResourceManager.writeData(for:toFile:) (see TLPHAsset.tempCopyMediaFile(...)). The only place a manual JPEG re-encode happens elsewhere is for an opt-in convertLivePhotosToJPG: true flow — and even there, at quality 1.0, not 0.9.

Suggested approach — prefer info[.imageURL] (gives the original captured file as-is, HEIF or JPEG depending on device), fall back to UIImage encoding only when the URL isn't available:

if let originalURL = info[.imageURL] as? URL {
    let destURL = tempDir.appendingPathComponent(UUID().uuidString + "." + originalURL.pathExtension)
    if (try? FileManager.default.copyItem(at: originalURL, to: destURL)) != nil {
        bypass(destURL)
    }
} else if let image = info[.originalImage] as? UIImage,
          let data = image.jpegData(compressionQuality: 1) {
    let destURL = tempDir.appendingPathComponent(UUID().uuidString + ".jpg")
    if (try? data.write(to: destURL)) != nil {
        bypass(destURL)
    }
}

To make info[.imageURL] available, we'd need to set picker.imageExportPreset = .compatible (or .current for HEIF when supported) in the picker setup around line 105, gated on whether didCaptureMediaURL is set so the existing flow isn't affected.

The result is that the bypass output matches what the app would get if it read the file from the photo library — no surprise format changes, no metadata loss. Curious what you think.

@Rasmussw friendly ping — no rush, just wanted to make sure the comments above didn't get lost. Happy to discuss the error-handling and encoding approaches whenever you have time.

@tilltue Sorry for the slow reply! Thanks for the review.

Error handling: Agreed, I think option A is the right call.

Photo encoding: Great catch. info[.imageURL] with imageExportPreset = .compatible is clearly the better approach, metadata is preserved and it stays consistent with the rest of the SDK. I'll implement your suggested pattern and will push a new commit shortly.

@Rasmussw This looks great — both points addressed exactly as discussed. The error handling now correctly invokes the callback only on success, and using info[.imageURL] with imageExportPreset = .compatible preserves the original format and metadata. The gating on didCaptureMediaURL != nil keeps the existing flow completely untouched.

Thanks so much for the contribution and for taking the time to iterate on the review feedback — really appreciate the care you put into this. I'll merge it and include it in the next release.

@tilltue Perfect! when do you plan to make the new release?

@Rasmussw It's out now! 🎉 Just released 2.1.19, which includes your didCaptureMediaURL callback.

• CocoaPods: https://cocoapods.org/pods/TLPhotoPicker — pod 'TLPhotoPicker', '~> 2.1.19'
• SPM: available via the 2.1.19 tag — .upToNextMajor(from: "2.1.19")

Thanks again for the great contribution and for patiently iterating through the review feedback — really appreciate the care you put into the error handling and the info[.imageURL] / imageExportPreset work. 🙏 Enjoy!

Hi @tilltue

Thanks for the quick release of 2.1.19!

Unfortunately I've run into an issue with the timing change from the "fix: avoid duplicate camera picker dismiss" commit. Moving bypass into the picker.dismiss completion handler breaks the use case where the caller needs to dismiss TLPhotosPickerViewController and present a new view controller from within the closure. When it looks like this:

pickerVC?.didCaptureMediaURL = { [weak pickerVC] url in
pickerVC?.dismiss(animated: true) {
controller.present(uploadVC, animated: true)
}
}

With the original timing (bypass called synchronously after picker.dismiss starts), this worked correctly. With 2.1.19, bypass is called from inside a dismiss completion handler, and calling another dismiss from there is unreliable on iOS, the upload view is never presented and the user ends up back on the photo picker screen.

Also worth noting, the withTLPHAssets completion already handles the dismiss internally before calling the callback, so the caller doesn't need to worry about it. It would be consistent if didCaptureMediaURL followed the same pattern, TLPhotoPicker dismisses itself first, then calls the closure, so the caller only needs to present their next viewcontroller.

Could you clarify what the intended usage pattern is when the caller needs to dismiss TLPhotosPickerViewController from within the closure? Or would it be possible to call bypass before picker.dismiss to avoid this issue?

Thanks again!

Best regards,
Rasmus

@Rasmussw I checked the call order and this is a valid regression from the 2.1.19 follow-up fix. The duplicate dismiss fix moved didCaptureMediaURL into the UIImagePickerController.dismiss completion, which changed the callback timing more than necessary.

One detail: the withTLPHAssets closure is currently called before TLPhotosPickerViewController starts dismissing, not after. But the important part is the same: didCaptureMediaURL should not force callers into a new transition timing compared with the original PR behavior.

I opened #385 to restore the original callback timing while still keeping the camera picker dismiss call single. The callback now runs outside the dismiss completion again and still only fires when the temp-file copy succeeds.

I also updated the docs PR (#384) so it no longer says the callback runs after the camera picker dismisses.

@Rasmussw The fix is released in 2.1.20. 🎉

• fix: restore camera media URL callback timing #385 restores the original synchronous callback timing — didCaptureMediaURL is invoked right after the camera picker dismiss starts, not from inside the dismiss completion handler — so your dismiss(animated:) { present(...) } pattern works again. The single-dismiss fix (no duplicate dismiss) and the success-only callback behavior are both preserved.
• docs: document camera media URL callback #384 updates the docs accordingly (it no longer says the callback runs after the camera picker dismisses).

Available now:

• CocoaPods: pod 'TLPhotoPicker', '~> 2.1.20'
• SPM: 2.1.20 tag

Thank you so much for the detailed regression report — the exact reproduction case (dismiss + present from within the closure) made it really easy to pinpoint and confirm the fix. Genuinely appreciate the careful follow-through on this. 🙏

@tilltue Perfect, thanks for the quick reply and the fix! Much appreciated.

Hi @tilltue

I've been testing further and found that there are still two issues with the current 2.1.20 implementation.

Bypass is called while the UIImagePickerController dismiss animation is still ongoing, so calling dismiss on TLPhotosPickerViewController from within the closure is ignored by iOS.

TLPhotosPickerViewController doesn't dismiss itself, leaving the caller responsible, which is inconsistent with how withTLPHAssets already works.

I've implemented a fix in my fork that addresses both: https://github.com/Rasmussw/TLPhotoPicker
The changes are:

TLCameraService: copy the file first, then call bypass in the picker.dismiss completion so the camera picker is fully dismissed before the callback fires.

TLPhotosPickerViewController: wrap didCaptureMediaURL to self-dismiss before invoking the caller's closure.
With this, the caller only needs to present their next view controller, no dismiss logic needed, exactly like withTLPHAssets.

I've tested it and it works correctly. Would you be open to merging these changes?

Best regards,
Rasmus

Hi @Rasmussw — thanks for digging in further and for the fork. I went through the call ordering again and I agree with both points:

1. In 2.1.20 bypass fires synchronously while the camera picker's dismiss animation is still running, so a dismiss on TLPhotosPickerViewController from inside the closure gets swallowed by UIKit.
2. The picker not dismissing itself leaves that on the caller, which is the awkward part.

Let's move the didCaptureMediaURL path to a self-dismiss model: the library dismisses the camera picker, then dismisses TLPhotosPickerViewController, then calls your closure — so the caller only has to present their next screen, never dismiss.

One correction on the rationale, just so the contract is documented accurately: withTLPHAssets actually invokes its completion before TLPhotosPickerViewController starts dismissing, not after (see dismiss(done:) — completionWithTLPHAssets?(...) runs, then self.dismiss(...)). So the ordering isn't identical. What we're matching is the useful part — "the library owns the dismiss, the caller doesn't." For didCaptureMediaURL the natural order is dismiss-then-callback.

Two structural notes before this goes in — could you fold these into the new PR?

1. Keep the public property a plain passthrough; don't wrap the closure in the setter.

The setter-wrapping in the fork makes didCaptureMediaURL's get/set asymmetric (the getter returns the wrapped closure, not what the caller assigned) and hides the dismiss behind a property assignment. Since TLCameraService already holds the host picker (presentingViewController, which it already force-casts to TLPhotosPickerViewController) and already dismisses the camera picker, the cleanest place for the host dismiss is the service's bypass path. So leave the VC property exactly as it is on master:

public var didCaptureMediaURL: ((URL) -> Void)? {
    get { cameraService.didCaptureMediaURL }
    set { cameraService.didCaptureMediaURL = newValue }
}

2. Single branch point + self-contained paths in the delegate method.

Split imagePickerController(_:didFinishPickingMediaWithInfo:) so the top-level if is the only branch, and each path owns its own picker.dismiss. Duplicating the dismiss across the two is fine — clarity over DRY. This is the part that kept tripping us up (which path dismisses, and when), so making it explicit should stop the churn:

func imagePickerController(_ picker: UIImagePickerController,
                           didFinishPickingMediaWithInfo info: [UIImagePickerController.InfoKey: Any]) {
    if let bypass = didCaptureMediaURL {
        bypassCapturedMedia(picker: picker, info: info, bypass: bypass)
    } else {
        saveCapturedMediaToLibrary(picker: picker, info: info)
    }
}

// Callback registered: copy to a temp file, dismiss the camera picker, then the
// host picker, then deliver the URL. Photo library is never touched.
private func bypassCapturedMedia(picker: UIImagePickerController,
                                 info: [UIImagePickerController.InfoKey: Any],
                                 bypass: @escaping (URL) -> Void) {
    let tempDir = FileManager.default.temporaryDirectory
    var capturedURL: URL?
    if let videoURL = info[.mediaURL] as? URL {
        let destURL = tempDir.appendingPathComponent(UUID().uuidString + ".mov")
        do { try FileManager.default.copyItem(at: videoURL, to: destURL); capturedURL = destURL } catch {}
    } else if let imageURL = info[.imageURL] as? URL {
        let ext = imageURL.pathExtension.isEmpty ? "jpg" : imageURL.pathExtension
        let destURL = tempDir.appendingPathComponent(UUID().uuidString + "." + ext)
        do { try FileManager.default.copyItem(at: imageURL, to: destURL); capturedURL = destURL } catch {}
    } else if let image = info[.originalImage] as? UIImage,
              let data = image.jpegData(compressionQuality: 1.0) {
        let destURL = tempDir.appendingPathComponent(UUID().uuidString + ".jpg")
        do { try data.write(to: destURL); capturedURL = destURL } catch {}
    }
    guard let capturedURL else {
        picker.dismiss(animated: true, completion: nil)
        return
    }
    picker.dismiss(animated: true) { [weak self] in
        self?.presentingViewController?.dismiss(animated: true) {
            bypass(capturedURL)
        }
    }
}

// Callback not registered: existing behavior — save to the photo library.
private func saveCapturedMediaToLibrary(picker: UIImagePickerController,
                                        info: [UIImagePickerController.InfoKey: Any]) {
    picker.dismiss(animated: true, completion: nil)
    if let image = info[.originalImage] as? UIImage {
        saveCapturedAsset(image: image)
    } else if let mediaType = info[.mediaType] as? String {
        let isMovieType: Bool
        if #available(iOS 14.0, *) {
            isMovieType = mediaType == UTType.movie.identifier
        } else {
            isMovieType = mediaType == "public.movie"
        }
        if isMovieType, let videoURL = info[.mediaURL] as? URL {
            saveCapturedAsset(videoURL: videoURL)
        }
    }
}

The info[.originalImage] → jpegData(1.0) fallback you re-added is good — just keep it as the last fallback after .imageURL (as above) so we don't re-encode when the original file is available.

Backward-compat: this only touches the registered-callback path; with no callback set, behavior stays exactly as before (save to library, single dismiss), so please keep that fully gated.

Since #383 is already merged, could you open this as a new PR against master? Please also update the docs (the #384 follow-up) to say the caller should only present from the closure and no longer dismiss. I'll cut a release once it's in. Thanks again for the careful follow-through on this!

Hi @tilltue

Thanks for the help! I have made the the new PR here: #386