[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/185#issuecomment-191317115 Made some changes to comply with your comments --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on a diff in the pull request: https://github.com/apache/cordova-plugin-camera/pull/185#discussion_r54747682 --- Diff: src/ios/CDVCamera.m --- @@ -402,6 +444,115 @@ - (NSString*)tempFilePath:(NSString*)extension return filePath; } +// This is not absolute, but more mightNeedOrientationCorrection +- (BOOL) needsOrientationCorrection:(UIImage*)image options:(CDVPictureOptions*)options --- End diff -- Agreed. The only reason this is a function is because I wanted to check: - if the image really needed an orientation correction (but that is an over optimization), - if we really wanted to correct gallery image orientation. Anyway, will be inlined. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on a diff in the pull request: https://github.com/apache/cordova-plugin-camera/pull/185#discussion_r54746664 --- Diff: src/ios/CDVCamera.m --- @@ -59,6 +59,59 @@ Licensed to the Apache Software Foundation (ASF) under one @implementation CDVPictureOptions +/* + API: + Camera: + - CameraPopoverHandle: --- End diff -- This was meant as a quick way to check the API (and most of all the options) while developing. I can get rid of it (but it can be a good way to know if we're in sync with the docs). --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on a diff in the pull request: https://github.com/apache/cordova-plugin-camera/pull/185#discussion_r54746727 --- Diff: src/ios/CDVCamera.m --- @@ -59,6 +59,59 @@ Licensed to the Apache Software Foundation (ASF) under one @implementation CDVPictureOptions +/* + API: + Camera: + - CameraPopoverHandle: + - setPosition(popoverOptions) => () + - getPicture(success, failure, options) => CameraPopoverHandle + - options: + - quality (number [0-100]) + - destinationType (number): + 0: DATA_URL + 1: FILE_URI + 2: NATIVE_URI + - sourceType (number): + 0: PHOTOLIBRARY + 1: CAMERA + 2: SAVEDPHOTOALBUM + - allowEdit (bool) + - encodingType (number): + 0: JPEG + 1: PNG + - targetWidth/targetHeight (number) + - mediaType (number): + 0: PICTURE + 1: VIDEO + 2: ALLMEDIA + - correctOrientation (bool) + - saveToPhotoAlbum (bool) + - cameraDirection (number) + 0: BACK + 1: FRONT + - popoverOptions (dictionnary) [iPad only]: + - x (number) + - y (number) + - width (number) + - height (number) + - arrowDir (number): + 1: ARROW_UP + 2: ARROW_DOWN + 4: ARROW_LEFT + 8: ARROW_RIGHT + 15: ARROW_ANY + - cleanup => () + */ + +/* + TODO: --- End diff -- Sorry, this is a leftover comment, this TODOs are already implemented. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/185#discussion_r54746333
--- Diff: src/ios/CDVCamera.h ---
@@ -87,19 +91,51 @@ typedef NSUInteger CDVMediaType;
{}
@property (strong) CDVCameraPicker* pickerController;
-@property (strong) NSMutableDictionary *metadata;
+@property (strong) NSDictionary* metadata;
@property (strong, nonatomic) CLLocationManager *locationManager;
-@property (strong) NSData* data;
+@property (strong) UIImage* image;
/*
- * getPicture
+ * takePicture
*
* arguments:
- * 1: this is the javascript function that will be called with the
results, the first parameter passed to the
- * javascript function is the picture as a Base64 encoded string
- * 2: this is the javascript function to be called if there was an error
+ * 1: success callback (called with either Base64, file URI or native URI)
--- End diff --
I'll remove that
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/185#discussion_r54745802
--- Diff: src/ios/CDVCamera.m ---
@@ -402,6 +444,115 @@ - (NSString*)tempFilePath:(NSString*)extension
return filePath;
}
+// This is not absolute, but more mightNeedOrientationCorrection
+- (BOOL) needsOrientationCorrection:(UIImage*)image
options:(CDVPictureOptions*)options
+{
+// TODO use image to detect if it needs an orientation correction
+
+// See FIXME #2
+//if (options.sourceType !=
UIImagePickerControllerSourceTypeCamera) {
+//return false;
+//}
+return options.correctOrientation;
+}
+
+- (BOOL) needsResize:(CDVPictureOptions*)options
+{
+return (options.targetSize.height > 0 && options.targetSize.width > 0);
+}
+
+- (BOOL) needsEdit:(UIImage*)image options:(CDVPictureOptions*)options
+{
+return [self needsOrientationCorrection:image options:options] ||
[self needsResize:options];
+}
+
+- (BOOL) needsSavingToPhotoAlbum:(UIImage*)image
options:(CDVPictureOptions*)options
+{
+/*
+ We save to the photo album if:
+ - the option is set
+ - the image is fetch from the camera OR the image has been edited (no
need to duplicate image in the library)
+ */
+BOOL isSourceCamera = options.sourceType ==
UIImagePickerControllerSourceTypeCamera;
+BOOL saveToPhotoAlbum = options.saveToPhotoAlbum && ([self
needsEdit:image options:options] || isSourceCamera);
+
+return saveToPhotoAlbum;
+}
+
+/*
+ Metadata is not needed for:
+ - source: gallery
+ - destination: NATIVE_URI
+ - no edit (not orientation and not resize and not allowEdits)
+
+ otherwise, it can be found:
+ - source: camera
+ => UIImagePickerControllerMediaMetadata
+ - source: gallery
+ => CGImage thingy
--- End diff --
Sorry, forgot to remove those comments. They were only meant to remember
how to access metadata in different situation (and might be totally outdated).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
Github user dieppe commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/185#discussion_r54745167
--- Diff: src/ios/CDVCamera.m ---
@@ -402,6 +444,115 @@ - (NSString*)tempFilePath:(NSString*)extension
return filePath;
}
+// This is not absolute, but more mightNeedOrientationCorrection
+- (BOOL) needsOrientationCorrection:(UIImage*)image
options:(CDVPictureOptions*)options
+{
+// TODO use image to detect if it needs an orientation correction
+
+// See FIXME #2
+//if (options.sourceType !=
UIImagePickerControllerSourceTypeCamera) {
+//return false;
+//}
+return options.correctOrientation;
+}
+
+- (BOOL) needsResize:(CDVPictureOptions*)options
+{
+return (options.targetSize.height > 0 && options.targetSize.width > 0);
+}
+
+- (BOOL) needsEdit:(UIImage*)image options:(CDVPictureOptions*)options
+{
+return [self needsOrientationCorrection:image options:options] ||
[self needsResize:options];
+}
+
+- (BOOL) needsSavingToPhotoAlbum:(UIImage*)image
options:(CDVPictureOptions*)options
+{
+/*
+ We save to the photo album if:
+ - the option is set
+ - the image is fetch from the camera OR the image has been edited (no
need to duplicate image in the library)
+ */
+BOOL isSourceCamera = options.sourceType ==
UIImagePickerControllerSourceTypeCamera;
+BOOL saveToPhotoAlbum = options.saveToPhotoAlbum && ([self
needsEdit:image options:options] || isSourceCamera);
+
+return saveToPhotoAlbum;
+}
+
+/*
+ Metadata is not needed for:
+ - source: gallery
+ - destination: NATIVE_URI
+ - no edit (not orientation and not resize and not allowEdits)
--- End diff --
Afaict, quality is not stored in metadata
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-190836981 @omefire I did close this one :wink: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: [iOS] Major refactor
GitHub user dieppe opened a pull request: https://github.com/apache/cordova-plugin-camera/pull/185 [iOS] Major refactor This is a rebased version of the previous pull-request https://github.com/apache/cordova-plugin-camera/pull/101 You can merge this pull request into a Git repository by running: $ git pull https://github.com/jnuine/cordova-plugin-camera ios-refactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-plugin-camera/pull/185.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #185 commit 5124898c237e5db78128d2b97ef74587b2690dd7 Author: Clément Vollet <clem...@jnuine.com> Date: 2015-06-05T16:46:49Z Major refactor The major thing added here is that metadata is correctly handled in most (all?) cases. I also think some bugs have been fixed, but I did not check the bug list ;) As a foreword, I want to say that I am far from being an expert in either Objective-C nor the iOS APIs (I only did some basic patches before). So please correct me if I say/did anything wrong. As a general design decision, I tried to break all the workflow in small readable methods that either return something (I chose to use reference passing when returning more than one result which I find clearer than returning an NSDictionary where the result is specified nowhere), or use the convention completionBlock / resultBlock / failureBlock which it seems Apple APIs use. There are three main paths from the image retrieval to the result. 1. The user wants the original image from the PhotoLibrary or from the SavedPhotoAlbum with a destination type NATIVE_URI. 2. The user wants a modified version of the image, wherever it comes from. 3. The user wants the geolocation. There are also some combinations of options that are disallowed because they donât make any sense: 1. if the user wants an image with a source type CAMERA, and a destination type NATIVE_URI, he has to save it to the photo album. An image that is not there does not have a NATIVE_URI. 2. if the user wants an image with a source type PHOTOLIBRARY or SAVEDPHOTOALBUM, needs editing (resize, orientation correction), and a destination type NATIVE_URI, he has to save it to the photo album. Otherwise, we would return the original image with no modification. As we can see, if we want a destination type of NATIVE_URI, there are only two possible paths: either we want an unmodified version of a library image, or we need to save to the photo album. Regarding metadata, we do not need it in only one case, when the destination type is NATIVE_URI, the source is the library, and no edits have to be done. Otherwise, we need to either fetch if from the UIImagePickerControllerMediaMetadata key of the info dictionary provided by UIImagePickerController in case the source type is CAMERA, or from the ALAsset corresponding to the library image (see https://developer.apple.com/library/ios/documentation/UIKit/Reference/UI ImagePickerControllerDelegate_Protocol/index.html#//apple_ref/doc/consta nt_group/Editing_Information_Keys). This actually introduces asynchronicity in the metadata retrieval, since we first need to fetch the ALAsset using UIImagePickerControllerReferenceURL. *We cannot use UIImage as it strips out most (all?) of the metadata*. Once we actually retrieved the metadata we need to modify it for every transformation of the image (orientation, resize, gps). In the previous code, the saving was done after computing the CDVResult. This is not the case anymore as we need access to the saved ALAsset when we want to return a NATIVE_URI. The saving is now done after all image editing has occurred. Since the metadata retrieval and the album saving is done asynchronously, the process pipeline itself is made asynchronous. We can use resultBlock and failureBlock to get the processed image with its corresponding metadata or to get the image Asset NATIVE_URI. CropToSize is not a valid option (it is always set to NO anyway) so it has been removed. The documentation lacks some clarity for two things (see FIXME #1 & #2): - Does the quality option apply to the library images as well. This would mean that we cannot use NATIVE_URI with the quality set, except if we save to the photo album. - Does the orientation correction apply to the library images as well. Same thing. I actually removed that options at first, but this would mean that we need to read the EXIF data in the js code and orient the image appropriately (except for NATIVE_URIs since the web view does that aut
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe closed the pull request at: https://github.com/apache/cordova-plugin-camera/pull/101 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Fix missing CAMERA permission ...
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/142#issuecomment-188872652 You can also check this fork https://github.com/jnuine/cordova-plugin-camera It includes a major refactor of the iOS plugin which fixes some bugs, checks for option consistency and allows access to the picture metadata in all cases that make sense (available as a [PR](https://github.com/apache/cordova-plugin-camera/pull/101) since June last year, but I guess merging PR is not cordova's team strong suit). On Android, it includes these commits and also handles better the kitkat+ gallery (sadly not the images from Google Drive). We use it in an app used daily both on Android and iOS. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-155457722 Very disappointed to see that our efforts have been completely overlooked. This PR actually fixes a bunch of bugs that have since been fixed, and others that may not have. The point of doing a major refactor was to simplify the plugin and make more apparent the role of each option on the image processing. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-115352285 @shazron Any updates on this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
Github user dieppe commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/101#issuecomment-109964558 The most probable is that it is a side effect, since I did not touch the implementation of `imageByScalingNotCroppingForSize` one bit :wink: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: Major refactor
GitHub user dieppe opened a pull request: https://github.com/apache/cordova-plugin-camera/pull/101 Major refactor Hi there, We needed access to the metadata which lead to asynchronous operations. We took the opportunity to refactor most of the plugin logic. Hopefully it's more readable and actually fixes some known bugs (e.g. we deal properly with NATIVE_URI wherever possible). This is a call for feedback and testing while I try to fix a known bug. The commit message is quite explicit, so here it is: --- The major thing added here is that metadata is correctly handled in most (all?) cases. I also think some bugs have been fixed, but I did not check the bug list ;) As a foreword, I want to say that I am far from being an expert in either Objective-C nor the iOS APIs (I only did some basic patches before). So please correct me if I say/did anything wrong. # Global flow As a general design decision, I tried to break all the workflow in small readable methods that either return something (I chose to use reference passing when returning more than one result which I find clearer than returning an NSDictionary where the result is specified nowhere), or use the convention completionBlock / resultBlock / failureBlock which it seems Apple APIs use. There are three main paths from the image retrieval to the result. 1. The user wants the original image from the PhotoLibrary or from the SavedPhotoAlbum with a destination type NATIVE_URI. 2. The user wants a modified version of the image, wherever it comes from. 3. The user wants the geolocation. # Option incompatibilities There are also some combinations of options that are disallowed because they donât make any sense: 1. if the user wants an image with a source type CAMERA, and a destination type NATIVE_URI, he has to save it to the photo album. An image that is not there does not have a NATIVE_URI. 2. if the user wants an image with a source type PHOTOLIBRARY or SAVEDPHOTOALBUM, needs editing (resize, orientation correction), and a destination type NATIVE_URI, he has to save it to the photo album. Otherwise, we would return the original image with no modification. As we can see, if we want a destination type of NATIVE_URI, there are only two possible paths: either we want an unmodified version of a library image, or we need to save to the photo album. # Metadata Regarding metadata, we do not need it in only one case, when the destination type is NATIVE_URI, the source is the library, and no edits have to be done. Otherwise, we need to either fetch if from the UIImagePickerControllerMediaMetadata key of the info dictionary provided by UIImagePickerController in case the source type is CAMERA, or from the ALAsset corresponding to the library image (see https://developer.apple.com/library/ios/documentation/UIKit/Reference/UIImagePickerControllerDelegate_Protocol/index.html#//apple_ref/doc/constant_group/Editing_Information_Keys). This actually introduces asynchronicity in the metadata retrieval, since we first need to fetch the ALAsset using UIImagePickerControllerReferenceURL. *We cannot use UIImage as it strips out most (all?) of the metadata*. Once we actually retrieved the metadata we need to modify it for every transformation of the image (orientation, resize, gps). # Saving to the Photo Album In the previous code, the saving was done after computing the CDVResult. This is not the case anymore as we need access to the saved ALAsset when we want to return a NATIVE_URI. The saving is now done after all image editing has occurred. # Asynchronous processing Since the metadata retrieval and the album saving is done asynchronously, the process pipeline itself is made asynchronous. We can use resultBlock and failureBlock to get the processed image with its corresponding metadata or to get the image Asset NATIVE_URI. # FIXMEs The documentation lacks some clarity for two things (see FIXME 1 2 in code): - Does the quality option apply to the library images as well. This would mean that we cannot use NATIVE_URI with the quality set, except if we save to the photo album. - Does the orientation correction apply to the library images as well. Same thing. I actually removed that options at first, but this would mean that we need to read the EXIF data in the js code and orient the image appropriately (except for NATIVE_URIs since the web view does that automagically, but orientation correction does not make sense here). # TODOs - usesGeolocation does not make sense for an image with a source type other that CAMERA (maybe on some fringe cases?) - fix a bug where allowsEdit + saveToPhotoAlbum + NATIVE_URI make the image save in the wrong orientation (despite the metadata having the correct
