boysetsfrog / vimpc

Official repository for vimpc a vi/vim inspired client for the Music Player Daemon (mpd). Pull requests are welcome.
GNU General Public License v3.0
266 stars 34 forks source link

Reading song metadata from MPD's name tag #93

Open Rio6 opened 4 years ago

Rio6 commented 4 years ago

Like #63, I was trying to play an HTTP stream from a m3u file (with song names inside #EXTINF directives). The playlist screen only show the URL but not the song names. After looking at mpc source, it seems like the extended m3u metadata is stored inside the name tag.

This PR allows vimpc to get name and comment tag from MPD and display them.

One thing I'm uncertain is on src/mpdclient.cpp:2139, I had to change && to || for the tags to update. It's probably not good as it changes the old behaviour, so I'd appreciate for advice on how to change that.

This is my first PR, please tell me if I can improve anything. Thanks very much.

tkapias commented 1 year ago

Hi, I use MPD with extended M3U playlists and I was looking for a way to use the playlist file to display stream metadata in 4 places:

Your PR solve at least the last case by fetching the metadata to the current playlist.

I updated the changes because there was some conflicts and it works fine.

I am not sure about the implication of the '||' conditions in src/mpdclient.cpp, someone else should check it.

There is the patch:

diff --git a/doc/help.txt b/doc/help.txt
index b4029e9..24fa232 100644
--- a/doc/help.txt
+++ b/doc/help.txt
@@ -487,6 +487,8 @@
     %n    - Track number
     %N    - Track number zero padded to two digits
     %c    - Disc number
+    %N    - Song name, This is not the song title
+    %C    - Comment
     %l    - Duration
     %f    - Filename/URI

@@ -501,7 +503,7 @@ The setting songfillchar is used to fill up space due to alignment when
 rendering a song. The default setting is ' '.

  Examples:
-    The default song format is: "{%a - %t}\|{%f}$E$R $H[$H%l$H]$H"
+    The default song format is: "{{%a - }%t}\|{%N}\|{%f}$E$R $H[$H%l$H]$H"
     This displays artist - title if they both exist otherwise
     displays the filename and displays the duration aligned
     to the right. The $H ensures that [ and ] will not change
