From 1d4e3edda91cf863d02ded6168a7f77c85a5eb4c Mon Sep 17 00:00:00 2001 From: Oleg Antonyan Date: Tue, 14 Jul 2026 21:49:14 +0200 Subject: [PATCH] parse more tags --- README.md | 2 +- app/playlist/mpdloader.cpp | 3 ++ app/playlist/sorter.cpp | 23 +++++++++++ app/playlist/sorter.h | 3 ++ app/playlist_ui/columnsconfig.cpp | 6 +++ app/settings_ui/settingsdialog.cpp | 3 +- app/sort_ui/sortingpresetsdialog.cpp | 3 ++ app/track.cpp | 59 +++++++++++++++++++++++++-- app/track.h | 10 +++++ tests/tst_columnsconfig.cpp | 24 +++++++++++ tests/tst_sorter.cpp | 61 ++++++++++++++++++++++++++++ tests/tst_track.cpp | 46 +++++++++++++++++++++ 12 files changed, 237 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index ad1d69608..0c7196442 100644 --- a/README.md +++ b/README.md @@ -176,7 +176,7 @@ columns_config: width_percent: 0 ``` -Available fields: artist, album, title, year, length, path, url, sample_rate, bitrate, channels, track_number, format, filename. +Available fields: artist, album, album_artist, title, year, genre, length, path, url, sample_rate, bitrate, channels, track_number, disc_number, format, filename. Available alignments: left, right. diff --git a/app/playlist/mpdloader.cpp b/app/playlist/mpdloader.cpp index 1e2400a9f..be8bc5492 100644 --- a/app/playlist/mpdloader.cpp +++ b/app/playlist/mpdloader.cpp @@ -60,6 +60,9 @@ namespace Playlist { 0, 0 ); + track.setAlbumArtist(song.albumArtist); + track.setGenre(song.genre); + track.setDiscNumber(song.discNumber > 0 ? static_cast(song.discNumber) : 0); } track.generateUidByHashing(playlist_name); track.setPlaylistName(playlist_name); diff --git a/app/playlist/sorter.cpp b/app/playlist/sorter.cpp index 2523e78f7..9d5f235e4 100644 --- a/app/playlist/sorter.cpp +++ b/app/playlist/sorter.cpp @@ -40,6 +40,12 @@ namespace Playlist { result = compare_artist(t1, t2); } else if (attr == "ALBUM") { result = compare_album(t1, t2); + } else if (attr == "ALBUMARTIST") { + result = compare_album_artist(t1, t2); + } else if (attr == "GENRE") { + result = compare_genre(t1, t2); + } else if (attr == "DISCNUMBER") { + result = compare_disc_number(t1, t2); } else if (attr == "YEAR") { result = compare_year(t1, t2); } else if (attr == "TRACKNUMBER") { @@ -92,4 +98,21 @@ namespace Playlist { int Sorter::compare_dir(const Track &t1, const Track &t2) const { return -QString::localeAwareCompare(t1.dir(), t2.dir()); } + + int Sorter::compare_album_artist(const Track &t1, const Track &t2) const { + return -QString::localeAwareCompare(t1.album_artist(), t2.album_artist()); + } + + int Sorter::compare_genre(const Track &t1, const Track &t2) const { + return -QString::localeAwareCompare(t1.genre(), t2.genre()); + } + + int Sorter::compare_disc_number(const Track &t1, const Track &t2) const { + if (t1.disc_number() < t2.disc_number()) { + return 1; + } else if (t1.disc_number() > t2.disc_number()) { + return -1; + } + return 0; + } } diff --git a/app/playlist/sorter.h b/app/playlist/sorter.h index 9ebe9e2f6..5f6a5d84c 100644 --- a/app/playlist/sorter.h +++ b/app/playlist/sorter.h @@ -26,6 +26,9 @@ namespace Playlist { int compare_title(const Track &t1, const Track &t2) const; int compare_artist(const Track &t1, const Track &t2) const; int compare_dir(const Track &t1, const Track &t2) const; + int compare_album_artist(const Track &t1, const Track &t2) const; + int compare_genre(const Track &t1, const Track &t2) const; + int compare_disc_number(const Track &t1, const Track &t2) const; }; } diff --git a/app/playlist_ui/columnsconfig.cpp b/app/playlist_ui/columnsconfig.cpp index f07f5e506..d74657e6f 100644 --- a/app/playlist_ui/columnsconfig.cpp +++ b/app/playlist_ui/columnsconfig.cpp @@ -99,10 +99,16 @@ namespace PlaylistUi { return track.artist(); } else if (fld == "album") { return track.album(); + } else if (fld == "album_artist") { + return track.album_artist(); + } else if (fld == "genre") { + return track.genre(); } else if (fld == "title") { return track.title(); } else if (fld == "year" && track.year() > 0) { return QString::number(track.year()); + } else if (fld == "disc_number" && track.disc_number() > 0) { + return QString::number(track.disc_number()); } else if (fld == "length") { return track.formattedDuration(); } else if (fld == "path") { diff --git a/app/settings_ui/settingsdialog.cpp b/app/settings_ui/settingsdialog.cpp index a8f79473f..014e7b158 100644 --- a/app/settings_ui/settingsdialog.cpp +++ b/app/settings_ui/settingsdialog.cpp @@ -46,7 +46,8 @@ namespace { }; const QStringList kColumnFields = { - "artist", "album", "title", "year", "length", "track_number", + "artist", "album", "album_artist", "title", "year", "genre", + "length", "track_number", "disc_number", "path", "url", "filename", "format", "bitrate", "channels", "sample_rate", }; diff --git a/app/sort_ui/sortingpresetsdialog.cpp b/app/sort_ui/sortingpresetsdialog.cpp index b396de393..7aa62a93f 100644 --- a/app/sort_ui/sortingpresetsdialog.cpp +++ b/app/sort_ui/sortingpresetsdialog.cpp @@ -88,10 +88,13 @@ void SortingPresetsDialog::on_buttonHelp_clicked() { "Fields available to sort (case insensitive):
" " * Artist
" " * Album
" + " * AlbumArtist
" " * Title
" " * Year
" + " * Genre
" " * Filename
" " * TrackNumber
" + " * DiscNumber
" " * Directory
" "
" "By default sort in ascending order. Add - before the field name to change to descending order.
" diff --git a/app/track.cpp b/app/track.cpp index bcb2ab8d2..656817a4b 100644 --- a/app/track.cpp +++ b/app/track.cpp @@ -12,6 +12,18 @@ #include #include +#include + +namespace { + QString firstProperty(const TagLib::PropertyMap &props, const char *key) { + const TagLib::StringList v = props.value(key); + if (v.isEmpty()) { + return QString(); + } + return QString(v.front().toCString(true)); + } +} + Track::Track() { _uid = 0; _begin = 0; @@ -128,23 +140,38 @@ bool Track::fillTags() { if(!f.isNull()) { if (f.tag()) { TagLib::Tag *tag = f.tag(); + const TagLib::PropertyMap props = tag->properties(); + _album_artist = firstProperty(props, "ALBUMARTIST"); _artist = QString(tag->artist().toCString(true)); if (_artist.isEmpty()) { - const TagLib::StringList aa = tag->properties().value("ALBUMARTIST"); - if (!aa.isEmpty()) { - _artist = QString(aa.front().toCString(true)); - } + _artist = _album_artist; } _album = QString(tag->album().toCString(true)); + _genre = QString(tag->genre().toCString(true)); _title = QString(tag->title().toCString(true)); _year = static_cast(tag->year()); _track_number = static_cast(tag->track()); + _disc_number = parseDiscNumber(firstProperty(props, "DISCNUMBER")); return true; } } return false; } +// DISCNUMBER is commonly "1/2" - take the numerator +quint16 Track::parseDiscNumber(const QString &raw) { + if (raw.isEmpty()) { + return 0; + } + const QString num = raw.section('/', 0, 0).trimmed(); + bool ok = false; + const uint value = num.toUInt(&ok); + if (!ok || value > std::numeric_limits::max()) { + return 0; + } + return static_cast(value); +} + bool Track::reload() { return fillAudioProperties() && fillTags(); } @@ -166,6 +193,18 @@ void Track::setDuration(quint64 dur) { _duration = dur; } +void Track::setAlbumArtist(const QString &aa) { + _album_artist = aa; +} + +void Track::setGenre(const QString &g) { + _genre = g; +} + +void Track::setDiscNumber(quint16 dn) { + _disc_number = dn; +} + void Track::setCue(bool is_cue) { _cue = is_cue; } @@ -192,6 +231,14 @@ QString Track::album() const { return _album; } +QString Track::album_artist() const { + return _album_artist; +} + +QString Track::genre() const { + return _genre; +} + QString Track::title() const { if (_title.isEmpty()) { if (isStream()) { @@ -342,6 +389,10 @@ quint16 Track::track_number() const { return _track_number; } +quint16 Track::disc_number() const { + return _disc_number; +} + quint32 Track::begin() const { return _begin; } diff --git a/app/track.h b/app/track.h index cee396d0d..bf02dec9c 100644 --- a/app/track.h +++ b/app/track.h @@ -24,6 +24,7 @@ class Track { explicit Track(const QUrl &stream_url, const QString &filepath_reference); static QString formattedTime(quint64 tm); + static quint16 parseDiscNumber(const QString &raw); bool isValid() const; @@ -31,6 +32,9 @@ class Track { bool fillTags(); bool reload(); void setDuration(quint64 dur); + void setAlbumArtist(const QString &aa); + void setGenre(const QString &g); + void setDiscNumber(quint16 dn); void setCue(bool is_cue = true); void generateUidByHashing(const QString &prefix); void setPlaylistName(const QString &pln); @@ -40,8 +44,11 @@ class Track { QUrl url() const; QString artist() const; QString album() const; + QString album_artist() const; + QString genre() const; QString title() const; quint16 year() const; + quint16 disc_number() const; quint32 duration() const; quint32 sample_rate() const; quint8 channels() const; @@ -80,8 +87,11 @@ class Track { QString filepath; QString _artist; QString _album; + QString _album_artist; + QString _genre; QString _title; quint16 _year; + quint16 _disc_number = 0; quint64 _duration; quint32 _sample_rate; quint8 _channels; diff --git a/tests/tst_columnsconfig.cpp b/tests/tst_columnsconfig.cpp index fe1266406..009c6a0b7 100644 --- a/tests/tst_columnsconfig.cpp +++ b/tests/tst_columnsconfig.cpp @@ -32,6 +32,8 @@ private slots: void valueByFieldName(); void valueForYearZeroIsEmpty(); void valueForUnknownFieldIsEmpty(); + void valueForNewTagFields(); + void valueForDiscNumberZeroIsEmpty(); void settersReplaceContents(); void serializeRoundTripsThroughDeserialize(); void vaidateThrowsOnMismatchedVectorSizes(); @@ -86,6 +88,28 @@ void TestColumnsConfig::valueByFieldName() { QCOMPARE(cfg.value(5, t), t.formattedDuration()); } +void TestColumnsConfig::valueForNewTagFields() { + ColumnsConfig cfg; + cfg.setFields(QVector{ QStringLiteral("album_artist"), + QStringLiteral("genre"), + QStringLiteral("disc_number") }); + Track t = mk(); + t.setAlbumArtist(QStringLiteral("Various Artists")); + t.setGenre(QStringLiteral("Jazz")); + t.setDiscNumber(2); + QCOMPARE(cfg.value(1, t), QStringLiteral("Various Artists")); + QCOMPARE(cfg.value(2, t), QStringLiteral("Jazz")); + QCOMPARE(cfg.value(3, t), QStringLiteral("2")); +} + +void TestColumnsConfig::valueForDiscNumberZeroIsEmpty() { + ColumnsConfig cfg; + cfg.setFields(QVector{ QStringLiteral("disc_number") }); + Track t = mk(); + QCOMPARE(t.disc_number(), quint16{0}); + QCOMPARE(cfg.value(1, t), QString()); +} + void TestColumnsConfig::valueForYearZeroIsEmpty() { ColumnsConfig cfg; Track t = mk(QStringLiteral("/p.mp3"), QStringLiteral("A"), diff --git a/tests/tst_sorter.cpp b/tests/tst_sorter.cpp index 23c44da49..4660de3fb 100644 --- a/tests/tst_sorter.cpp +++ b/tests/tst_sorter.cpp @@ -30,6 +30,10 @@ private slots: void unknownCriterionIsIgnored(); void titleArtistAlbumFilenameOrderings(); void sortVectorEndToEnd(); + void albumArtistOrdering(); + void genreOrdering(); + void discNumberOrdering(); + void discNumberBeforeTrackNumber(); }; void TestSorter::defaultCriteriaIsCanonical() { @@ -126,5 +130,62 @@ void TestSorter::sortVectorEndToEnd() { QCOMPARE(v.at(2).year(), quint16{2010}); } +void TestSorter::albumArtistOrdering() { + Sorter s(QStringLiteral("ALBUMARTIST")); + Track a = mk({}, {}, {}, {}, 0, 0); + Track b = mk({}, {}, {}, {}, 0, 0); + a.setAlbumArtist(QStringLiteral("Aphex Twin")); + b.setAlbumArtist(QStringLiteral("Boards of Canada")); + QVERIFY(s.condition(a, b)); + QVERIFY(!s.condition(b, a)); + + Sorter desc(QStringLiteral("-ALBUMARTIST")); + QVERIFY(desc.condition(b, a)); +} + +void TestSorter::genreOrdering() { + Sorter s(QStringLiteral("GENRE")); + Track a = mk({}, {}, {}, {}, 0, 0); + Track b = mk({}, {}, {}, {}, 0, 0); + a.setGenre(QStringLiteral("Ambient")); + b.setGenre(QStringLiteral("Rock")); + QVERIFY(s.condition(a, b)); + QVERIFY(!s.condition(b, a)); + QVERIFY(!s.condition(a, a)); +} + +void TestSorter::discNumberOrdering() { + Sorter s(QStringLiteral("DISCNUMBER")); + Track d1 = mk({}, {}, {}, {}, 0, 0); + Track d2 = mk({}, {}, {}, {}, 0, 0); + d1.setDiscNumber(1); + d2.setDiscNumber(2); + QVERIFY(s.condition(d1, d2)); + QVERIFY(!s.condition(d2, d1)); + QVERIFY(!s.condition(d1, d1)); + + Sorter desc(QStringLiteral("-DISCNUMBER")); + QVERIFY(desc.condition(d2, d1)); +} + +// The multi-disc fix: disc 2 track 1 must not sort beside disc 1 track 1. +void TestSorter::discNumberBeforeTrackNumber() { + Sorter s(QStringLiteral("DISCNUMBER / TRACKNUMBER")); + Track d1t1 = mk(QStringLiteral("/d1t1.flac"), {}, {}, {}, 1, 0); + Track d1t2 = mk(QStringLiteral("/d1t2.flac"), {}, {}, {}, 2, 0); + Track d2t1 = mk(QStringLiteral("/d2t1.flac"), {}, {}, {}, 1, 0); + d1t1.setDiscNumber(1); + d1t2.setDiscNumber(1); + d2t1.setDiscNumber(2); + + QVector v{d2t1, d1t2, d1t1}; + std::sort(v.begin(), v.end(), [&s](const Track &a, const Track &b) { + return s.condition(a, b); + }); + QCOMPARE(v.at(0).filename(), QStringLiteral("d1t1.flac")); + QCOMPARE(v.at(1).filename(), QStringLiteral("d1t2.flac")); + QCOMPARE(v.at(2).filename(), QStringLiteral("d2t1.flac")); +} + QTEST_GUILESS_MAIN(TestSorter) #include "tst_sorter.moc" diff --git a/tests/tst_track.cpp b/tests/tst_track.cpp index 7de0675a8..588764cd1 100644 --- a/tests/tst_track.cpp +++ b/tests/tst_track.cpp @@ -14,6 +14,13 @@ private slots: void formattedTimeDays(); void formattedTimeRoundsDownToSeconds(); + void parseDiscNumberPlain(); + void parseDiscNumberWithTotal(); + void parseDiscNumberEmptyOrGarbage(); + void parseDiscNumberOutOfRange(); + + void extraTagSettersRoundTrip(); + void fullCtorPopulatesFields(); void fullCtorGeneratesNonZeroUid(); void fullCtorWithNonexistentPathIsInvalid(); @@ -66,6 +73,9 @@ void TestTrack::defaultTrackIsZeroed() { QCOMPARE(t.sample_rate(), quint32{0}); QCOMPARE(t.year(), quint16{0}); QCOMPARE(t.track_number(), quint16{0}); + QCOMPARE(t.disc_number(), quint16{0}); + QVERIFY(t.album_artist().isEmpty()); + QVERIFY(t.genre().isEmpty()); QVERIFY(!t.isCue()); QVERIFY(!t.isMpd()); QVERIFY(!t.isStream()); @@ -368,5 +378,41 @@ void TestTrack::isStreamWithEmptyUrlIsFalse() { QVERIFY(!t.isStream()); } +void TestTrack::parseDiscNumberPlain() { + QCOMPARE(Track::parseDiscNumber(QStringLiteral("1")), quint16{1}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("12")), quint16{12}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral(" 2 ")), quint16{2}); +} + +void TestTrack::parseDiscNumberWithTotal() { + QCOMPARE(Track::parseDiscNumber(QStringLiteral("1/2")), quint16{1}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("2/2")), quint16{2}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral(" 3 / 4 ")), quint16{3}); +} + +void TestTrack::parseDiscNumberEmptyOrGarbage() { + QCOMPARE(Track::parseDiscNumber(QString()), quint16{0}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("")), quint16{0}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("disc one")), quint16{0}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("/2")), quint16{0}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("-1")), quint16{0}); +} + +void TestTrack::parseDiscNumberOutOfRange() { + QCOMPARE(Track::parseDiscNumber(QStringLiteral("65535")), quint16{65535}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("65536")), quint16{0}); + QCOMPARE(Track::parseDiscNumber(QStringLiteral("99999999999")), quint16{0}); +} + +void TestTrack::extraTagSettersRoundTrip() { + Track t; + t.setAlbumArtist(QStringLiteral("Various Artists")); + t.setGenre(QStringLiteral("Jazz")); + t.setDiscNumber(2); + QCOMPARE(t.album_artist(), QStringLiteral("Various Artists")); + QCOMPARE(t.genre(), QStringLiteral("Jazz")); + QCOMPARE(t.disc_number(), quint16{2}); +} + QTEST_GUILESS_MAIN(TestTrack) #include "tst_track.moc"