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.
Created attachment 8557189 [details] [review] 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...
Attachment #8557189 - Flags: feedback?(dflanagan)
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.
Attachment #8557189 - Flags: feedback?(dflanagan) → feedback-
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.
Attachment #8557189 - Flags: feedback- → review?(dflanagan)
I added a bonus commit to this which makes album art crossfade when a new album starts. It's a lot smoother looking!
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.
Attachment #8557189 - Flags: review?(dflanagan) → review+
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.
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.
One last try and then I'll just land it myself.
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.
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
status-b2g-master: --- → fixed
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.