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

Fixed EXIF orientation in MediaStoreRequestHandler #1459

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

st1hy
Copy link

@st1hy st1hy commented Jul 20, 2016

Merged changes from #1269 into this and fixed merge conflicts.

In my test even those changes do not fix orientation problems 100%, because at least on my phones orientation from content resolver do not always match actual EXIF in file. Therefore I've added reading EXIF directly from file. This data tends to be correct pretty much always.

I've left reading from content resolver as a fall-back if there is an exception when reading file.

RobertM-BLS and others added 4 commits January 18, 2016 11:01
… translate it to ExifInterface.ORIENTATION_* value.

Picasso determines orientation according to ExifInterface.ORIENTATION_*. All possible values in MediaStore.Image.ORIENTATION are: 0, 90, 180, 270 (according to: https://developer.android.com/reference/android/provider/MediaStore.Images.ImageColumns.html#ORIENTATION). The values differ, thus orientation read from MediaStore must be translated to ExifInterface.ORIENTATION_* value.

Updated RequestHandler::getExifOrientation description.
# Conflicts:
#	picasso/src/main/java/com/squareup/picasso/RequestHandler.java
…h ExifInterface. If file cannot be read use value provided by content resolver.
@cjwilliams24680
Copy link

Is there any update on this?

@JakeWharton
Copy link
Collaborator

Updates are posted as comments to issues so if there's no new comments then
there's no new updates.

On Thu, Oct 13, 2016 at 5:24 PM Chris Williams [email protected]
wrote:

Is there any update on this?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1459 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEEETBW0x_AGzbq0zDgpG5zHD7lThraks5qzqGbgaJpZM4JQ1et
.

@lmller
Copy link

lmller commented Feb 16, 2017

this fix doesn't seem to work, since BitmapHunter.transformResult(Request data, Bitmap result, int exifRotation) does a matrix.preRotate(exifRotation);
so my image is now rotated by 8 degrees, instead of ORIENTATION_ROTATE_270 (which is 8)

return exifInterface.getAttributeInt(ExifInterface.TAG_ORIENTATION,
ExifInterface.ORIENTATION_UNDEFINED);

} catch (Exception ignored) {
Copy link

Choose a reason for hiding this comment

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

Instead of catching and ignoring all exceptions, I think the proper way would be to retrieve the mime type of the image and only attempt to get orientation from exif metadata if the type supports exif (jpeg), otherwise return undefined.

return exifOrientation;
}

static int getExitOrientationFromFile(ContentResolver contentResolver, Uri uri) {
Copy link

Choose a reason for hiding this comment

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

exif*. I think it would be more clear to parse the file path and pass it in here instead of a uri. It seems incorrect to call this "getExifOrientationFromFile" and not pass a file.

@ghost
Copy link

ghost commented Apr 26, 2019

this fix doesn't seem to work, since BitmapHunter.transformResult(Request data, Bitmap result, int exifRotation) does a matrix.preRotate(exifRotation);
so my image is now rotated by 8 degrees, instead of ORIENTATION_ROTATE_270 (which is 8)

May be too late, but exifRotation takes value from getExifRotation(int orientation):

static int getExifRotation(int orientation) {
    int rotation;
    switch (orientation) {
      case ORIENTATION_ROTATE_90:
      case ORIENTATION_TRANSPOSE:
        rotation = 90;
        break;
      case ORIENTATION_ROTATE_180:
      case ORIENTATION_FLIP_VERTICAL:
        rotation = 180;
        break;
      case ORIENTATION_ROTATE_270:
      case ORIENTATION_TRANSVERSE:
        rotation = 270;
        break;
      default:
        rotation = 0;
    }
    return rotation;`
  }

Those it can be only 0, 90, 180, 270.
Details - #1269
On my opinion the fix from #1269 sufficient. Supplying content as Uri for image we rely on data from ImageProvider(on this case we don't care about exif info). When supplying file as uri we rely on exif info. There is still issue on library - Images received from MediaProvider.Images ignore rotation after decoding(getExifRotation(int orientation) always returns 0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants