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

Picasso and MediaStore.Images.Thumbnails bug #872

Open
dextorer opened this issue Feb 5, 2015 · 9 comments
Open

Picasso and MediaStore.Images.Thumbnails bug #872

dextorer opened this issue Feb 5, 2015 · 9 comments

Comments

@dextorer
Copy link

dextorer commented Feb 5, 2015

We have been experiencing a weird behavior on our music player app, which loads local cover art images using Picasso. Some users reported wrong cover art images on some albums on a total random basis: instead of the album cover art, they had a screenshot. Clearing the cache, reinstalling the app and event clearing MediaStore's data didn't seem to help.

We managed to replicate the issue on a test device and, after quite a lot of headaches, we narrowed down the problem to the MediaStoreRequestHandler class. This line

bitmap = Images.Thumbnails.getThumbnail(contentResolver, id, picassoKind.androidKind, options);

apparently returned a wrong Bitmap. Googling the issue led to this Stackoverflow question and subsequently to this official bug. Long story short, it seems that SOMETIMES the MediaStore is returning a wrong image instead of returning null. This happens especially when the asked size is MICRO_KIND (our album cover arts were resized to 328x328, which falls in this particular size).

Now, we are aware that this has nothing to do with Picasso itself, but still, this is a nasty bug which might affect other developers as well. It would be great if Picasso could be aware of that or, at least, provide a way around it.

Proposed solution

The MediaStoreRequestHandler class queries the MediaStore service in order to obtain a suitable-sized Bitmap according to the requested size. A possible solution might involve adding a dedicated flag on the Request class, so that MediaStoreRequestHandler looks like this:

@Override public Result load(Request data, int networkPolicy) throws IOException {
    ContentResolver contentResolver = context.getContentResolver();
    int exifOrientation = getExifOrientation(contentResolver, data.uri);

    String mimeType = contentResolver.getType(data.uri);
    boolean isVideo = mimeType != null && mimeType.startsWith("video/");

    if (data.hasSize() && !data.ignoreMediaStoreThumbnails) { // additional flag

      [...]

      if (bitmap != null) {
        return new Result(bitmap, DISK, exifOrientation);
      }
    }

    return new Result(decodeContentStream(data), DISK, exifOrientation);
  }

Unfortunately, since it was causing a riot on our users, we had to go for a dirty quick fix, which is:

@Override public Result load(Request data, int networkPolicy) throws IOException {
    ContentResolver contentResolver = context.getContentResolver();
    int exifOrientation = getExifOrientation(contentResolver, data.uri);
    return new Result(decodeContentStream(data), DISK, exifOrientation);
  }

Of course, that is not the way to go. Hence, we look forward to hearing from you on this.

@dnkoutso
Copy link
Collaborator

dnkoutso commented Feb 7, 2015

@dextorer thanks for the report. Its unfortunate but can this be handled with a custom RequestHandler in your case?

@dextorer
Copy link
Author

dextorer commented Feb 8, 2015

We could probably extend MediaStoreRequestHandler and override the load() method using the three-lines long one I briefly wrote above. Not sure about how Picasso would behave having two handlers for the same authority though.

However, other developers might experience the same issue and blame Picasso for loading the wrong images from MediaStore, which is not the case. It would be great if Picasso could be aware of this particular problem, either by exposing a flag that ignore existing thumbnails or by offering another MediaStoreRequestHandler, so that painful experiences like the one we had can be avoided 👍

Thanks for your interest, anyway.

@dnkoutso
Copy link
Collaborator

dnkoutso commented May 9, 2015

Please provide a PR and we can merge this.

@dnkoutso dnkoutso added this to the Picasso 2.5.3 milestone May 9, 2015
@dextorer
Copy link
Author

@dnkoutso Are you OK with adding a flag to the Request class (which was our proposal)? Do you suggest other approaches?

@dnkoutso
Copy link
Collaborator

Will the flag be part of the API? I find it a bit weird to expand the API only to add a flag that is relevant to thumbnails and images loaded from the mediastore.

@dextorer
Copy link
Author

Agreed. Instead, this flag could live inside MediaStoreRequestHandler. However, in order to enable/disable it, the API still needs an additional method to retrieve the preloaded handlers.

It would look like this, more or less:

  1. Retrieve the MediaStoreRequestHandler that is added by default
  2. Enable the flag

Again, I am open to any suggestions here. :)

@devankit
Copy link

the same issue for me in my music player but the wrong image occurance increases 5 fold if i .fit() method when loading a image . and does picasso try 3 times for local images also??

@ypresto
Copy link

ypresto commented Jun 10, 2015

Maybe related: #985

@JakeWharton
Copy link
Member

Good info in #1341 about this.

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