writing to readonly audio file is silently ignored

[Dustin Spicuzza]

When exaile tries to write a tag to disk, it doesn't bother checking to see if it actually read it. Now, it does pretend to persist the data and it is readily retrievable in the database, but the user is not aware that it is not actually persisted. This patch makes the user aware of the problem, and also reloads the tags from the file. It really should be a sync so that the user doesn't think a tag is set when it really isn't... but its not clear to me the best way to do this.

I've also pushed this to my dj_improvements branch.

[Dustin Spicuzza (dustin-virtualroadside)]

Oh, the text in the dialog box should be escaped. I've pushed this change to my branch as well.

[reacocard (reacocard)]

Part of the reason it behaves the way it does is because it's NEVER guaranteed that any particular tag will be written out to the file. The support for most tag formats handles only a fixed set of tags due to limitations of the tag format or our backend, so tags that cannot be synced to the file itself are kept in Exaile's DB to allow for some degree of storing metadata in those cases.

Definitely a good idea to warn the user when we fail to write out though. It looks like your patch halts the entire batch of writes with a dialog? Might be better to just aggregate the list of failures and display them all at once at the end, if necessary.

Wrt the comment about read_tags(), once the tags are read it should already be firing off a track_tags_changed event so that interested parties can update themselves properly. I'm somewhat concerned about the use of read_tags() as a rollback mechanism however, since as noted above the tag backend's supported tags are not necessarily the same as those in the DB itself, so if the user has added tags that are not in the tag backend, those alterations would remain even though the other changes would be reverted. A better approach might be to just keep a copy of the original tags around and to restore those if the write fails.

[Dustin Spicuzza (dustin-virtualroadside)]

Interesting. While it may be true that we don't guarantee that any particular tag will be written out to file, it seems rather non-intuitive for what I would expect. At least, for basic types of tags.

I accidentally discovered this bug when I copied a bunch of mp3's from a data CD and it retained the readonly permissions. Almost lost a bunch of work categorizing them before I realized it. So, definitely want to warn the user that the write failed. I didn't realize you could use the dialog to write lots of files at a time, actually. I suppose that was why it was in a loop.. :p

Hm. I'll think about this, won't have time to look at it again until Sunday. There's very few places that actually write tags, so perhaps just warning the user and storing the tags in exaile's DB is enough. But the idea of the DB presenting a different view to the user than what is actually in the file is a bit disturbing to me.

[reacocard (reacocard)]

> the idea of the DB presenting a different view to the user than what
> is actually in the file is a bit disturbing to me.

Well pretty much all players do this to some extent - things like rating and playcount are almost never stored in tags (for good reason), only in internal DBs. The fact that we apply this to all metadata, including tags, is in part an artifact of how most of the code doesn't distinguish between internal tags and external tags for the sake of internal simplicity and consistency. Many other player also do not allow editing of arbitrary tags as we do, only a fixed set of allowable tags, which is generally a subset of the set of tags which can be stored in all the tag formats the player supports, so the issue cannot actually arise for them.

Perhaps we could add a visual indicator in the tag editor about whether a given tag is actually capable of being written to the file or not? Just an icon next to the field (with an explanatory tooltip) should suffice. This probably requires exposing more info from xl.metadata however, since at the moment there's no way to know what tag format a given Track uses or what tags can be written to a given format.

[Dustin Spicuzza (dustin-virtualroadside)]

After some thought, I'll agree with you partially -- the track should not force a re-read of tags if there is a write failure. This should be up to whoever called write_tags() originally. I've adjusted my patch, and committed the relevant changes to my branch.

I like the idea of a visual indicator of whether a tag is supported. Perhaps add this to a TODO list somewhere.

The one thing that I really don't like is relying on the file to not change while Exaile isn't looking. This makes it annoying to use the player with other tools that DO modify the file -- perhaps a favorite tag editor, or even editing the audio of the file in place. The use case I imagine is as follows:

1) User adds file to exaile
2) User edits some tag in external audio player
3) User edits some other tag in exaile

When the user performs step 3, since exaile is only relying on cached data, the user will unexpectedly lose any changes made by the external program. Other players such as Winamp reloads/updates metadata when the user plays a track. Perhaps the solution for exaile is to have something that merges the tags on playback -- so it loads the tags from the file, and only replaces the tags that the file has, and keeps any tags that aren't currently in the file.

This last bit probably belongs in another bug.

[Dustin Spicuzza (dustin-virtualroadside)]

After some thought, I'll agree with you partially -- the track should not force a re-read of tags if there is a write failure. This should be up to whoever called write_tags() originally. I've adjusted my patch, and committed the relevant changes to my branch.

I like the idea of a visual indicator of whether a tag is supported. Perhaps add this to a TODO list somewhere.

The one thing that I really don't like is relying on the file to not change while Exaile isn't looking. This makes it annoying to use the player with other tools that DO modify the file -- perhaps a favorite tag editor, or even editing the audio of the file in place. The use case I imagine is as follows:

1) User adds file to exaile
2) User edits some tag in external audio player
3) User edits some other tag in exaile

When the user performs step 3, since exaile is only relying on cached data, the user will unexpectedly lose any changes made by the external program. Other players such as Winamp reloads/updates metadata when the user plays a track. Perhaps the solution for exaile is to have something that merges the tags on playback -- so it loads the tags from the file, and only replaces the tags that the file has, and keeps any tags that aren't currently in the file.

This last bit probably belongs in another bug.