Zeugma440 / atldotnet

Fully managed, portable and easy-to-use C# library to read and edit audio data and metadata (tags) from various audio formats, playlists and CUE sheets
MIT License
459 stars 61 forks source link

Track : Nullable string fields #169

Closed sandreas closed 1 year ago

sandreas commented 1 year ago

The problem

In https://github.com/sandreas/tone/issues/15#issue-1292428672 I try to remove metadata properties completely which does not work as expected.

Unfortunately I did not find a way to completely remove a property - just set it to empty string, which is not the same in my opinion.

My question: Is there a way to get rid of an existing string property - so REALLY removing it?

Code To Reproduce Issue [ Good To Have ]

// ATL.Settings.NullAbsentValues = true; // does not change the behaviour in any way (which is good)
var track = new Track("a-file.m4b");
track.Album = null; // does nothing instead of removing the property, which I had expected
// track.Album = ""; // sets the property to empty string (and does not remove it)
track.Save();

I would:

What do you think?

Zeugma440 commented 1 year ago

With the current API :

I have not forgotten you've made a convincing case to evolve the API to allow nullable strings. I just need some time for this to happen, as it is a major evolution. For now, please cope with setting empty strings.

I'll keep that issue open as a reminder. Thanks for your understanding~

sandreas commented 1 year ago

With the current API :

Thank you for clarifying this :-) I think this description would also make a decent FAQ entry, that can be updated as soon as this has been changed.

sandreas commented 1 year ago

So to summarize, currently, we have the following types with the according values to remove the value:

But what about these? Are my assumptions correct? (especially about Lyrics):

Zeugma440 commented 1 year ago

Let me rephrase that

Removing simple fields

Field type Value when Settings.NullAbsentValues is false Value when Settings.NullAbsentValues is true
int 0 null
float 0.0f null
DateTime DateTime.MinValue null
string "" ""

Removing complex fields

Field Value
Lyrics Touché... setting to null doesn't work ! There's no way to remove lyrics at the moment 😅
Chapters new List<ChapterInfo>() or Track.Chapters.Clear()
EmbeddedPictures new List<PictureInfo>() or Track.EmbeddedPictures.Clear()
AdditionalFields new Dictionary<string, string>() or Track.AdditionalFields.Clear()

=> During the upcoming API evolution, I'll make it so setting null to Lyrics actually removes them.

sandreas commented 1 year ago

Touché... setting to null doesn't work ! There's no way to remove lyrics at the moment 😅

Ahaa! I knew I had super nitpicking powers! ;)

=> During the upcoming API evolution, I'll make it so setting null to Lyrics actually removes them.

Great, thank you. I'm really exited about this evolution. If you need my nitpicking to you help with that, just ask =D I'm really impressed with the improvements you did since I started asking the sh.. out of you. You really DO care about your stuff!

Zeugma440 commented 1 year ago

I haven't had the time to initiate any API evolution, but I did make a small change to be able to remove lyrics.

Setting Track.Lyrics to null now works as expected since today's v4.23

sandreas commented 1 year ago

Cool, thank you. I did not have the time to do anything in the last weeks... although I have already put some effort in an AdvancedTrack, its not gonna finish in the next month. Lots of things are happening atm ;) I'll keep you updated.