Closed
Bug 877474
Opened 11 years ago
Closed 11 years ago
[music] Music app shows filename *and* title field for Ogg Vorbis files
Categories
(Firefox OS Graveyard :: Gaia::Music, defect)
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).
Comment 1•11 years ago
|
||
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?
Assignee | ||
Comment 2•11 years ago
|
||
(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.
Assignee | ||
Comment 3•11 years ago
|
||
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 4•11 years ago
|
||
Comment on attachment 782795 [details]
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+
Assignee | ||
Comment 5•11 years ago
|
||
Landed with review comments addressed: https://github.com/mozilla-b2g/gaia/pull/11234
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
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.
Description
•