Skip to content
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

GH-329: (android) Fixes issue: Exif data lost on many cases #331

Merged
merged 2 commits into from Feb 21, 2019

Conversation

AlvaroHerrero
Copy link
Contributor

@AlvaroHerrero AlvaroHerrero commented Aug 26, 2018

Platforms affected

Android

What does this PR do?

Reads the exif data from the file on these cases:
-When destType = DATA_URL
-When file is selected from gallery.
-When targetHeight or targetWidth is set, quality is not 100 or correctOrientation is true

What testing has been done on this change?

Tested on two differente android devices.

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

closes #329

@AlvaroHerrero AlvaroHerrero changed the title GH-329: (android) Fixes issue: Exif data lost on differente cases GH-329: (android) Fixes issue: Exif data lost on many cases Aug 26, 2018
@@ -994,6 +994,7 @@ private Bitmap getScaledAndRotatedBitmap(String imageUrl) throws IOException {
// read exifData of source
exifData = new ExifHelper();
exifData.createInFile(filePath);
exif.readExifData();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this variable used? If this.correctOrientation is true, it is overwritten again and then used to get the orientation, but otherwise?

@@ -994,6 +994,7 @@ private Bitmap getScaledAndRotatedBitmap(String imageUrl) throws IOException {
// read exifData of source
exifData = new ExifHelper();
exifData.createInFile(filePath);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OT to your PR, but man is this a bad variable file. I read it as "create in file", but it actually means "create in(put)file".

@@ -994,6 +994,7 @@ private Bitmap getScaledAndRotatedBitmap(String imageUrl) throws IOException {
// read exifData of source
exifData = new ExifHelper();
exifData.createInFile(filePath);
exifData.readExifData();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't really understand how reading some data into a property/class variable of exifData influences the further execution of code. Can you explain?

@AlvaroHerrero
Copy link
Contributor Author

createInFile is the name of the function in the ExifHelper. It initializes all exif properties to null, but it is when readExifData is called when actual exif values are set.
Later, when exifData.writeExifData is called the stored values are set into the new file (rotated or whatever proccess the code does to it).
I think when the createInFile is called, it should be followed by the readExifData one, as it is done inside processResultFromCamera function.

@janpio janpio added this to 🐣 New PR / Untriaged in Apache Cordova: Plugin Pull Requests Sep 16, 2018
@janpio janpio moved this from 🐣 New PR / Untriaged to ⛔ Blocked: Tests failing in Apache Cordova: Plugin Pull Requests Sep 16, 2018
@janpio janpio moved this from ⛔ Blocked: Tests failing to ⏳ Ready for Review in Apache Cordova: Plugin Pull Requests Sep 30, 2018
@ddsky
Copy link

ddsky commented Oct 3, 2018

I have tested this and it works as intended. Voting for a pull of this fix.

@janpio
Copy link
Member

janpio commented Oct 3, 2018

(@ddsky You should be able to leave an actual "Approve" review in the "Files changed" tab - that will make it easier for us to spot the positive review. Thanks!)

Copy link

@ddsky ddsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked for me

Copy link

@j3k0 j3k0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me and saved my day!

@ddsky
Copy link

ddsky commented Oct 29, 2018

any news on when this will be merged?

@janpio
Copy link
Member

janpio commented Oct 30, 2018

You will learn about it here by an update to the PR being posted by GitHub. (If I am involved, it will now happen 2 minutes later as I had to open this notification, read your message and reply.)

Tilexou added a commit to Tilexou/cordova-plugin-camera that referenced this pull request Nov 6, 2018
Includes the pull request apache#331 (apacheGH-329) from the main fork
Includes the pull request apache#238 from the main fork
@BorntraegerMarc
Copy link

Can I help in anyway to get this PR merged? It has been quite some time and it seems like everything is ok 🙂

@janpio janpio merged commit 81b878d into apache:master Feb 21, 2019
Apache Cordova: Plugin Pull Requests automation moved this from ⏳ Ready for Review to 🏆 Merged, waiting for Release Feb 21, 2019
@BorntraegerMarc
Copy link

thanks all for merging! Could someone tell me when the next version release is planned? 🙂

@janpio
Copy link
Member

janpio commented Feb 21, 2019

No, as there is not ETA for releases. Someone from the committer team will have to take the time to prepare the release (which means understanding all the PRs merged, giving them another round of testing, doing the actual release. this time probably also updating CI configuration as the last release was some time ago and new OS versions were released - which of course also have to be tested.)

@mirko77
Copy link

mirko77 commented Mar 7, 2019

Exif metadata are not saved on iOS either (iOS 12) with the following settings:

camera_options={
  quality: 50,
  sourceType: source_type,
  destinationType: Camera.DestinationType.FILE_URI,
  targetWidth: 1024,
  targetHeight: 1024,
  encodingType: Camera.EncodingType.JPEG,
  correctOrientation: true
};

Is this issue related?

@AlvaroHerrero
Copy link
Contributor Author

It should not. I just fixed the Java code for Android.

@mirko77
Copy link

mirko77 commented Mar 7, 2019

I will have to open another issue then, could not find any existing one about EXIF data missing on iOS.
Works fine on Android though, and I do not have the PR in my code. Weird

@BorntraegerMarc
Copy link

@mirko77 EXIF data is working fine for us on iOS 12

@mirko77
Copy link

mirko77 commented Mar 7, 2019

@mirko77 EXIF data is working fine for us on iOS 12

@BorntraegerMarc I cannot figure it out. Even if I take a picture with the camera app and send it to me via Gmail, the image does not have any EXIF metadata in it. Any suggestions?

@chintan13
Copy link

I am also facing this issue. let me once you create special issue for it, so I can also follow progress on it.

@mirko77
Copy link

mirko77 commented Mar 11, 2019

I am also facing this issue. let me once you create special issue for it, so I can also follow progress on it.

Do you mean you cannot get the EXIF data in iOS as well?

@dlydiard
Copy link

i also do not get EXIF data on iOS when using the camera with:
{
quality: 85,
sourceType: Camera.PictureSourceType.CAMERA,
mediaType: Camera.MediaType.PICTURE,
allowEdit: false,
correctOrientation: true
}

@chintan13
Copy link

Yes in case of IOS HEIC image format
sourceType = this.camera.PictureSourceType.PHOTOLIBRARY;
destinationType = this.camera.DestinationType.FILE_URI

We are loosing metadata from image.

@mirko77
Copy link

mirko77 commented Mar 19, 2019

Has this issue been reported yet? @chintan13 @dlydiard

@chintan13
Copy link

no but there is open pr from last two years,
I suspect now that code might be much outdated.

@janpio
Copy link
Member

janpio commented Jun 19, 2019

Please open an issue @chintan13 and link to the PR - this will greatly improve the chance that the PR gets merged.

@ryaa
Copy link

ryaa commented Nov 10, 2019

Has this issue been reported yet? @chintan13 @dlydiard

I added an issue for the problem with EXIF on iOS (see #524) and provided a potential fix (see #525)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Apache Cordova: Plugin Pull Requests
🏆 Merged, waiting for Release
Development

Successfully merging this pull request may close these issues.

Exif data lost when correctOrientation is true
9 participants