Right now, we don't support album art embedded in Vorbis comments, and there's a comment in the metadata parser that expresses some confusion as to how it would be handled. While I'm not certain of the exact implementation, we can see what other media players do and then follow along with that. This should be pretty straightforward once we take a look at some sample files.

I'll take it.

I'd recommend working on this a bit later, actually. I haven't had a chance to yet, but I'm going to write up a message about "phase 2" of the metadata improvements. Like with phase 1 (bug 841949), I'd like to land all the improvements in one go so that we don't force people to sit through the music app reparsing all their files. The main things I want for phase 2 are: vorbis album art (this bug), album artist, proper support for multi-value fields (i.e. store them as an array in the DB), genres, and probably ReplayGain support. I don't think we should try to do this in phase 1, since we should try to land bug 841949 sooner, rather than later.

Sorry, to be clearer: I foresee two times people will have to deal with a full reparse of their music collection: bug 841949, and the "phase 2" bug, which I haven't filed yet.

Bug 1065858 is phase 2.

Also we'll do Flac at the same time.

If we can get FLAC support in first, I'll port it over to be used by FLAC too.

Also I haven't tested Opus.

Comment on attachment 8534141 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26660 This looks mostly-correct, but I have a few comments on GitHub that should be addressed. It also looks like tests aren't being run, since they're commented out?

Comment on attachment 8534141 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26660 re-requesting review. Removed FLAC support as it is broken at the source: the sample I made doesn't conform to VorbisComment. That was the test not run. Addressed comments in Ogg support. This should be good to go if you feel it is.

Yep, my sample file actually uses a separate metadata block for the cover art. So it is out of scope for this bug. See bug 1118021

On the other hand we reuse the binary parsing :-D

Comment on attachment 8534141 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26660 This looks mostly good, but the changes to the test utilities concern me. Currently, the unit tests are testing the case for blobs that have no filename (e.g. things from the open activity). I think this is what we should be testing here, since that's easier to accidentally regress (QA is less likely to check that album art works from an open activity than for the main app). I plan on testing external artwork for "real" files, but that'll go somewhere else, I think. Look for a few bugs to be filed on improving the unit tests; I've already got some small patches in progress. There are a couple other minor comments on GitHub too.

Comment on attachment 8534141 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26660 addressed comments again

Comment on attachment 8534141 [details] Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/26660 r=me with two tiny editorial changes. Thanks!

Merged https://github.com/mozilla-b2g/gaia/commit/e3b73d0ad66eda4948dcd3657b8f5411e8678d7b

Quick note about Opus: I need to find real world sample / use case to determine its status. Will file bugs accordingly.