Make MediaFile extendable

[geigerzaehler]

This will allow plugins to write and read custom tags and synchronize them with the database as requested in #572 and #637. Coincidentally, this can also solve geigerzaehler/beets-check#1 and (to some extend) #608.

TODO

• Figure out how to expose the API to plugins.
• Read custom tags into flexible fields

[sampsyo]

Great idea, and a great replacement for the old plugin API that we removed recently that did this in a less-good way.

One question: it seems to me like the update method (while definitely convenient here) doesn't exactly supersede save. For example, this obviously doesn't save anything:

mediafile.artist = 'something'
mediafile.update({})

I think the imperative interface (set then save) is useful in enough scenarios that it's worth keeping. (Especially if we follow the plan to eventually spin it off as a reusable library.) But keeping the new update is good too for batch updates as in Item.write(). And this is a nice solution for no-op saves too, IMO.

[geigerzaehler]

Some issues came up in #608.

Say you assign to art (i.e., the legacy CoverArtField). This causes the images array to change to have just one element. But if you assigned the right image into art, that assignment would not be detected as a change. So the change to the length of images would not be persisted on save().

The same happens for genres. Maybe a better approach would be to cache all initial values and compare them when we want to write the file. As for performance issues: Mutagen already caches all tags, so the comparison only strains the cpu and memory. But those are incredibly cheap compared to writing to disk.

Similarly, the current implementation without the dirty flag will (e.g.) normalize weird formatting in ReplayGain fields when they're assigned. With the dirty flag, the field only gets saved (and the formatting updated) if the encoded value actually changes.

This is not solved by the cache above and leads me to think, that the mediafile layer is not the right place to solve this at all. The logic should be contained in mutagen, which can make a byte for byte comparison. I’ll have a look that this and maybe open a PR in their project.

[sampsyo]

All good points. To argue against myself for a moment, it's possible that these issues are unimportant: for example, it could actually be desirable that beets doesn't fiddle with your existing non-standard ReplayGain tags if you don't change their represented values.

But this is one reason why it's important to keep save() around as a non-deprecated API. Its contract can be "writes the file no matter what", which is useful in certain circumstances when you really need too update the file on disk. update() can be the more common case when you have a batch of values and want laziness, possibly at the expense of no-ops where changes would otherwise occur.

One possible alternative: we could move the dirty tracking to Item rather than MediaFile. The Model base class already has a dirty field set for database updates; we could do exactly the same thing for MediaFile tag updates in Item. Then the contract with MediaFile is clear and the level of abstraction is high. Nice and clean!

BTW, this is refactoring is getting nicer and nicer. I really like the removal of the duplicate information that lets us rid ourselves of some constants in library.py. Totally awesome! ✨

[geigerzaehler]

Ok, I think I have to concede—once again. This "lazy write" business needs thorough inspection. I also realized the mixing property setters and the update() method leads to some unexpected results.

The rest should be alright from a feature perspective. If we can't find any more bugs this should be ready to merge.

[sampsyo]

It looks awesome; I really like this direction and the simplification it brings to the various field lists we used to have. Great insight here!

In the mean time, I'll also make a note to try adding as-needed writes in Item.