Feature: Adding support to read from IDv2 tag - Patch included

[M-Bab]

Support for tag reading is an often requested feature of the Clementine music player. For me it was the last feature missing to the ultimate music player. Although the ultimatelyricsreader is already pretty strong it sometimes cant find lyrics of local songs. Of course a tag reader is also faster and doesn't need an internet connection. Therefore I developed a tag reader mechanism for which the patch is included at the bottom of the thread.

What works and how:

• ext tag reader checks for USLT or SYLT frames and stores them
• Database schema 48 added to have a lyrics field in the music database
• Song code modified to read/write lyrics stored in database
• Lyricsprovider written to show the lyrics in the songinfo tab

What doesn't work:

• The Lyricsprovider itself unfortunately :-(. If I replace some code in the Ultimatelyricsprovider the tag lyrics are displayed. But the taglyricsinfoprovider does not trigger the display of the CollapsibleInfoPane on the InfoResultReady-Signal. Although taglyricsinfoprovider as QOBJECT inherits from SongInfoProvider and is added to SongInfoView.fetcher . Any ideas/tips on this are appreciated. Anything else works.

Further possible improvements: If someone is interested and the tagreader may join the Clementine code there are more ideas how I could extend it:

• Options if the tagreader shall be actually activated
• Edit lyrics in the tag editor
• Show ultimatelyrics only if no taglyrics are available
• Store ultimatelyrics in the tag lyrics if empty

4b84abf523da94b877cdd1b2af37b4fc26835719
 data/data.qrc                                      |   1 +
 data/schema/schema-48.sql                          |   4 +
 ext/libclementine-tagreader/tagreader.cpp          |  14 ++
 .../tagreadermessages.proto                        |   1 +
 src/CMakeLists.txt                                 |   2 +
 src/core/database.cpp                              |   2 +-
 src/core/mpris1.cpp                                |   1 +
 src/core/organiseformat.cpp                        |   5 +-
 src/core/song.cpp                                  |  15 +-
 src/core/song.h                                    |   2 +
 src/songinfo/songinfoview.cpp                      |   2 +
 src/songinfo/taglyricsinfoprovider.cpp             |  41 ++++
 src/songinfo/taglyricsinfoprovider.h               |  31 +++
 src/ui/mainwindow.cpp                              |   1 +
 src/ui/organisedialog.cpp                          |   1 +
 src/widgets/osd.cpp                                |   2 +
 17 files changed, 228 insertions(+), 121 deletions(-)

diff --git a/data/data.qrc b/data/data.qrc
index 162c2f7..a7a6d3a 100644
--- a/data/data.qrc
+++ b/data/data.qrc
@@ -426,5 +426,6 @@
         <file>providers/seafile.png</file>
         <file>icons/32x32/internet-services.png</file>
         <file>providers/radiotunes.png</file>
+        <file>schema/schema-48.sql</file>
     </qresource>
 </RCC>
diff --git a/data/schema/schema-48.sql b/data/schema/schema-48.sql
new file mode 100644
index 0000000..c2e8ede
--- /dev/null
+++ b/data/schema/schema-48.sql
@@ -0,0 +1,4 @@
+ALTER TABLE %allsongstables ADD COLUMN lyrics TEXT;
+
+UPDATE schema_version SET version=48;
+
diff --git a/ext/libclementine-tagreader/tagreader.cpp b/ext/libclementine-tagreader/tagreader.cpp
index 2f6ca4a..1d798e6 100644
--- a/ext/libclementine-tagreader/tagreader.cpp
+++ b/ext/libclementine-tagreader/tagreader.cpp
@@ -143,6 +143,7 @@ void TagReader::ReadFile(const QString& filename,

   QString disc;
   QString compilation;
+  QString lyrics;

   // Handle all the files which have VorbisComments (Ogg, OPUS, ...) in the same
   // way;
@@ -184,6 +185,15 @@ void TagReader::ReadFile(const QString& filename,
         compilation =
             TStringToQString(map["TCMP"].front()->toString()).trimmed();

+      if (!map["USLT"].isEmpty())
+      {
+          lyrics = TStringToQString((map["USLT"].front())->toString()).trimmed();
+          qLog(Debug) << "Read ULST lyrics " << lyrics;
+      }
+      else
+          if(!map["SYLT"].isEmpty())
+            lyrics = TStringToQString((map["SYLT"].front())->toString()).trimmed();
+
       if (!map["APIC"].isEmpty()) song->set_art_automatic(kEmbeddedCover);

       // Find a suitable comment tag.  For now we ignore iTunNORM comments.
@@ -365,6 +375,9 @@ void TagReader::ReadFile(const QString& filename,
     song->set_compilation(compilation.toInt() == 1);
   }

+  if (! lyrics.isEmpty())
+    song->set_lyrics(lyrics.toStdString());
+
   if (fileref->audioProperties()) {
     song->set_bitrate(fileref->audioProperties()->bitrate());
     song->set_samplerate(fileref->audioProperties()->sampleRate());
@@ -612,6 +625,7 @@ bool TagReader::SaveFile(const QString& filename,
                  tag);
     SetTextFrame("TCOM", song.composer(), tag);
     SetTextFrame("TIT1", song.grouping(), tag);
+    SetTextFrame("USLT", song.lyrics(), tag);
     // Skip TPE1 (which is the artist) here because we already set it
     SetTextFrame("TPE2", song.albumartist(), tag);
     SetTextFrame("TCMP", std::string(song.compilation() ? "1" : "0"), tag);
diff --git a/ext/libclementine-tagreader/tagreadermessages.proto b/ext/libclementine-tagreader/tagreadermessages.proto
index f90dbce..225e034 100644
--- a/ext/libclementine-tagreader/tagreadermessages.proto
+++ b/ext/libclementine-tagreader/tagreadermessages.proto
@@ -51,6 +51,7 @@ message SongMetadata {
   optional string etag = 30;
   optional string performer = 31;
   optional string grouping = 32;
+  optional string lyrics = 33;
 }

 message ReadFileRequest {
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 37faa3f..9c0ca3e 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -312,6 +312,7 @@ set(SOURCES
   songinfo/songkickconcerts.cpp
   songinfo/songkickconcertwidget.cpp
   songinfo/songplaystats.cpp
+  songinfo/taglyricsinfoprovider.cpp
   songinfo/ultimatelyricslyric.cpp
   songinfo/ultimatelyricsprovider.cpp
   songinfo/ultimatelyricsreader.cpp
@@ -604,6 +605,7 @@ set(HEADERS
   songinfo/songkickconcerts.h
   songinfo/songkickconcertwidget.h
   songinfo/songplaystats.h
+  songinfo/taglyricsinfoprovider.h
   songinfo/ultimatelyricslyric.h
   songinfo/ultimatelyricsprovider.h
   songinfo/ultimatelyricsreader.h
diff --git a/src/core/database.cpp b/src/core/database.cpp
index e351aa3..e3bbee5 100644
--- a/src/core/database.cpp
+++ b/src/core/database.cpp
@@ -47,7 +47,7 @@
 #include <QVariant>

 const char* Database::kDatabaseFilename = "clementine.db";
-const int Database::kSchemaVersion = 47;
+const int Database::kSchemaVersion = 48;
 const char* Database::kMagicAllSongsTables = "%allsongstables";

 int Database::sNextConnectionId = 1;
diff --git a/src/core/mpris1.cpp b/src/core/mpris1.cpp
index 6f95b5a..c563553 100644
--- a/src/core/mpris1.cpp
+++ b/src/core/mpris1.cpp
@@ -331,6 +331,7 @@ QVariantMap Mpris1::GetMetadata(const Song& song) {
   AddMetadata("composer", song.composer(), &ret);
   AddMetadata("performer", song.performer(), &ret);
   AddMetadata("grouping", song.grouping(), &ret);
+  AddMetadata("lyrics", song.lyrics(), &ret);
   if (song.rating() != -1.0) {
     AddMetadata("rating", song.rating() * 5, &ret);
   }
diff --git a/src/core/organiseformat.cpp b/src/core/organiseformat.cpp
index 6d62155..27ccd42 100644
--- a/src/core/organiseformat.cpp
+++ b/src/core/organiseformat.cpp
@@ -50,7 +50,8 @@ const QStringList OrganiseFormat::kKnownTags = QStringList() << "title"
                                                              << "samplerate"
                                                              << "extension"
                                                              << "performer"
-                                                             << "grouping";
+                                                             << "grouping"
+                                                             << "lyircs";

 // From http://en.wikipedia.org/wiki/8.3_filename#Directory_table
 const char OrganiseFormat::kInvalidFatCharacters[] = "\"*/\\:<>?|";
@@ -191,6 +192,8 @@ QString OrganiseFormat::TagValue(const QString& tag, const Song& song) const {
     value = song.performer();
   else if (tag == "grouping")
     value = song.grouping();
+  else if (tag == "lyrics")
+    value = song.lyrics();
   else if (tag == "genre")
     value = song.genre();
   else if (tag == "comment")
diff --git a/src/core/song.cpp b/src/core/song.cpp
index 36ac735..7e26c16 100644
--- a/src/core/song.cpp
+++ b/src/core/song.cpp
@@ -111,7 +111,8 @@ const QStringList Song::kColumns = QStringList() << "title"
                                                  << "effective_albumartist"
                                                  << "etag"
                                                  << "performer"
-                                                 << "grouping";
+                                                 << "grouping"
+                                                 << "lyrics";

 const QString Song::kColumnSpec = Song::kColumns.join(", ");
 const QString Song::kBindSpec =
@@ -151,6 +152,7 @@ struct Song::Private : public QSharedData {
   QString composer_;
   QString performer_;
   QString grouping_;
+  QString lyrics_;
   int track_;
   int disc_;
   float bpm_;
@@ -278,6 +280,7 @@ const QString& Song::playlist_albumartist() const {
 const QString& Song::composer() const { return d->composer_; }
 const QString& Song::performer() const { return d->performer_; }
 const QString& Song::grouping() const { return d->grouping_; }
+const QString& Song::lyrics() const { return d->lyrics_; }
 int Song::track() const { return d->track_; }
 int Song::disc() const { return d->disc_; }
 float Song::bpm() const { return d->bpm_; }
@@ -334,6 +337,7 @@ void Song::set_albumartist(const QString& v) { d->albumartist_ = v; }
 void Song::set_composer(const QString& v) { d->composer_ = v; }
 void Song::set_performer(const QString& v) { d->performer_ = v; }
 void Song::set_grouping(const QString& v) { d->grouping_ = v; }
+void Song::set_lyrics(const QString& v) { d->lyrics_ = v; }
 void Song::set_track(int v) { d->track_ = v; }
 void Song::set_disc(int v) { d->disc_ = v; }
 void Song::set_bpm(float v) { d->bpm_ = v; }
@@ -490,6 +494,7 @@ void Song::InitFromProtobuf(const pb::tagreader::SongMetadata& pb) {
   d->composer_ = QStringFromStdString(pb.composer());
   d->performer_ = QStringFromStdString(pb.performer());
   d->grouping_ = QStringFromStdString(pb.grouping());
+  d->lyrics_ = QStringFromStdString(pb.lyrics());
   d->track_ = pb.track();
   d->disc_ = pb.disc();
   d->bpm_ = pb.bpm();
@@ -513,6 +518,8 @@ void Song::InitFromProtobuf(const pb::tagreader::SongMetadata& pb) {
   d->filetype_ = static_cast<FileType>(pb.type());
   d->etag_ = QStringFromStdString(pb.etag());

+  std::string tmp_lyrics = pb.lyrics();
+
   if (pb.has_art_automatic()) {
     d->art_automatic_ = QStringFromStdString(pb.art_automatic());
   }
@@ -535,6 +542,7 @@ void Song::ToProtobuf(pb::tagreader::SongMetadata* pb) const {
   pb->set_composer(DataCommaSizeFromQString(d->composer_));
   pb->set_performer(DataCommaSizeFromQString(d->performer_));
   pb->set_grouping(DataCommaSizeFromQString(d->grouping_));
+  pb->set_lyrics(DataCommaSizeFromQString(d->lyrics_));
   pb->set_track(d->track_);
   pb->set_disc(d->disc_);
   pb->set_bpm(d->bpm_);
@@ -625,6 +633,7 @@ void Song::InitFromQuery(const SqlRow& q, bool reliable_metadata, int col) {

   d->performer_ = tostr(col + 38);
   d->grouping_ = tostr(col + 39);
+  d->lyrics_ = tostr(col + 40);

   InitArtManual();

@@ -950,6 +959,7 @@ void Song::BindToQuery(QSqlQuery* query) const {

   query->bindValue(":performer", strval(d->performer_));
   query->bindValue(":grouping", strval(d->grouping_));
+  query->bindValue(":lyrics", strval(d->lyrics_));

 #undef intval
 #undef notnullintval
@@ -1058,7 +1068,8 @@ bool Song::IsMetadataEqual(const Song& other) const {
          d->samplerate_ == other.d->samplerate_ &&
          d->art_automatic_ == other.d->art_automatic_ &&
          d->art_manual_ == other.d->art_manual_ &&
-         d->cue_path_ == other.d->cue_path_;
+         d->cue_path_ == other.d->cue_path_ &&
+         d->lyrics_ == other.d->lyrics_;
 }

 bool Song::IsEditable() const {
diff --git a/src/core/song.h b/src/core/song.h
index 938429f..d3c08b7 100644
--- a/src/core/song.h
+++ b/src/core/song.h
@@ -171,6 +171,7 @@ class Song {
   const QString& composer() const;
   const QString& performer() const;
   const QString& grouping() const;
+  const QString& lyrics() const;
   int track() const;
   int disc() const;
   float bpm() const;
@@ -249,6 +250,7 @@ class Song {
   void set_composer(const QString& v);
   void set_performer(const QString& v);
   void set_grouping(const QString& v);
+  void set_lyrics(const QString& v);
   void set_track(int v);
   void set_disc(int v);
   void set_bpm(float v);
diff --git a/src/songinfo/songinfoview.cpp b/src/songinfo/songinfoview.cpp
index 076512a..46b84f0 100644
--- a/src/songinfo/songinfoview.cpp
+++ b/src/songinfo/songinfoview.cpp
@@ -20,6 +20,7 @@
 #include "songinfoview.h"
 #include "ultimatelyricsprovider.h"
 #include "ultimatelyricsreader.h"
+#include "taglyricsinfoprovider.h"

 #ifdef HAVE_LIBLASTFM
 #include "lastfmtrackinfoprovider.h"
@@ -48,6 +49,7 @@ SongInfoView::SongInfoView(QWidget* parent)
 #ifdef HAVE_LIBLASTFM
   fetcher_->AddProvider(new LastfmTrackInfoProvider);
 #endif
+  fetcher_->AddProvider(new TagLyricsInfoProvider);
 }

 SongInfoView::~SongInfoView() {}
diff --git a/src/songinfo/taglyricsinfoprovider.cpp b/src/songinfo/taglyricsinfoprovider.cpp
new file mode 100644
index 0000000..2e3be0e
--- /dev/null
+++ b/src/songinfo/taglyricsinfoprovider.cpp
@@ -0,0 +1,41 @@
+/* This file is part of Clementine.
+   Copyright 2010, David Sansome <me@davidsansome.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "songinfotextview.h"
+#include "taglyricsinfoprovider.h"
+#include "core/logging.h"
+
+void TagLyricsInfoProvider::FetchInfo(int id, const Song& metadata) {
+
+  QString lyrics;
+  lyrics = metadata.lyrics();
+
+  if (!lyrics.isEmpty()) {
+    CollapsibleInfoPane::Data data;
+    data.id_ = "tag/lyrics";
+    data.title_ = tr("Lyrics from the ID3v2 tag");
+    data.type_ = CollapsibleInfoPane::Data::Type_Lyrics;
+
+    SongInfoTextView* editor = new SongInfoTextView;
+    editor->setPlainText(lyrics);
+    data.contents_ = editor;
+
+    emit InfoReady(id, data);
+  }
+  emit Finished(id);
+}
+
diff --git a/src/songinfo/taglyricsinfoprovider.h b/src/songinfo/taglyricsinfoprovider.h
new file mode 100644
index 0000000..0d8f393
--- /dev/null
+++ b/src/songinfo/taglyricsinfoprovider.h
@@ -0,0 +1,31 @@
+/* This file is part of Clementine.
+   Copyright 2010, David Sansome <me@davidsansome.com>
+
+   Clementine is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Clementine is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Clementine.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef TAGLYRICSINFOPROVIDER_H
+#define TAGLYRICSINFOPROVIDER_H
+
+#include "songinfoprovider.h"
+
+class TagLyricsInfoProvider : public SongInfoProvider {
+  Q_OBJECT
+
+ public:
+  void FetchInfo(int id, const Song& metadata);
+
+};
+
+#endif  // TAGLYRICSINFOPROVIDER_H
diff --git a/src/ui/mainwindow.cpp b/src/ui/mainwindow.cpp
index fd81eaa..305ea0b 100644
--- a/src/ui/mainwindow.cpp
+++ b/src/ui/mainwindow.cpp
@@ -2661,6 +2661,7 @@ void MainWindow::HandleNotificationPreview(OSD::Behaviour type, QString line1,
     fake.set_genre("Classical");
     fake.set_composer("Anonymous");
     fake.set_performer("Anonymous");
+    fake.set_lyrics("None");
     fake.set_track(1);
     fake.set_disc(1);
     fake.set_year(2011);
diff --git a/src/ui/organisedialog.cpp b/src/ui/organisedialog.cpp
index 250b199..fb978a2 100644
--- a/src/ui/organisedialog.cpp
+++ b/src/ui/organisedialog.cpp
@@ -65,6 +65,7 @@ OrganiseDialog::OrganiseDialog(TaskManager* task_manager, QWidget* parent)
   tags[tr("Composer")] = "composer";
   tags[tr("Performer")] = "performer";
   tags[tr("Grouping")] = "grouping";
+  tags[tr("Lyrics")] = "lyrics";
   tags[tr("Track")] = "track";
   tags[tr("Disc")] = "disc";
   tags[tr("BPM")] = "bpm";
diff --git a/src/widgets/osd.cpp b/src/widgets/osd.cpp
index 87f6ddb..a1cbf11 100644
--- a/src/widgets/osd.cpp
+++ b/src/widgets/osd.cpp
@@ -337,6 +337,8 @@ QString OSD::ReplaceVariable(const QString& variable, const Song& song) {
     return song.performer();
   } else if (variable == "%grouping%") {
     return song.grouping();
+  } else if (variable == "%lyrics%") {
+    return song.lyrics();
   } else if (variable == "%length%") {
     return song.PrettyLength();
   } else if (variable == "%disc%") {

[amuttsch]

Can you please provide a pull request rather than a patch file? That way it's easy to review the code.

Thanks.

[M-Bab]

All right! Turned it into Pull request https://github.com/clementine-player/Clementine/pull/4784 . As I said the issue remains that the CollapsibleInfoPane show is not actually triggered. But after spending a lot of time for the search after the cause I decided to commit the work I have done and request for Clementine-expert help on this remaining issue. I am really curious what I am missing there ...