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
462 stars 61 forks source link

Can we have an option to disable ID3v2.2-3 separator? #281

Closed gnattu closed 1 month ago

gnattu commented 1 month ago

The problem

Currently the ID3v2.2/2.3 tagged artists will get automatically separated with the predefined separator in ATL (the / character). However, that character is also common among artists' names. For our use case, we allow the user to have a more customizable control on the splitting behavior: https://github.com/jellyfin/jellyfin/pull/12385, which makes the internal split of ATL a bit redundant.

Environment

Details

The behavior is likely implemented here:

https://github.com/Zeugma440/atldotnet/blob/0532913a2aa7ba7284fcc0693de9404c50bf32cc/ATL/AudioData/IO/ID3v2.cs#L1958-L1961

Code To Reproduce Issue [ Good To Have ]

Having a file sample.mp3 with Artist tag A/BC in ID3v2.3

Track track = new Track("sample.mp3");
var artist = track.Artist // `/` will be replaced by `;`

It would be good to have an option to disable this in ATL.Settings to disable splitting for ID3 versions below ID3v2.4

Zeugma440 commented 1 month ago

Hi there,

Thanks for explaining your issue.

Do you know you can change the default character to whichever suits you best?

Example :

ATL.Settings.DisplayValueSeparator = '/'; // or even ' ' if you fancy

The property is static; you just have to do that once (e.g. at app startup) to change it for all subsequent calls.

gnattu commented 1 month ago

Yes, we are using ATL.Settings.DisplayValueSeparator, but that is also used for other purposes:

Our current workaround is to check the MetadataFormats of the track, and if the format have ID3v2 as short name but not ID3v2.4 as full name, we replace the DisplayValueSeparator back to /, but that is not ideal in my opinion.

Zeugma440 commented 1 month ago

Alright, thanks for clarifying.

That's a very specific thing to ask. To be honest, I'd normally say that's a client issue, not a library issue.

However, I understand that Jellyfin recently switched from another library to ATL and built a part of its features around the behaviour of the previous library, which is a delicate situation I'd love to help you solve.

As what you're asking does not add complexity to the library, I'm okay with implementing it.

I'll make it part of next release, as soon as the other recently opened issues are solved.

Zeugma440 commented 1 month ago

Fix is available in today's v6.05. Please close this issue if it works on your side~

gnattu commented 1 month ago

Confirmed working