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

Unify cover art sources in a configurable manner #1380

Open
OxygenCobalt opened this issue May 17, 2024 · 2 comments
Open

Unify cover art sources in a configurable manner #1380

OxygenCobalt opened this issue May 17, 2024 · 2 comments

Comments

@OxygenCobalt
Copy link
Contributor

[REQUIRED] Use case description

My app has two sources of cover art. It has a Coil setup with a bunch of extensions/settings options that would ideally be provided by BitmapLoader, and just plain MediaStore URIs stored in MediaItems. However, It seems as if ExoPlayer seemingly erratically uses three different cover art sources depending on what's being applied.

  1. The MediaSessionCompat cover metadata is directly read from the MediaMetadata.artworkUri or MediaMetadata.artworkData of a MediaItem, judging by this code. This is not configurable.
  2. The media notification reads from BitmapLoader, which can be configurable.
  3. Since ExoPlayer does auto-parse metadata, it's also possible for cover art data extracted from there to be used as well. I don't know when this is triggered and it's not configurable from what I know.

This leads to several issues.

  1. There is no way to definitely ensure that album covers are being loaded through your own source of data, such as Glide/Coil. BitmapLoader will only apply to the notification, not the MediaSessionCompat.
  2. If the sources of cover data differ for whatever reason between the notification and session, the media notification will begin to behave erratically from Android 11 onwards, often picking random covers. This makes it impossible to even use BitmapLoader in it's current form, since you have no idea if it's going to randomly pick some other cover source.

Please correct me if I don't fully understand the behavior or reasoning here.

Proposed solution

Make updates to the covers in media notification, MediaSessionCompat, and other places rely on a synchronized call to BitmapLoader that allows full configuration on what cover data should be used. The only place this can be excluded is anything related to the MediaBrowser, since I know that's a little infeasible.

Alternatives considered

I can somewhat alleviate this on my end by unifying my cover sources with my own internal URI-based system, but this is extremely complicated and I can't do this right now. It also doesn't rule out the case of ExoPlayer auto-parsing it's own metadata and overriding mine, since I don't know what conditions trigger that.

@tianyif
Copy link
Contributor

tianyif commented May 20, 2024

Hi @OxygenCobalt,

Thanks for diving deep around the MediaSession and BitmapLoader! Please see my answers below to some of your points but feel free to correct me if I misunderstood anything:

Since ExoPlayer does auto-parse metadata, it's also possible for cover art data extracted from there to be used as well. I don't know when this is triggered and it's not configurable from what I know.

Our current strategy is always prioritizing to use artworkUri or artworkData from the MediaItem.mediaMetadata in the presence of any of them, and when none of them are set with the MediaItem.mediaMetadata, we will use the artwork extracted from the stream if it is available. However, for now we haven't had more flexibility than that - eg. let the app set their preferred source of the bitmap, which is tracked as an enhancement for later #1221.

And if you have both artworkUri and artworkData set, the default implementation of BitmapLoader will prioritize to decode the one from artworkData than loading from artworkUri, but this behaviour is customizable by app's implementation.

BitmapLoader will only apply to the notification, not the MediaSessionCompat

The BitmapLoader injected with MediaSession.Builder is also used for updating the metadata on MediaSessionCompat. In fact, for Android 13+, the bitmap on the media notification is updated exactly via this way.

And I also saw that you've mentioned that from Android 11 onwards, the media notification picks the random covers. Could you please elaborate this issue a bit?

@OxygenCobalt
Copy link
Contributor Author

@tianyif

Since Android 11, media notifications draw from either the notification data (content title, content text, bitmap) or the MediaSession's current MediaMetadata. The issue is that the data picked depends on several factors, such as the OEM build, internal race conditions, or notification rate limiting. I discovered this on my own before I was using Media3, where if I did not synchronize metadata updates to be in the order of MediaSession and THEN the medianNotification, the metadata would not properly update. My final system was something like:

  1. Asynchronously load the bitmap once and block until completed
  2. Update MediaSession metadata
  3. Update notification metadata

As far as I am aware, Media3 still launches two cover loads from BitmapLoader in parallel, so you're going to run into similar confusing metadata desynchronizations if cover sources differ.

I have a hunch that this issue is at least related to new ones I've discovered when moving to Media3:

  • I accidentally set artworkUri to a URI to a cover that differed from the one I was loading with my custom BitmapLoader, and on some devices that have different timing the notification would prefer artworkUri over BitmapLoader, but others would do the opposite.
  • My BitmapLoader returns full-resolution images, but if I were to force a metadata update with replaceMediaItems, suddenly the notification cover would turn low-resolution despite it not meaningfully change.

I'm not fully confident about the root cause, but both of these seem to me that there is no way to guarantee cover consistency here without causing confusing behavior. I've just been forced to fall back to whatever ExoPlayer does instead rather than using my own internal loader until I can effectively rebuild the entire MediaStore image system and just pass URIs everywhere.

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

No branches or pull requests

3 participants