fix: video export failing when the source file is already an .mp4 - WPB-26457#5006
fix: video export failing when the source file is already an .mp4 - WPB-26457#5006jullianm wants to merge 2 commits into
Conversation
|
@findms could you please test it and share info here whether it fixes your issue or not. |
|
Test Results1 934 tests 1 907 ✅ 2m 27s ⏱️ Results for commit 7889cb6. Summary: workflow run #29103775233 |
There was a problem hiding this comment.
Pull request overview
Fixes a Wire iOS video-conversion failure in AVURLAsset.convertVideoToUploadFormat that occurred when the source video already had an .mp4 extension and lived in the temp directory, causing the conversion output path to collide with (and previously delete) the source file mid-export.
Changes:
- Export conversion output into a uniquely named (UUID) intermediate file, then move it to the final
outputURLafter export completes. - Prevent cleanup from deleting the “source” file when the converted output path collides with the source path.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch let moveError { | ||
| WireLogger.ui.error("Cannot move converted video to \(outputURL): \(moveError)") | ||
| DispatchQueue.main.async { | ||
| completion(nil, self, moveError) | ||
| } |
| // Export into a uniquely named intermediate file rather than directly to `outputURL`. | ||
| // `outputURL`'s name is derived from the source asset's filename with the extension | ||
| // swapped to `.mp4`, so when the source is already an `.mp4` it collides with the | ||
| // source path itself. Deleting that "leftover" file out from under the export session | ||
| // while it's still reading from it caused it to fail. | ||
| let exportingURL = URL(fileURLWithPath: NSTemporaryDirectory()) | ||
| .appendingPathComponent(UUID().uuidString) | ||
| .appendingPathExtension(outputURL.pathExtension) |
@jullianm I have tried building with this branch fix/video-export-fails but it’s same for me..Still after selecting a video file from picker it doesn't show as attached(or compressing ). |
netbe
left a comment
netbe
left a comment
There was a problem hiding this comment.
Looks good, it would be good to add tests for this
Issue
Exporting/converting a video to upload format was failing with
AVFoundationErrorDomain Code=-11800(wrappingNSOSStatusErrorDomain Code=-16979) whenever the source video's temp filename already ended in.mp4.Root cause:
convertVideoToUploadFormat/convert(filename:)derive the converted file's output path from the source's basename with the extension swapped to.mp4. When the source was already.mp4, the computed output path was identical to the source path. The code then deleted any "leftover" file at that path before starting the export — which deleted the source file theAVAssetExportSessionwas still reading from, causing the export to fail.Fixing that first collision exposed a second, related bug: once the export was made to succeed by writing to a distinct intermediate file and moving it into place, the converted file ended up at the same path as the source. The
deleteSourceFilecleanup step inconvertVideoToUploadFormatthen deleted that path, wiping out the just-converted file before it could be uploaded (NSCocoaErrorDomain Code=260/ENOENTdownstream, e.g. inprocessWireDriveVideo).Both are fixed in
AVAsset+VideoConvert.swift:convert(filename:)now exports to a uniquely named (UUID) intermediate file, then moves it to the final output path only after the export session has finished reading from the source.convertVideoToUploadFormatnow skips deleting the source file if its path is the same as the converted output's path (i.e. when they collided).Testing
.mp4extension (e.g.processWireDriveVideoinConversationInputBarViewController+ImagePickerDriveConversation.swift) — before the fix this failed export with-11800/-16979, and after a partial fix failed upload withENOENT. After both fixes, conversion and upload succeed.AVURLAsset_conversionTests.testThatVideoIsConvertedToUploadFormat(Wire-iOS Tests/AVAsset/AVURLAsset+conversionTests.swift) still covers the non-colliding case.Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: