Standardizing + Correcting plugin.xml and Licensing #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dieppe
pushed a commit
to jnuine/cordova-plugin-camera
that referenced
this pull request
Jun 5, 2015
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/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). #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. #CropToSize CropToSize is not a valid option (it is always set to NO anyway) so it has been removed. #FIXMEs The documentation lacks some clarity for two things (see FIXME apache#1 & apache#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 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 orientation afaict) WARNING: I did not test the usesGeolocation option due to a lack of time (and, frankly, a lack of interest;). ACKNOWLEDGMENT: Shoutout to @athibaud who also took part in the refactoring effort. Any bug’s on him! ;)
dieppe
pushed a commit
to jnuine/cordova-plugin-camera
that referenced
this pull request
Nov 10, 2015
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/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). #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. #CropToSize CropToSize is not a valid option (it is always set to NO anyway) so it has been removed. #FIXMEs The documentation lacks some clarity for two things (see FIXME apache#1 & apache#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 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 orientation afaict) WARNING: I did not test the usesGeolocation option due to a lack of time (and, frankly, a lack of interest;). ACKNOWLEDGMENT: Shoutout to @athibaud who also took part in the refactoring effort. Any bug’s on him! ;)
dieppe
pushed a commit
to jnuine/cordova-plugin-camera
that referenced
this pull request
Feb 18, 2016
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/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). #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. #CropToSize CropToSize is not a valid option (it is always set to NO anyway) so it has been removed. #FIXMEs The documentation lacks some clarity for two things (see FIXME apache#1 & apache#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 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 orientation afaict) WARNING: I did not test the usesGeolocation option due to a lack of time (and, frankly, a lack of interest;). ACKNOWLEDGMENT: Shoutout to @athibaud who also took part in the refactoring effort. Any bug’s on him! ;)
dieppe
pushed a commit
to jnuine/cordova-plugin-camera
that referenced
this pull request
Mar 1, 2016
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 apache#1 & apache#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 automagically, but orientation correction does not make sense here). - 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 orientation afaict) WARNING: I did not test the usesGeolocation option due to a lack of time (and, frankly, a lack of interest;). ACKNOWLEDGMENT: Shoutout to @athibaud who also took part in the refactoring effort. Any bug’s on him! ;)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.