diff --git a/src/clientstate.cpp b/src/clientstate.cpp
index d00c981..f46fd76 100644
--- a/src/clientstate.cpp
+++ b/src/clientstate.cpp
@@ -372,18 +372,24 @@ void ClientState::DisplaySongInformation()
       {
          char const * const cartist  = mpd_song_get_tag(currentSong_, MPD_TAG_ARTIST, 0);
          char const * const ctitle   = mpd_song_get_tag(currentSong_, MPD_TAG_TITLE, 0);
+         char const * const cname    = mpd_song_get_tag(currentSong_, MPD_TAG_NAME, 0);
          char const * const curi     = mpd_song_get_uri(currentSong_);
          uint32_t     const duration = mpd_song_get_duration(currentSong_);
          uint32_t     const elapsed  = elapsed_;
          uint32_t     const remain   = (duration > elapsed) ? duration - elapsed : 0;
          std::string  const artist   = (cartist != NULL) ? cartist : "Unknown";
          std::string  const title    = (ctitle != NULL) ? ctitle : "";
+         std::string  const name     = (cname != NULL) ? cname : "";
          std::string  const uri      = (curi != NULL) ? curi : "";

          if (title != "")
          {
             snprintf(titleStr, 512, "%s - %s", artist.c_str(), title.c_str());
          }
+         else if (name != "")
+         {
+            snprintf(titleStr, 512, "%s", name.c_str());
+         }
          else
          {
             snprintf(titleStr, 512, "%s", uri.c_str());
diff --git a/src/mpdclient.cpp b/src/mpdclient.cpp
index a4516ca..b5a09d0 100644
--- a/src/mpdclient.cpp
+++ b/src/mpdclient.cpp
@@ -2133,7 +2133,7 @@ void Client::GetAllMetaInformation()
       {
          EventData Data; Data.song = NULL; Data.uri = mpd_song_get_uri(nextSong); Data.pos1 = -1;

-         if (((settings_.Get(Setting::ListAllMeta) == false) &&
+         if (((settings_.Get(Setting::ListAllMeta) == false) ||
               (Main::Library().Song(Data.uri) == NULL)) ||
              // Handle "virtual" songs embedded within files
              (mpd_song_get_end(nextSong) != 0))
@@ -2512,6 +2512,8 @@ Song * Client::CreateSong(mpd_song const * const song) const
    newSong->SetAlbum      (mpd_song_get_tag(song, MPD_TAG_ALBUM,  0));
    newSong->SetTitle      (mpd_song_get_tag(song, MPD_TAG_TITLE,  0));
    newSong->SetTrack      (mpd_song_get_tag(song, MPD_TAG_TRACK,  0));
+   newSong->SetName       (mpd_song_get_tag(song, MPD_TAG_NAME,    0));
+   newSong->SetComment    (mpd_song_get_tag(song, MPD_TAG_COMMENT, 0));
    newSong->SetURI        (mpd_song_get_uri(song));
    newSong->SetGenre      (mpd_song_get_tag(song, MPD_TAG_GENRE, 0));
    newSong->SetDate       (mpd_song_get_tag(song, MPD_TAG_DATE, 0));
@@ -2552,7 +2554,7 @@ void Client::QueueMetaChanges()
          {
             Song * newSong = NULL;

-            if (((settings_.Get(Setting::ListAllMeta) == false) &&
+            if (((settings_.Get(Setting::ListAllMeta) == false) ||
                  (Main::Library().Song(Data.uri) == NULL)) ||
                 // Handle "virtual" songs embedded within files
                 (mpd_song_get_end(nextSong) != 0))
diff --git a/src/settings.hpp b/src/settings.hpp
index d6dbd7c..2ca18d3 100644
--- a/src/settings.hpp
+++ b/src/settings.hpp
@@ -88,7 +88,7 @@
    /* Lists to show in the lists window */ \
    X(Playlists,        "playlists", "mpd", "all|mpd|files") \
    /* Song format string */ \
-   X(SongFormat,       "songformat", "{{%a - }%t}|{%f}$E$R $H[$H%l$H]$H", ".*") \
+   X(SongFormat,       "songformat", "{{%a - }%t}|{%N}|{%f}$E$R $H[$H%l$H]$H", ".*") \
    /* Song format fill character */ \
    X(SongFillChar,       "songfillchar", " ", ".") \
    /* Sort based on song format */ \
diff --git a/src/song.cpp b/src/song.cpp
index f97fa38..5296814 100644
--- a/src/song.cpp
+++ b/src/song.cpp
@@ -26,14 +26,16 @@
 #include "buffer/directory.hpp"
 #include "buffer/library.hpp"

-const std::string UnknownArtist = "Unknown Artist";
-const std::string UnknownAlbum  = "Unknown Album";
-const std::string UnknownTitle  = "Unknown";
-const std::string UnknownURI    = "Unknown";
-const std::string UnknownGenre  = "Unknown";
-const std::string UnknownTrack  = "";
-const std::string UnknownDate   = "Unknown";
-const std::string UnknownDisc   = "";
+const std::string UnknownArtist  = "Unknown Artist";
+const std::string UnknownAlbum   = "Unknown Album";
+const std::string UnknownTitle   = "Unknown";
+const std::string UnknownURI     = "Unknown";
+const std::string UnknownGenre   = "Unknown";
+const std::string UnknownTrack   = "";
+const std::string UnknownName    = "Unknown";
+const std::string UnknownComment = "Unknown";
+const std::string UnknownDate    = "Unknown";
+const std::string UnknownDisc    = "";

 std::vector<std::string> Mpc::Song::Artists;
 std::map<std::string, uint32_t> Mpc::Song::ArtistMap;
@@ -70,6 +72,8 @@ Song::Song() :
    virtualEnd_  (0),
    uri_         (""),
    title_       (""),
+   name_        (""),
+   comment_     (""),
    lastFormat_  (""),
    formatted_   (""),
    entry_       (NULL)
@@ -87,6 +91,8 @@ Song::Song(Song const & song) :
    duration_    (song.duration_),
    uri_         (song.URI()),
    title_       (song.Title()),
+   name_        (song.Name()),
+   comment_     (song.Comment()),
    lastFormat_  (song.lastFormat_),
    formatted_   (song.formatted_)
 {
@@ -251,6 +257,44 @@ std::string const & Song::Title() const
    return title_;
 }

+void Song::SetName(const char * name)
+{
+   lastFormat_ = "";
+
+   if (name != NULL)
+   {
+      name_ = name;
+   }
+   else
+   {
+      name_ = UnknownName;
+   }
+}
+
+std::string const & Song::Name() const
+{
+   return name_;
+}
+
+void Song::SetComment(const char * comment)
+{
+   lastFormat_ = "";
+
+   if (comment != NULL)
+   {
+      comment_ = comment;
+   }
+   else
+   {
+      comment_ = UnknownComment;
+   }
+}
+
+std::string const & Song::Comment() const
+{
+   return comment_;
+}
+
 void Song::SetTrack(const char * track)
 {
    Set(track, track_, Tracks, TrackMap);
@@ -431,10 +475,12 @@ std::string Song::FormatString(std::string fmt) const
    SongInfo['t'] = &Mpc::Song::Title;
    SongInfo['n'] = &Mpc::Song::Track;
    SongInfo['N'] = &Mpc::Song::ZeroPaddedTrack;
+   SongInfo['P'] = &Mpc::Song::Name;
    SongInfo['f'] = &Mpc::Song::URI;
    SongInfo['d'] = &Mpc::Song::Date;
    SongInfo['y'] = &Mpc::Song::Year;
    SongInfo['c'] = &Mpc::Song::Disc;
+   SongInfo['C'] = &Mpc::Song::Comment;

    SongInfo['r'] = &Mpc::Song::Artist;
    SongInfo['R'] = &Mpc::Song::Artist;
@@ -457,6 +503,7 @@ std::string Song::ParseString(std::string::const_iterator & it, bool valid) cons
 {
    std::string result;
    bool tmp_valid = false;
+   bool parse_next = true;

    if (SongInfo.empty() == true)
    {
@@ -475,15 +522,18 @@ std::string Song::ParseString(std::string::const_iterator & it, bool valid) cons
       {
          *++it;
          uint32_t len = result.length();
-         result += ParseString(it, valid);
+         result += ParseString(it, valid && parse_next);

-         if (result.length() != len)
+         if (parse_next)
          {
-             tmp_valid = true;
-         }
-         else
-         {
-             tmp_valid = false;
+            if (result.length() != len)
+            {
+                tmp_valid = true;
+            }
+            else
+            {
+                tmp_valid = false;
+            }
          }
       }
       else if (*it == '}')
@@ -494,8 +544,7 @@ std::string Song::ParseString(std::string::const_iterator & it, bool valid) cons
       }
       else if (*it == '|')
       {
-         valid = (valid) ? tmp_valid : valid;
-         valid = !valid;
+         parse_next = !tmp_valid;
       }
       else if (*it == '%')
       {
@@ -509,8 +558,9 @@ std::string Song::ParseString(std::string::const_iterator & it, bool valid) cons
                   (*it == 'r') || (*it == 'R') ||
                   (*it == 'm') || (*it == 'M') ||
                   (*it == 'l') || (*it == 't') ||
-                  (*it == 'n') || (*it == 'f') ||
-                  (*it == 'd') || (*it == 'c') ||
+                  (*it == 'n') || (*it == 'P') ||
+                  (*it == 'f') || (*it == 'd') ||
+                  (*it == 'c') || (*it == 'C') ||
                   (*it == 'y') || (*it == 'N'))
          {
             SongFunction Function = SongInfo[*it];
diff --git a/src/song.hpp b/src/song.hpp
index d4b9a08..bb8d2eb 100644
--- a/src/song.hpp
+++ b/src/song.hpp
@@ -108,6 +108,12 @@ namespace Mpc
       void SetTitle(const char * title);
       std::string const & Title() const;

+      void SetName(const char * name);
+      std::string const & Name() const;
+
+      void SetComment(const char * comment);
+      std::string const & Comment() const;
+
       void SetTrack(const char * track);
       std::string const & Track() const;
       std::string const & ZeroPaddedTrack() const;
@@ -178,6 +184,8 @@ namespace Mpc
       int32_t     virtualEnd_;
       std::string uri_;
       std::string title_;
+      std::string name_;
+      std::string comment_;

       mutable std::string lastFormat_;
       mutable std::string formatted_;
diff --git a/src/window/infowindow.cpp b/src/window/infowindow.cpp
index 7c6fb66..c3346ea 100644
--- a/src/window/infowindow.cpp
+++ b/src/window/infowindow.cpp
@@ -84,32 +84,42 @@ void InfoWindow::Print(uint32_t line) const
          wprintw(window, "%s", song->Title().c_str());

          wattron(window, A_BOLD);
-         mvwaddstr(window, 8, 0, " Duration    : ");
+         mvwaddstr(window, 8, 0, " Name        : ");
+         wattroff(window, A_BOLD);
+         wprintw(window, "%s", song->Name().c_str());
+
+         wattron(window, A_BOLD);
+         mvwaddstr(window, 9, 0, " Duration    : ");
          wattroff(window, A_BOLD);
          wprintw(window, "%d:%.2d", Mpc::SecondsToMinutes(song->Duration()), Mpc::RemainingSeconds(song->Duration()));

          wattron(window, A_BOLD);
-         mvwaddstr(window, 9, 0, " Genre       : ");
+         mvwaddstr(window, 10, 0, " Genre       : ");
          wattroff(window, A_BOLD);
          wprintw(window, "%s", song->Genre().c_str());

          wattron(window, A_BOLD);
-         mvwaddstr(window, 10, 0, " Date        : ");
+         mvwaddstr(window, 11, 0, " Comment     : ");
+         wattroff(window, A_BOLD);
+         wprintw(window, "%s", song->Comment().c_str());
+
+         wattron(window, A_BOLD);
+         mvwaddstr(window, 12, 0, " Date        : ");
          wattroff(window, A_BOLD);
          wprintw(window, "%s", song->Date().c_str());

          wattron(window, A_BOLD);
-         mvwaddstr(window, 11, 0, " Disc        : ");
+         mvwaddstr(window, 13, 0, " Disc        : ");
          wattroff(window, A_BOLD);
          wprintw(window, "%s", song->Disc().c_str());

          wattron(window, A_BOLD);
-         mvwaddstr(window, 13, 0, " Playlist    : ");
+         mvwaddstr(window, 14, 0, " Playlist    : ");
          wattroff(window, A_BOLD);
          wprintw(window, "%s", (song->Reference() > 0) ? "Yes" : "No");

          wattron(window, A_BOLD);
-         mvwaddstr(window, 14, 0, " Position    : ");
+         mvwaddstr(window, 15, 0, " Position    : ");
          wattroff(window, A_BOLD);
          wprintw(window, "%d", Main::Playlist().Index(song) + 1);