beets icon indicating copy to clipboard operation
beets copied to clipboard

Add support for multi-valued tags

Open ryanakca opened this issue 10 years ago • 72 comments

Please add support for multi-valued tags. For example, this could be used for the multi-valued occurrences of PERFORMER / COMPOSER / ENSEMBLE / etc. recommended by http://age.hobba.nl/audio/mirroredpages/ogg-tagging.html .

ryanakca avatar Jan 25 '14 15:01 ryanakca

Related to the genre-specific feature request in #119. See also some discussion in a half-baked pull request, #437.

This will be a reasonably large task, involving changing the interface to MediaFile to support lists, making commensurate changes to the database abstraction, and sorting out the mapping for each of the file formats we support.

sampsyo avatar Jan 25 '14 20:01 sampsyo

Musicbrainz now supports multiple values for artist as well. While keeping the names themselves as a list might be cumbersome (you'd need to keep track of the join phrases), supporting lists for the artist ids should fit right into this use case.

pprkut avatar Feb 07 '14 15:02 pprkut

Congratulations to @geigerzaehler for what was obviously quite a lot of work for PR #527. As I understand it, this laid the foundation for multi-value tags, specifically adding support for genre lists.

I am very interested in seeing this support added for the artist tags as well. I would be willing to spend some time on this myself. I have read through #527's thread and spent a small amount of time looking over those commits. I obviously have a lot of stuff to search through and the information is definitely there for me to find, but I was hoping somebody more familiar with this could give me a few tips on some files/commits that I might want to begin with.

dan-boehm avatar Mar 07 '14 17:03 dan-boehm

To clarify a bit about what I think @pprkut was talking about, MusicBrainz does support multiple artists in its schema. It also provides access to that through the API. Here are a few examples:

http://musicbrainz.org/ws/2/release-group/38d1077e-c270-49a8-92ca-87e7eb8d9fe4?inc=artists http://musicbrainz.org/ws/2/release/965f06a9-5e30-4fe5-9601-2989ebdb69f5?inc=recordings+artist-credits http://musicbrainz.org/ws/2/release-group/bdaeec2d-94f1-46b5-91f3-340ec6939c66?inc=artists http://musicbrainz.org/ws/2/recording/724aac2d-d800-4979-a4e9-8d9287ba57fb?inc=artists

So the information is there. The only concern I'd have about whether or not to implement this is that not every player supports multi-artist tags. Perhaps the best option would be to provide a config option to either use the multi-artist tags or join them as a single string (as it works now).

dan-boehm avatar Mar 07 '14 17:03 dan-boehm

Hi, @Boehemyth—glad you're interested! Yes, the recent work in MediaFile is the first step toward making this work more generally.

The next step—which is arguably even more challenging—is to cleanly extend the database abstraction to store multi-valued fields. All of this should be done in the beets.dbcore package, which is our generic library for representing objects stored in a SQLite database. The documentation there and in Sphinx (see the "developers" section on RTD) should provide a reasonable intro, but please let me know if you have any questions about how things currently work.

I have no idea how to best to best go about adding this feature set to dbcore. We should somehow allow every kind of field (fixed, flexible, and computed) to have a list value while still—for compatibility and simplicity where lists are less likely to be useful—providing a non-list version of the field. That is, it would be great if code could refer to item.artist and get a single string even if there's another route to get the full list of artists.

It's also not clear what the underlying SQLite schema should be. I'm open to suggestions if anyone has ideas.

Thanks for volunteering!

sampsyo avatar Mar 07 '14 18:03 sampsyo

That is, it would be great if code could refer to item.artist and get a single string even if there's another route to get the full list of artists.

I agree. MusicBrainz returns a seperator for each artist except the last one. If we were to store these seperators in a new field as a list in the same order as the artists in the artist field, we could write a function to construct the string.

It's also not clear what the underlying SQLite schema should be. I'm open to suggestions if anyone has ideas.

I'm not exactly sure how we should go about this either. I think our best bet is to store the list as a single string with a seperator character for the individual elements. I think that's how some of the audio tags handle this anyhow. Maybe it makes sense to just stick with that? I'd have to look back at that PR to remember how it was determined to be handled. The icing on the cake is that we wouldn't need any changes to the current database schema.

It may also be possible to store entire python data structures as BLOBs in the database. I'd have to investigate that to be sure, but I personally like the other option better anyways.

We should somehow allow every kind of field (fixed, flexible, and computed) to have a list value while still—for compatibility and simplicity where lists are less likely to be useful—providing a non-list version of the field.

The one limitation to the string idea is that it won't work for other SQLite datatypes. I don't think that that will be a problem. From where I stand, there aren't any non-string fixed attributes that could benefit from a list. I'm not as sure about flexible or computed fields. I can't think of a case myself, but maybe somebody else can?

dan-boehm avatar Mar 07 '14 19:03 dan-boehm

From where I stand, there aren't any non-string fixed attributes that could benefit from a list. I'm not as sure about flexible or computed fields. I can't think of a case myself, but maybe somebody else can?

The EchoNest API provides the raw analysis data they use in determining their higher-level attributes like danceability, etc. For example, they do beat detection, the result of which is simply an array of sample or frame indices with confidences or probabilities for each. Of course, this is a massive amount of data that I doubt anyone would be interested in keeping inside a beets database. Anyway, it's a (far-fetched) use case for lists of floats.

pedros avatar Mar 07 '14 19:03 pedros

Here are two reasons for adding list-valued fields everywhere:

  • Attachments (see #111) should be built on flexible attributes. We should allow multiple attachments of the same type, which will be much easier if we have list-valued attributes. Attachments need to be stored as SQLite BLOBs (not strings) to avoid Unicode issues.
  • Consistency. It would be really nice not to have to worry about types when storing lists.

Querying would also be complicated by packing multiple values into a string. We'd have to split each value and compare against the query rather than just pattern-matching.

sampsyo avatar Mar 07 '14 21:03 sampsyo

These are good points. Unfortunately, that means we have to do this the standard way, which means implement new tables, which I was really hoping to avoid, because it really will be a pain.

I'll use album-artists as an example. We'd have to create a new table that lists the album/albumartist relationships. Then for queries, we have to query the album on that table to get all of the artists. I know it sounds pretty round-about, but that method actually does work remarkably well. I do understand what you mean now about how much work this will end up being.

The issue is that if we really want to make this work for EVERY field, things are going to get messy very quickly. There are some things we could do to help clean it up a bit. For instance, to use the artist example again, we could have an Artist table that stores relevant info about an artist. We'd still need an album-artist table and item-artist table, but at least now you only have two artist relationship tables instead of 6 (including artist_sort and mbartist_id).

Also, I'm not entirely sure how that would work for flexible fields. The question is can we assume a one-to-many relationship. For instance, in the case of Attachments, each attachment is related to exactly one Album or Item right? If that can be assumed about flexible fields, we could probably just create a new table for each flexible field, instead of adding the flexible field to the album or item table.

dan-boehm avatar Mar 07 '14 21:03 dan-boehm

Perhaps we might be able to get away with not implementing lists with a few of the fixed fields that we know doesn't need this functionality. Dates for instance? Or Track/Disk numbers?

dan-boehm avatar Mar 07 '14 22:03 dan-boehm

The issue is that if we really want to make this work for EVERY field, things are going to get messy very quickly.

I actually don't know whether I agree with this—I think creating one-off tables for a few fields where lists are important could be messier in the long run! I'm certain that, in the future, we'll think of new fields (existing and novel) that will need to be listified, and needing to retrofit each of these independently sounds a bit like a nightmare. For maintainability, it would be awesome to have a generic facility for list-valued fields that can be reused with zero (or near-zero) additional code.

Ideally, this would work by devising an ingenious and efficient mechanism for adding lists to every field, whether we need it or not. This would make listification orthogonal to the field set (abstraction FTW!). I realize that this might be needlessly complex, though—in which case we can consider a design that can't provide lists for every field but needs only tiny code changes to support multiple values for a new field. (Stuff like dates could be excluded if that's harder for some reason, as you proposed.)

sampsyo avatar Mar 07 '14 22:03 sampsyo

I would be willing to spend some time on this myself.

That's awesome @Boehemyth. Personally, I think that there is not that much to work left in the MediaFile API, apart from refactoring, refining, and documenting. Adding support for lists in tags was motivated by the native ability of most tagging formats to store those. We should keep away any additional logic that would be better fitted to database models. But I noticed you already shifted towards the database issue.

As for the discussion that’s unfolding I just would like to point out that it’s probably best to delay writing code and first put something written up on the wiki. There is already a lot of great conceptual stuff there.

It also might be time to push for a document oriented database.

geigerzaehler avatar Mar 07 '14 22:03 geigerzaehler

We also have to consider the user interface: How will we present lists to the user and let them modify these lists.

geigerzaehler avatar Mar 07 '14 22:03 geigerzaehler

I think creating one-off tables for a few fields where lists are important could be messier in the long run! I'm certain that, in the future, we'll think of new fields (existing and novel) that will need to be listified, and needing to retrofit each of these independently sounds a bit like a nightmare. For maintainability, it would be awesome to have a generic facility for list-valued fields that can be reused with zero (or near-zero) additional code.

By listifying every field, we would essentially be having a separate table for every field. This is inefficient, messy, and (I believe) unnecessary. However, your concern is valid. I think the best solution would be one where we can incrementally convert fields to lists without having to convert everything at once. This does mean we would have to support both list/non-list fields, which will be a pain up-front, but shouldn't be too much of a problem in the long-run. So long as it doesn't take much effort to move a field to the list system down the road if it is deemed necessary.

I have an idea for this that I'm currently looking at. I'll post again when I have something to present.

We also have to consider the user interface: How will we present lists to the user and let them modify these lists.

@geigerzaehler brings up a really good point, and I honestly don't know right now. I kind of want to focus on the backend implementation right now.

dan-boehm avatar Mar 07 '14 23:03 dan-boehm

Yes, it would be great to avoid a separate table for every list-valued field! I can only think of two ways around this, both of which have obvious issues:

  • Move everything to flexible fields (key/value schema).
  • Leave SQLite behind entirely, as suggested by @geigerzaehler.

These obvious roadblocks are exactly why this issue has stagnated for so long…

sampsyo avatar Mar 07 '14 23:03 sampsyo

As for the discussion that’s unfolding I just would like to point out that it’s probably best to delay writing code

I agree about the code-writing part. It's best to have a plan before getting too deep.

first put something written up on the wiki.

I can do that once I have a more concrete suggestion. Where on the wiki would I post such a thing?

It also might be time to push for a document oriented database.

That would probably make things easier, but I have no experience in such a thing.

dan-boehm avatar Mar 07 '14 23:03 dan-boehm

I know it has been awhile, but I've been looking into this issue in my spare time. Based on what I've learned about python's sqlite3 package, relational database design, and how beets currently works, I've come to a number of conclusions:

  1. The "correct" way of putting lists into a sqlite database is to create a separate table that uses foreign keys to link back to the main entry, and then have another column in this reference table for items in the list. Obviously this is a problem if we want to implement lists for every field.
  2. It would be possible to define another boolean in the schema definitions for items and albums in library.py. This boolean would define whether it is a list field or not. The idea is to try to make the conversion to lists for things we need to without doing so with things we obviously would not. This would be a very complicated solution. So much so that as I kept looking into all the things that would have to be changed, I would frequently get lost and frustrated with just how difficult this would be. I think it's possible, but it would be a real huge PitA.
  3. The final option I came up with is probably the easiest for us to implement. I noticed that no matter the type of field, it will always convert the value to a string before storing it to the database. I was surprised by this, because the sqlite3 package has a way of doing this same sort of thing for you, but this is good because we can do regular expressions for queries on TEXT columns from directly within SQLite.

SQLite provides the REGEXP keyword that tells it to use a user-defined function to see if there is a regular expression match.

import sqlite3

def re_fn(expr, item):
    reg = re.compile(expr)
    return reg.search(item) is not None

conn = sqlite3.connect(:memory:)
conn.create_function("REGEXP", 2, re_fn)

Now we can do something like this:

cur = conn.cursor()
cur.execute('SELECT FROM ? WHERE ? REGEXP ?', (tableName, columnName, pattern))

And it will return any row where the column matches the patern.

If we were to use a separator character that we know won't conflict with any character in any value, we can store it like that. For illustration's sake, let's say we used a semicolon for the seperator character. Then we can query using a regular expression like

pattern = '^(.*;)*(' + queryValue + ')(;.*)*$'

I haven't dove into the Query class enough to understand exactly how querying the database in beets works, but I think that this could be an acceptable solution. Can someone with a better understanding of beets think of any major problems or complications with this idea?

dan-boehm avatar Apr 07 '14 20:04 dan-boehm

Thanks for all the thoughts on this! We should indeed get moving on list-valued fields.

I'm a little confused about your comment about conversion to strings. I am pretty sure we don't do this—can you explain why you think we're converting all values to strings?

In that light, packing all lists as separated strings is somewhat worrisome. Serializing and deserializing is a lot of overhead to pay for every query. Especially for numeric fields, where this will require a bunch of manipulation on each row to do any comparison.

Another way we could do this would be to effectively make all fields into flexible fields: put it all in one big id/key/value table. There are also indirection overheads there, but at least the possibility of optimization (unlike with serialization to strings).

sampsyo avatar Apr 08 '14 05:04 sampsyo

I'm a little confused about your comment about conversion to strings. I am pretty sure we don't do this—can you explain why you think we're converting all values to strings?

I'm sorry. I think I didn't look at the types.py carefully enough. I misinterpreted a comment in there, and I didn't look at the code enough to confirm what I thought I read.

Serializing and deserializing is a lot of overhead to pay for every query. Especially for numeric fields, where this will require a bunch of manipulation on each row to do any comparison.

I agree. Especially since number values aren't being stored as strings. But I still think it's the best option without doing a complete redesign of the database schema. And I definitely think it's the best option if we want list support for every field.

Another way we could do this would be to effectively make all fields into flexible fields: put it all in one big id/key/value table. There are also indirection overheads there, but at least the possibility of optimization (unlike with serialization to strings).

How would we optimize such a table? The only thing I can think of to do would be to index by the item/album #. This would still be extremely inefficient. We'd literally need one SQL for every field insert/update/read. Items have almost 30 fields. That's 30 INSERT SQLs just for adding a single item to the database. Add support for lists and that number can be even bigger. Reading/Updating the database would be just as inefficient unless we also indexed by column name, but that will just make inserts take even longer, and it still wouldn't be as fast as a single query to the database.

Unless I'm missing a better way to optimize this, I'm having a really hard time agreeing that this would be preferable to serializing/deserializing strings.

dan-boehm avatar Apr 09 '14 16:04 dan-boehm

You make lots of great arguments for why the key/value approach would also be inefficient. Clearly, what we really want here is a document-oriented database and we're working around the fact that we don't have one.

Maybe the most helpful thing to do here would be to hold a "bake-off". To me, it's not completely obvious to me which approach is faster—key-value or string serialization. An ambitious contributor could try implementing tiny, toy versions of each. Both implementations should implement exactly the same interface. They could then write a couple of benchmarks that demonstrate the performance characteristics and argue—with data!—that one is faster than the other.

Any takers?

sampsyo avatar Apr 09 '14 23:04 sampsyo

I think the effort would be better spent on moving to a document oriented database.

geigerzaehler avatar Apr 10 '14 13:04 geigerzaehler

@geigerzaehler I looked into both of the possibilities given in the wiki. One of them would result in the same problems we are currently having, because you have to define tables and schema anyways. The other, I think it was called SQLiteDBM, will not suit our needs. It essentially works by pickling/unpickling python structures and storing them in a sqlite database. This would result in worse efficiency problems than what we are proposing above, because we would no longer be able to query the database directly.

dan-boehm avatar Apr 10 '14 18:04 dan-boehm

@sampsyo I think that's probably the best thing to do, even if it does seem like a pain. Perhaps we should begin by defining exactly what we want the interface to be?

I can't help but think that there has got to be some software out there already that can do this the way we want. I did a little looking into the document-oriented databases that were mentioned in the wiki, and while they would be really awesome to work with, I think they're a step back from our current database. Perhaps somebody else would like to look into it to see if they agree. Still, I can't help but feel there should already be a solution to this.

dan-boehm avatar Apr 10 '14 18:04 dan-boehm

Yes, there are suspicious performance problems with some of the alternative databases. But if we're going to be comparing with data, it might also make sense to compare against those just to make sure our intuition is correct!

One project that caught my eye (not finished, but has the right idea for our use case): https://github.com/dw/acid/


And yes, it would totally be a good idea to settle on the interface. We should come up with something clean and regular. I don't exactly know what it should look like, but I like the idea of allowing simple access when you just need a single value alongside list access when you need all the data. Not sure if that's possible to support elegantly, but it's nice to dream. :wink:

sampsyo avatar Apr 10 '14 18:04 sampsyo

Acid does look pretty interesting. At a quick glance, it seems quite a bit more powerful than the other options I looked at. I am having a hard time figuring out exactly where the project is at. Do you know anything about that?

dan-boehm avatar Apr 10 '14 19:04 dan-boehm

No, it's a little opaque in terms of project status. Maybe it's worth contacting the author?

sampsyo avatar Apr 10 '14 19:04 sampsyo

Found Blitz-DB. It quite new but seems promising. Haven't had a look at the code though.

geigerzaehler avatar Apr 11 '14 11:04 geigerzaehler

Very cool; BlitzDB is indeed very promising. One unfortunate wrinkle is that the current backend uses a directory full of individual JSON files, which is somewhat inconvenient compared to a database single file. But their priorities are definitely aligned with ours.

I suppose we also shouldn't rule out heavily modifying something existing like BlitzDB or even starting from scratch.

sampsyo avatar Apr 11 '14 17:04 sampsyo

Hey guys ! Just discovering beets and just... waow !

I'd like to know if there's any news on this, especially regarding multi-artitsts support (and multi-albumartists). Just did a test with XBMC (Kodi soon) which now supports musicbrainz tags.

Thanx a bunch !

Jucgshu avatar Aug 05 '14 16:08 Jucgshu

So, there appear to be a few options here:

  1. Replace the entire database with a document store. This is unlikely to happen any time soon, and is a massive undertaking.
  2. Change the schema so that everything uses key/value pairs. After all the work is done to update the entire schema, this would make enabling list support for any field essentially a noop. Schema for the k/v table might be something like: id(int), key(string), stringValue(string), intValue(int), dateValue(datetime), etc for the standard types, and the referencing field would then define the type, which would determine the value column and allow for efficient comparisons of various value types. However, this approach means negatively impacting every query because you have zero denorm'd data, and all queries now require joins.
  3. Store multiple values in a single field. If people aren't interested in querying the multi-value fields and just want their music tagged correctly, this is probably fine - just store the multiple values in their own (additional) field, with a fixed separator and write tags with whatever separator the user has configured, leave the existing primary field as is in the DB. If multi-value relationships do need to be queried though, this is not a viable answer. I'd actually be fine with this 'cheat' though, because all I care about is tagging.
  4. Just use a join table for each multi-value field. This is the standard and sane answer, why does this option appear to be frowned upon here?

pdf avatar Jan 07 '15 13:01 pdf