Closed Bug 877474 Opened 8 years ago Closed 8 years ago

[music] Music app shows filename *and* title field for Ogg Vorbis files

Categories

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

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: squib, Assigned: squib)

Details

Attachments

(1 file)

The issue here is that we pre-fill the title in our metadata with the filename, and then the title field from the Vorbis comment gets added to it, instead of replacing it. There are a few ways we could fix this:

1) Be smarter when pulling data from the Vorbis comment so that it overwrites correctly
2) Mark the filename in the title field as "temporary" somehow
3) Add the filename *after* parsing the file
4) (My preference) Never add the filename as the "title" metadata, and instead fall back to
   the filename in the UI side

As a future enhancement, when encountering multiple fields with the same name in a Vorbis comment, we should store those as an array instead of concatenating them together. Then the UI could join the strings together in an appropriate way (usually with commas or slashes).
I don't like #4 because it adds more work for the Music app.  I don't like exposing arrays in metadata because it is a special case for Vorbis and the other types don't do it.

Maybe the vorbis metadata parser can store things internally in an array, and then when it is done reading comments, it can concatenate them and overwrite the default title field?
(In reply to David Flanagan [:djf] from comment #1)
> I don't like exposing arrays in metadata because it is a special case for Vorbis and the
> other types don't do it.

id3v2 also supports multiple values per frame. In id3v2.3, this only works for a handful of frames (mostly artist names in various spots), but in id3v2.4, all text frames can have multiple values.

Storing these as an array is especially useful for filtering. If, say, I have a song with artists Alice and Bob, I'd expect to see that song when I go to the artist view for Alice. For my own usage, this is even more important for genres, since I tend to tag my music with all the genres that fit. Then I can filter by genre to get a decent mix of songs that all have a similar mood/theme.

Using arrays for this stuff is a longer-term goal though, since we currently don't support multi-value fields at all. It's something that I think is worth thinking about though, since good metadata support becomes increasingly important as people's music collections grow.

As for the main goal of this bug, I think I'll go with either (1) or (3) then. That keeps things simple.
Ok, I went with (1) above, as it was the simplest. I tested against Ogg Vorbis files with single- and multiple-value fields.
Assignee: nobody → squibblyflabbetydoo
Status: NEW → ASSIGNED
Attachment #782795 - Flags: review?(dflanagan)
Comment on attachment 782795 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/11234

That is a nice simple patch. Two comments on github, but addressing them is optional.  I didn't run the code, but assuming that you have, land it when ready.
Attachment #782795 - Flags: review?(dflanagan) → review+
Landed with review comments addressed: https://github.com/mozilla-b2g/gaia/pull/11234
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.