zero: Remove tags instead of setting them to null

[ghost]

This issue was automatically migrated from Google Code. Original author: adrian.sampson (December 21, 2012 06:31:58) Original issue: http://code.google.com/p/beets/issues/detail?id=478

[mineo]

Was there ever any progress on this? I've now used some dirty hack (namely, if not val: return in MediaFile.set) to achieve not having empty tags in my files but I'm not sure that fits into beets' overall architecture .

[sampsyo]

No, I haven't looked into this in a while -- although the implementation should be straightforward except for dealing with cross-format compatibility in the Mutagen API. Therein lies the most helpful thing we could do now: add __delete__ to MediaField and its related classes and experiment with a few different formats to make sure Mutagen behaves as expected.

[bossanova808]

Just further on our replaygain chats - tried with a flac just now and these RG tags are set to 0s which is very wrong and results in very loud playback.

@yevgenybezman I presume you are factoring this in to https://github.com/sampsyo/beets/pull/598 - pretty essential in this case as this behaviour is definitely not good.

[sampsyo]

That's a pretty good reason to prioritize differentiating non-existent fields from zero-valued fields.

[yevgenybezman]

Yes, of course.

[geigerzaehler]

This is a pretty important issue. I would like to change the MediaFile API so that setting properties to None will delete that tag from the file.

[sampsyo]

Sounds reasonable. We could also map this to del mediafile.field (i.e., the descriptor's __delete__ method) in case setting to None causes problems with existing code.

[geigerzaehler]

With 7b954d9 this works now for the ReplayGain fields. We are on our way to make it work for all fields.

[sampsyo]

Awesome. I'm still not 100% sure whether we want to remove null-normalization in all cases for all fields since it's incredibly useful to know that item.title is always a string. But that doesn't preclude removing tags in the zero plugin nonetheless (by interposing on writes).

[sampsyo]

Another alternative would be to provide a "nullification" filter when writing tags—when the artist field is the empty string, for example, it could be removed from the file. This would be the opposite of the de-nullification we already do.

[sampsyo]

@yevgenybezman, did you intend to close this? I don't think we've implemented it yet.

[yevgenybezman]

Oops, pressed the wrong button.

[hkhanna]

I was considering solving this by having zero trigger on after_write rather than on write. Then, zero could open the mediafile directly and strip the specified tags from the file post-write, similar to the way scrub does it now. This would allow us to sidestep the difficultly caused by null-normalization. Does anyone see any obvious issues with this approach?

[sampsyo]

Well, one reason we were considering the more complicated approach—transparently removing tags when their values are empty in the database—is that it would make the tag-deletion functionality available to other components. For example, you could manually delete a tag using the modify command.

But your proposal does sidestep the complexity of our earlier boil-the-ocean proposal, which needs to figure out how to add nullability to every built-in field in the beets system. A classic functionality-vs-effort trade-off...

[geigerzaehler]

I think this has been resolved (test case).

[sampsyo]

Hmm… @geigerzaehler, it looks like that test checks that we're not mucking with the database (awesome!) but I think this ticket is about the eternal bugaboo about removing ID3 frames and the like. I don't think that's resolved yet, right?

[geigerzaehler]

The MediaFile implementation deletes tags/frames that are None. I also checked it manually.

[sampsyo]

Great! Thanks for clarifying, @geigerzaehler.