On Sun, Nov 23, 2008 at 5:06 AM, Peter Szilagyi <peszilagyi@...> wrote:
> On Sun, Nov 23, 2008 at 00:20, Jeremy Evans <jeremyevans0@...> wrote:
>> struct playlist_data_t doesn't store the metadata, it only stores the
>> artist, title, and album strings (and only the first matching one in
>> the metadata, so it loses multiple artists). In order for this
>> feature to work, it will need to store a metadata_t (and hopefully a
>> fileinfo_t for additional info). Do we want to continue storing the
>> artist, title, and album strings separately in playlist_data_t, or
>> should we remove them and store just the metadata_t? If we keep them,
>> there is less existing code to change, but removing them is probably
>> better in the long run. Removing them looks like it would cause
>> issues with album nodes, though (I don't use album nodes, so I'm not
>> sure how they operate).
> Beware that not all audio files are tagged with metadata, and certain
> formats (e.g. WAV) do not support metadata at all. Still they have
> artist/album/track titles (derived from the Music Store), thus relying
> on the metadata_t struct only is not an option for these cases.
> Generally, data inherited from the Music Store should have precedence
> over metadata.
True. I don't use the Music Store at all, so I wasn't aware of what
effect it had. I suppose I can expose the existing artist, title, and
album to the extended title formatting code, though it won't be that
helpful if multiple artists are used.
> Maybe the set of accessible metadata fields should be limited in a
> sane way, and the supported fields could be stored in the
> playlist_data_t struct in a smart way so that no space is used up for
> non-existent fields (e.g. NULL-terminated list of key-value pairs,
> with #defined integer keys). The currently existing album/artist/track
> titles could be incorporated into this structure.
I consider this a bad idea, as it requires arbitrary decisions on what
to include and what not to include. I was planning on having the
title formatting code look up entries in the metadata or fileinfo as
needed. I think it is best to keep the existing entries and just add
a metadata_t * and fileinfo_t * to playlist_data_t. I don't know
enough about the internals to be removing structure entries. Is there
any reason that storing those in playlist_data_t wouldn't work?
>> The playlist on disk format should store the computed title string
>> instead of the artist, album, and title separately. This should make
>> loading playlists faster, as well. We still have to deal with reading
>> in old playlists, but that is fairly easy (ignore the existing artist,
>> album, and title entries and use the file entry to get the metadata,
>> which is read in before creating the computed title string). Again,
>> there could be issues with album nodes.
> The reason why the album, artist and track titles are stored
> separately in playlist_data_t (and on disk, consequently) is to allow
> for regenerating the title after the formatting string is changed for
> the tracks that are already added to the Playlist. If only the result
> was stored, there would be no easy way to decrypt the individual
> components and put together the new title string accorting to the
> changes. This is an issue concerning not only album nodes, but normal
> toplevel tracks as well.
I figured that that was the reason that the artist, album, and title
were stored separately, but since it's a bad idea to add all metadata
to the playlist, with extended title formatting support, you'd have to
add a computed title string (at least if you turned the support on)
unless you wanted to reread the file metadata every time the playlist
was loaded. If you changed the title format string, you would have to
rescan all files, but that would be the only time.