deduplicate identical tags (same tag type and value)

[repomaa]

this fixes problems with duplicate values for track/tracknumber, disc/discnumber etc. without side effects.

[bastelfreak]

👍

[carnager]

oh seems to be exactly what I requested once 👍

[zeltak]

+1

[MaxKellermann]

This adds overhead by making this O(n^2). If you don't want duplicate tag values, don't store duplicate tag values in the file. MPD is the wrong place to fix this up. Fix your files instead of adding kludges to MPD.

[repomaa]

I think you need to see the overhead in context: it adds overhead only if you actually have multiple values for a tag and even then only on db updates.

[repomaa]

It's a very small change, and I agree that one shouldn't store duplicate tag values but sometimes you don't have a choice (mpd not being the only player accessing the library etc.) I really don't see the performance as an issue. And if it's not an issue i don't see why this shouldn't just be merged and be done with it. It would solve so many problems.

[repomaa]

how would you feel about using std::set instead of std::vector for the values then? This would have the complexity of O(log(n))

[MaxKellermann]

it adds overhead only if you actually have multiple values for a tag

Completely not true. Your code walks the whole tag item list for each new item.

and even then only on db updates

Also not true. This happens every time a Tag object gets assembled. That happens during database updates, during MPD startup while loading the database, and when a song/stream gets played.

how would you feel about using std::set instead of std::vector for the values then? This would have the complexity of O(log(n))

This adds even more overhead. The complexity is smaller, but the constant factor is a lot bigger.

And it's still the wrong place to do it. MPD reads the tags as-is, and if the tags are wrong, you see wrong tags - if there are duplicate tags, you'll see duplicate tags. Fix them. No lame excuses.

[repomaa]

it adds overhead only if you actually have multiple values for a tag

Completely not true. Your code walks the whole tag item list for each new item.

The whole tag item list being one element long in 99.9% of the cases. I'd say that's neglegtable performance wise. Your only reason for not merging is being unwilling to fix problems that don't have their roots in mpd per se. I understand your position, but I honestly fail to see any harm in fixing a problem many mpd users experience when there are virtually no downsides.

I'd say most if not all CSV-parsers support using other separators than , for example even though that's not real CSV. Same thing.

MPD reads the tags as-is, and if the tags are wrong, you see wrong tags - if there are duplicate tags, you'll see duplicate tags.
This is not true. You're already ignoring "album artist" tags for example.

[repomaa]

I'm happy to provide benchmarks to prove my point if that will persuade you.

[MaxKellermann]

No, I'm not interested at all. Let's stop discussing this.