1126466 - [music] Use full-size album art in the player UI

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Music, defect)

Description

[Jim Porter (:squib)]

Currently, we use thumbnail-sized images in the music player. This looks bad on high-resolution devices, since we're scaling up from 300x300 pixels. Instead, we should use the full-size version of the art so that things look prettier.

Now would also be a good time to improve the transition between tracks, especially when they're in the same album. There's no need for a transition in that case.

Comment 1

[Jim Porter (:squib)]

Attachment: Fix it

David: Does this seem like a sensible way to improve the loading of album art in the player view? We're using an offscreen Image object to get the onload event, but I think I did things right so that Gecko will decode the image at the right size.

Of course, I'm also open to further improvements here...

Comment 2

[David Flanagan [:djf]]

Comment on attachment 8557189 [details] [review]
Fix it

f- because I think that the combination of background-image plus an offscreen image will cause the image to be decoded at full size. See my comments on github.

Sometimes I worry about keeping too many blob urls or references to blobs around. Gallery seems to get away with it for all of its thumbnails. I don't know whether there is anything different about these blob urls that use blobs sliced out of much larger blobs.  At some point it might be good to do some memory profiling and make sure that this album art cache isn't causing a lot of memory to be allocated.  And if so, you could perhaps convert to a cache that flushes the least recently used items.

Comment 3

[Jim Porter (:squib)]

Comment on attachment 8557189 [details] [review]
Fix it

Ok, this should be done. Only the last three commits in the PR matter; the others are only there because this work revealed a few bugs in the lazy loading for JS files in the music app.

I've opted to remove the "offscreen images" entirely in this PR. For the player view, they weren't actually helping. Things still look the same even without waiting for an offscreen image to load (in fact, I think that code wasn't doing what we expected anymore, since Gecko downsamples images now).

For the sublist view, since we're coming from a list view, we should already have the thumbnail cached, so we don't even need to fade in. This is pretty much how all the other list-based views work anyway.

Comment 4

[Jim Porter (:squib)]

I added a bonus commit to this which makes album art crossfade when a new album starts. It's a lot smoother looking!

Comment 7

[David Flanagan [:djf]]

Comment on attachment 8557189 [details] [review]
Fix it

Jim,

This looks good to me. Various comments on github, including an idea for resolving your album art race condition. But pretty much everything I had to say was an optional suggestion.

Sorry that this review took so long.

Comment 8

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#UB3Du4bdSKa6GVqxKOXWuw

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 9

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#ZhpN1XvVQnyvh01wiflXrw

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 10

[Jim Porter (:squib)]

One last try and then I'll just land it myself.

Comment 11

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#pJyxFA7eSJKofOncEnnQdQ

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 12

[Kevin Grandon :kgrandon]

Try run here looks good: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=944df3b020a5f40716ded346b6afaf5b45e66a93

Let's land for now. (Not sure what's causing the autoland issues, but will look into it)

In master: https://github.com/mozilla-b2g/gaia/commit/4b192bc9c2a9e343418b39aed5dc7b38b949373d