-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(ios): corrupted image on ios 13 #501
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the PR and the debugging! Some minor comments from my side.
Besides that, does the change work on lower iOS versions, too?
src/ios/CDVCamera.m
Outdated
@@ -670,17 +670,21 @@ - (void)imagePickerControllerReturnImageResult | |||
{ | |||
CDVPictureOptions* options = self.pickerController.pictureOptions; | |||
CDVPluginResult* result = nil; | |||
|
|||
NSMutableData *dest_data = [NSMutableData data]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use camelCase for variable names. Besides that, can we find a better name than dest_data
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timbru31 i renamed it to imageDataWithExif
. is that fine for you?
Yes i have tested it on ios 12 too. Edit: This was tested successfully on ios 10 also as denoted by this comment: |
@mevinDhun @timbru31 Is there any ETA on merging this PR? |
@mevinDhun I've taken the latest from the fork and tried to test on 13.1.3. The photo doesn't show up |
I've tried on 13.1.3 wkWebview, it didn't work. |
@eastwillie @anubhavberry i have tested it on ios 13.1 and 13.2 and confirm it is working properly. |
@mevinDhun `
}, Here is all the error: 2019-10-31 15:04:39.376208+0400 Cluelez[3788:1203412] getCFDataBytesAtOffset:1209: : *** ERROR *** CGImageSource was created with data size: 50178 - current size is only: 0 |
I have been experience the same problem on iOS 13.2 and the problem seems to be that when we copy the image with metadata (exif data) from the source to destination in imagePickerControllerReturnImageResult method of CDVCamera.m the resulting file is of 0 size and CGImageDestinationFinalize returns false (fails!). If I disable geolocation for camera by setting the below in config.xml and everything works well.
I tried two fixes and both seems to be working:
I removed
and added instead
plus I replaced this line
with this line
Both fixes seems to be working. I'm still wondering why CGImageDestinationAddImageFromSource fails while CGImageDestinationAddImage works fine when saving the image with the metadata. Any ideas? |
@ryaa Thanks, I saw a dialog ask permission for location, but less than 1 second and disappeared. That explained it. |
add correct id to plugin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes in package.json
and plugin.xml
should be reverted. Also the branch will need to be rebased with master as there is conflicting files.
Resolved Conflicts from apache-master
Apache master
Closed by #712 |
Platforms affected
ios 13.0, 13.1
Motivation and Context
When taking an image on ios 13 via camera, the path is returned correctly but the temp image is corrupted.
Here is the issue:
#492
Description
Upon debugging, i found that the image data in
self.data
inCDVCamera.m
is being released/corrupted after setting the metadata viaimagePickerControllerReturnImageResult
The solution is to create a copy of the
NSData
and use that to write the image on disk.Testing
Open camera on ios 13. Take an image. The image returned should be displayed correctly
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)