[WIP] Implements of in-app EXIF and location anonymization #1712
Conversation
*Add method anonymizeCoord() that reduces the resolution of GPS coordinates. Think of each coordinate as a pixel. This function reduces the resolution of "the world" to one coordinate(pixel) for every x kilometers(x is the variable prefLocationAccuracy) *Add method getAnonymizedDecimalCoords() that invokes anonymizeCoord() for latitude and longtitude and returns the coordinates with reduced accuracy in "lat|long" format. *Add method redactEXIFData() that removes exif tags from image and returns the Uri referring to the new, anonymized image. *Change behaviour of getPathOfMediaOrCopy() such that if anonymization is enabled, the function makes a copy of the file. The path to this copy is stored in variable fileOrCopyPath and the function always returns this path.
*Call FileProcessor.redactEXIFFields() and pass the uri returned by redactEXIFFields() to the Contribution object instead of the Uri of the unprocessed file. *Move coordinate extraction logic from local function to FileProcessor to decrease code reuse.
|
Maybe the next step would be adding options for setting EXIF and location behaviour per image in a way that would also be a permanent solution to #1599 Update "automatically get current location" setting text to inform that files will be geotagged with it |
|
I'm not sure why the conflicts remain as there seems to be nothing left that needs changing to allow a merge. |
|
Hi @ilgazer thanks for your PR but it is huge, which is something makes tester unhappy:/
|
*Call FileProcessor.redactEXIFFields() and pass the uri returned by redactEXIFFields() along with anonymized coordinates to the uploadController object instead of the Uri of the unprocessed file.
This reverts commit 35ce75a
|
Hey @neslihanturan, I know the PR is a bit big but it's all related to
#1686 . Also, I have fixed the strings.xml issue. I also tried to solve the conflicts but the conflict markers seem to still be there. I will look into it some more.
Edit: There are new conflicts I missed, will fix them when I can.
|
Codecov Report
@@ Coverage Diff @@
## master #1712 +/- ##
=========================================
+ Coverage 3.69% 3.71% +0.01%
=========================================
Files 191 193 +2
Lines 9876 9988 +112
Branches 885 903 +18
=========================================
+ Hits 365 371 +6
- Misses 9484 9589 +105
- Partials 27 28 +1
Continue to review full report at Codecov.
|
Create LongTitleMultiSelectListPreference to be used where MultiSelectListPreference was previously used.
|
@ilgazer , sorry for late testing, I was busy with other tasks in project. Currently I tried to remove Camera Model EXIF and uploaded a photo. But EXIF was still there, here is the photo I uploaded after selecting Remove EXIF tag checkbox: https://commons.m.wikimedia.org/wiki/File:Triangular_bungalow_on_seaside.jpg Besides functional problem, I think removing something by selecting check boxes doesn't feel very natural from user interface point of view. I think, instead of Remove EXIF Tags item, we can have Manage EXIF Tags item, and behind, Dislplay author, Display Copyritght checkbox items. Selected means we will include that EXIF tag (so they will all be selected as default, user can deselect if they want), unelected means it will be hidden. |
|
Your UI recommendation really makes sense and would actually be easy to
implement. I will also look at the first bug tomorrow.
…On 22:40, Wed, 29 Aug 2018 neslihanturan, ***@***.***> wrote:
@ilgazer <https://github.com/ilgazer> , sorry for late testing, I was
busy with other tasks in project. Currently I tried to remove Camera Model
EXIF and uploaded a photo. But EXIF was still there, here is the photo I
uploaded after selecting Remove EXIF tag checkbox:
https://commons.m.wikimedia.org/wiki/File:Triangular_bungalow_on_seaside.jpg
Besides functional problem, I think removing something by selecting check
boxes doesn't feel very natural from user interface point of view. I think,
instead of *Remove EXIF Tags* item, we can have *Manage EXIF Tags* item,
and behind, *Dislplay author*, *Display Copyritght* checkbox items.
Selected means we will include that EXIF tag (so they will all be selected
as default, user can deselect if they want), unelected means it will be
hidden.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1712 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHRLo43jwqs38AUSh4-R2xdAS9neWWELks5uVu4wgaJpZM4VPYZ5>
.
|
…ching preference items with EXIF tags.
|
Ok, so I implemented a new setting to remove XMP data, which is something similar to EXIF, but there are no good Android libraries to edit XMP data. Because of this, I only implemented deleting all XMP data by parsing the raw jpeg file and finding the XMP section. A future patch for better XMP support could be considered but that could easily turn into implementing a whole library, which is something I have inadequate time for. I also changed to Manage Exif tags and the ticked items being kept, which means that if there are not new bugs, this PR should finally be ready to merge. |
|
Thanks @ilgazer , I am testing it now |
|
I agree with @misaochan |
|
Then we should be more open about that and specify the precision used to fill in the template. |
@ilgazer Sure, that sounds like a good plan, let's go with that. :) Let us discuss this further at #181 , and it would be great if we could ask in the Commons Village Pump as well. In the meantime, you could remove the GPS accuracy Setting and fix the other bugs, so we can re-test and merge this PR while discussion is ongoing? It is better to have more modular PRs anyway. ;) |
|
Just to clarify, the remaining bugs (which I added in an edit later, so it might not be visible to those using email updates) are: I have tested the PR on API 24 Nexus S emulator. My uploads work as usual, but:
|
Good catch! Fixed it.
Could your image have no XMP metadata? You can upload the picture here to see its XMP metadata: http://metapicz.com
This seems incredibly odd. The only piece of code capable of doing that is the XMP removal code. Could you give a link to an image that is losing its EXIF data on upload? |
|
@ilgazer https://commons.wikimedia.org/wiki/File:Yong_tau_fu.jpg is one of them. |
|
Could you upload its original form so that I can take a better look at it? |
|
@ilgazer Shall I email you the picture? I don't think Commons will be happy with me uploading a duplicate, lol. |
|
I would love that
|
|
That would be great! :)
…On 09:12, Tue, 18 Sep 2018 Josephine Lim, ***@***.***> wrote:
@ilgazer <https://github.com/ilgazer> Shall I email you the picture? I
don't think Commons will be happy with me uploading a duplicate, lol.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1712 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHRLo2qwNT6K2Mac2Bd5HBLfwTOKO0FBks5ucI7ggaJpZM4VPYZ5>
.
|
|
Any updates with this PR @misaochan ? Does it work for you? |
|
IIRC I emailed @ilgazer the image, but he hasn't gotten back to me about it? |
domdomegg
changed the title
|
Well, I am not sure what to do with this PR, seems like it is sleeping for a while. It has conflicts and known issues. On the other hand, would be great to have this code after it is fixed. So my vote is keeping this open, someone else can take this up and fix since the code looks good overall. |
|
I will be terribly busy till the middle of June, so I agree that that
someone else taking over the PR would be the best course of action.
|
|
Hello! |
|
No you shouldn't. The purpose all that function was to make sure that the
file was copied before we destroy its metadata. Honestly I think you should
rethink where the metadata removal should be done and tie in the EXIF
clensing code accordingly.
…On Mar 23, 2019 3:21 PM, "Vitaly V. Pinchuk" ***@***.***> wrote:
Hello!
Need some help.
I'm trying to merge changes from ilgazer:master to up-to-date master and
having some troubles.
For example, FileProcessor class uses FileUtils.copy() method, which is no
longer available.
Should I put copy() back into FileUtils class or is there a better
solution?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1712 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AHRLo6mTh4G3JiGkRdo9KgK8kuY9wz4hks5vZhxmgaJpZM4VPYZ5>
.
|
|
@ilgazer, I'm trying to use ExifInterface and it's not clear for me which one I should prefer: By the way, it seems that only (1) returns image's latitude and longitude with getLatLong(float[]). Methods getLatLong() of both (2) and (3) return null despite the existence of image's location tag. |
|
Please have a look at these two images. The smaller one was taken in emulator and the other one with my OP6. Both of them are shown as having geo tags in Google Photos app. But only OP6's image seems to have lat/long tags while reading with android.support.media.ExifInterface.getLatLong().
|
|
I don't remember which one works but you should use the one I used
previously. I am not sure if I used the other one anywhere. If I used both,
use the one that works.
|
7 participants
Title (required)
Fixes #1686 Implementation of in-app EXIF modification
Fixes #212 Remove from EXIF: copyright field (name)
Fixes #181 Remove from EXIF: geo info if too close to home
Fixes #70 Option to "anonymize" pictures
Description (required)
There are multiple issues about privacy concerns location and EXIF-related privacy concerns. These commits implement two different settings for removing certain EXIF rags from the picture and for reducing the accuracy or completely eliminating geolocation data from the picture.
The bulk of the anonymization logic is implemented in FileProcessor to prevent code reuse. The existing coordinate extraction logic in MultipleShareActivity is also replaced by a FileProcessor to further reduce code reuse.
Add method anonymizeCoord() that reduces the resolution of GPS coordinates. Think of each coordinate as a pixel. This function reduces the resolution of "the world" to one coordinate(pixel) for every x kilometres (x is the variable prefLocationAccuracy). This approach was chosen over changing the coordinates by a random amount because with that approach; if a user uploads sufficiently many pictures from the same location, statistical analysis can reveal the real location of the user.
The Android support library com.android.support:exifinterface was added as some required features are lacking from earlier Android versions.
As a minor detail, strings.xml was standardized to 4 spaces per tab. That's why there are so many removed lines.
Tests performed (required)
Tested on API 22 Genymotion Google Nexus 5 emulator, with build variant, BetaDebug and ProdDebug.
Screenshots showing what changed (optional)