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.

Sign up for GitHub

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
Open

Fixed EXIF orientation in MediaStoreRequestHandler #1459

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9dee444
MediaStoreRequestHandler: read MediaStore.Image.ORIENTATION value and…
RobertM-BLS Jan 18, 2016
e02b0b9
Merge remote-tracking branch 'RobertM-BLS/mediastore-orientation-fix'
st1hy Jul 20, 2016
c000726
When reading exif in MediaStoreRequestHandler, read file directly wit…
st1hy Jul 20, 2016
36ddc4c
Fixed opening input stream without any reason and not closing it. Fo…
st1hy Jul 31, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions picasso/src/main/java/com/squareup/picasso/MediaStoreRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,19 @@
import android.database.Cursor;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.media.ExifInterface;
import android.net.Uri;
import android.provider.MediaStore;

import java.io.IOException;

import static android.content.ContentResolver.SCHEME_CONTENT;
import static android.content.ContentUris.parseId;
import static android.provider.MediaStore.Images;
import static android.provider.MediaStore.Video;
import static android.provider.MediaStore.Images.Thumbnails.FULL_SCREEN_KIND;
import static android.provider.MediaStore.Images.Thumbnails.MICRO_KIND;
import static android.provider.MediaStore.Images.Thumbnails.MINI_KIND;
import static android.provider.MediaStore.Video;
import static com.squareup.picasso.MediaStoreRequestHandler.PicassoKind.FULL;
import static com.squareup.picasso.MediaStoreRequestHandler.PicassoKind.MICRO;
import static com.squareup.picasso.MediaStoreRequestHandler.PicassoKind.MINI;
Expand All @@ -40,6 +42,9 @@ class MediaStoreRequestHandler extends ContentStreamRequestHandler {
private static final String[] CONTENT_ORIENTATION = new String[] {
Images.ImageColumns.ORIENTATION
};
private static final String[] CONTENT_DATA = new String[] {
Images.ImageColumns.DATA
};

MediaStoreRequestHandler(Context context) {
super(context);
Expand Down Expand Up @@ -102,16 +107,58 @@ static PicassoKind getPicassoKind(int targetWidth, int targetHeight) {
}

static int getExifOrientation(ContentResolver contentResolver, Uri uri) {
int exifOrientation = getExitOrientationFromFile(contentResolver, uri);
if (exifOrientation == ExifInterface.ORIENTATION_UNDEFINED) {
exifOrientation = getExifOrientationFromContentResolver(contentResolver, uri);
}
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.

Cursor cursor = null;
try {
contentResolver.openInputStream(uri);
cursor = contentResolver.query(uri, CONTENT_DATA, null, null, null);
if (cursor == null || !cursor.moveToFirst()) {
return 0;
}
String filePath = cursor.getString(0);
ExifInterface exifInterface = new ExifInterface(filePath);
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.

// In case of error during reading exif, assume no rotation.
return ExifInterface.ORIENTATION_UNDEFINED;
} finally {
if (cursor != null) {
cursor.close();
}
}
}

static int getExifOrientationFromContentResolver(ContentResolver contentResolver, Uri uri) {
Cursor cursor = null;
try {
cursor = contentResolver.query(uri, CONTENT_ORIENTATION, null, null, null);
if (cursor == null || !cursor.moveToFirst()) {
return 0;
}
return cursor.getInt(0);

int rotation = cursor.getInt(0);
switch (rotation) {
case 90:
return ExifInterface.ORIENTATION_ROTATE_90;
case 180:
return ExifInterface.ORIENTATION_ROTATE_180;
case 270:
return ExifInterface.ORIENTATION_ROTATE_270;
default:
return ExifInterface.ORIENTATION_NORMAL;
}
} catch (RuntimeException ignored) {
// If the orientation column doesn't exist, assume no rotation.
return 0;
return ExifInterface.ORIENTATION_UNDEFINED;
} finally {
if (cursor != null) {
cursor.close();
Expand Down