Script 'mail_helper' called by obssrc
Hello community,

here is the log from the commit of package upmpdcli for openSUSE:Factory 
checked in at 2021-01-20 18:26:55
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comparing /work/SRC/openSUSE:Factory/upmpdcli (Old)
 and      /work/SRC/openSUSE:Factory/.upmpdcli.new.28504 (New)
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Package is "upmpdcli"

Wed Jan 20 18:26:55 2021 rev:4 rq:864367 version:1.5.8

Changes:
--------
--- /work/SRC/openSUSE:Factory/upmpdcli/upmpdcli.changes        2021-01-01 
21:13:59.663405218 +0100
+++ /work/SRC/openSUSE:Factory/.upmpdcli.new.28504/upmpdcli.changes     
2021-01-20 18:27:21.911514462 +0100
@@ -1,0 +2,8 @@
+Tue Jan 19 11:14:55 UTC 2021 - Michael Pujos <pujos.mich...@gmail.com>
+
+- update to 1.5.8:
+  * Fix a number of memory leak and locking issues detected by 
valgrind/helgrind.
+  * Improve performance of big playlist insert by rate-limiting events.
+  * Fix possible crash when started while mpd is not running.
+
+-------------------------------------------------------------------

Old:
----
  upmpdcli-1.5.7.tar.gz

New:
----
  upmpdcli-1.5.8.tar.gz

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Other differences:
------------------
++++++ upmpdcli.spec ++++++
--- /var/tmp/diff_new_pack.WKcHnS/_old  2021-01-20 18:27:23.639516059 +0100
+++ /var/tmp/diff_new_pack.WKcHnS/_new  2021-01-20 18:27:23.639516059 +0100
@@ -1,7 +1,7 @@
 #
 # spec file for package upmpdcli
 #
-# Copyright (c) 2020 SUSE LLC
+# Copyright (c) 2021 SUSE LLC
 #
 # All modifications and additions to the file contributed by third parties
 # remain the property of their copyright owners, unless otherwise agreed
@@ -17,7 +17,7 @@
 
 
 Name:           upmpdcli
-Version:        1.5.7
+Version:        1.5.8
 Release:        0
 Summary:        UPnP Media Renderer front-end to MPD, the Music Player Daemon
 License:        GPL-2.0-or-later

++++++ upmpdcli-1.5.7.tar.gz -> upmpdcli-1.5.8.tar.gz ++++++
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/configure new/upmpdcli-1.5.8/configure
--- old/upmpdcli-1.5.7/configure        2020-12-30 15:29:57.000000000 +0100
+++ new/upmpdcli-1.5.8/configure        2021-01-10 19:28:33.000000000 +0100
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.69 for upmpdcli 1.5.7.
+# Generated by GNU Autoconf 2.69 for upmpdcli 1.5.8.
 #
 # Report bugs to <j...@lesbonscomptes.com>.
 #
@@ -580,8 +580,8 @@
 # Identity of this package.
 PACKAGE_NAME='upmpdcli'
 PACKAGE_TARNAME='upmpdcli'
-PACKAGE_VERSION='1.5.7'
-PACKAGE_STRING='upmpdcli 1.5.7'
+PACKAGE_VERSION='1.5.8'
+PACKAGE_STRING='upmpdcli 1.5.8'
 PACKAGE_BUGREPORT='j...@lesbonscomptes.com'
 PACKAGE_URL='http://www.lesbonscomptes.com/upmpdcli'
 
@@ -1331,7 +1331,7 @@
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures upmpdcli 1.5.7 to adapt to many kinds of systems.
+\`configure' configures upmpdcli 1.5.8 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1398,7 +1398,7 @@
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of upmpdcli 1.5.7:";;
+     short | recursive ) echo "Configuration of upmpdcli 1.5.8:";;
    esac
   cat <<\_ACEOF
 
@@ -1523,7 +1523,7 @@
 test -n "$ac_init_help" && exit $ac_status
 if $ac_init_version; then
   cat <<\_ACEOF
-upmpdcli configure 1.5.7
+upmpdcli configure 1.5.8
 generated by GNU Autoconf 2.69
 
 Copyright (C) 2012 Free Software Foundation, Inc.
@@ -1826,7 +1826,7 @@
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by upmpdcli $as_me 1.5.7, which was
+It was created by upmpdcli $as_me 1.5.8, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   $ $0 $@
@@ -2696,7 +2696,7 @@
 
 # Define the identity of the package.
  PACKAGE='upmpdcli'
- VERSION='1.5.7'
+ VERSION='1.5.8'
 
 
 # Some tools Automake needs.
@@ -6136,7 +6136,7 @@
 # report actual input values of CONFIG_FILES etc. instead of their
 # values after options handling.
 ac_log="
-This file was extended by upmpdcli $as_me 1.5.7, which was
+This file was extended by upmpdcli $as_me 1.5.8, which was
 generated by GNU Autoconf 2.69.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -6203,7 +6203,7 @@
 cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
 ac_cs_config="`$as_echo "$ac_configure_args" | sed 's/^ //; 
s/[\\""\`\$]/\\\\&/g'`"
 ac_cs_version="\\
-upmpdcli config.status 1.5.7
+upmpdcli config.status 1.5.8
 configured by $0, generated by GNU Autoconf 2.69,
   with options \\"\$ac_cs_config\\"
 
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/configure.ac 
new/upmpdcli-1.5.8/configure.ac
--- old/upmpdcli-1.5.7/configure.ac     2020-12-30 15:27:16.000000000 +0100
+++ new/upmpdcli-1.5.8/configure.ac     2021-01-10 19:26:19.000000000 +0100
@@ -1,4 +1,4 @@
-AC_INIT([upmpdcli], [1.5.7], [j...@lesbonscomptes.com],
+AC_INIT([upmpdcli], [1.5.8], [j...@lesbonscomptes.com],
              [upmpdcli], [http://www.lesbonscomptes.com/upmpdcli])
 AC_PREREQ([2.53])
 AC_CONFIG_MACRO_DIRS([m4])
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/main.cxx 
new/upmpdcli-1.5.8/src/main.cxx
--- old/upmpdcli-1.5.7/src/main.cxx     2020-12-30 14:28:30.000000000 +0100
+++ new/upmpdcli-1.5.8/src/main.cxx     2021-01-10 19:14:33.000000000 +0100
@@ -167,6 +167,10 @@
 {
     LOGDEB("Got sig" << endl);
     for (auto& dev : devs) {
+        // delete has a tendancy to crash (it works most of the time
+        // though). Anyway, we're exiting, so just call shouldExit()
+        // which will send the byebyes.
+        //delete dev;
         dev->shouldExit();
     }
     if (mpdclip) {
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' 
old/upmpdcli-1.5.7/src/mediaserver/cdplugins/uprcl/uprcl-app.py 
new/upmpdcli-1.5.8/src/mediaserver/cdplugins/uprcl/uprcl-app.py
--- old/upmpdcli-1.5.7/src/mediaserver/cdplugins/uprcl/uprcl-app.py     
2020-12-30 14:30:49.000000000 +0100
+++ new/upmpdcli-1.5.8/src/mediaserver/cdplugins/uprcl/uprcl-app.py     
2021-01-10 19:25:54.000000000 +0100
@@ -127,6 +127,8 @@
             elif not idpath:
                 entries = _rootentries()
             else:
+                if len(rootmap) == 0:
+                    _rootentries()
                 entries = _browsedispatch(objid, bflg, offset, count)
         finally:
             uprclinit.g_dblock.release_read()
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' 
old/upmpdcli-1.5.7/src/mediaserver/cdplugins/uprcl/uprcltags.py 
new/upmpdcli-1.5.8/src/mediaserver/cdplugins/uprcl/uprcltags.py
--- old/upmpdcli-1.5.7/src/mediaserver/cdplugins/uprcl/uprcltags.py     
2020-12-30 14:30:49.000000000 +0100
+++ new/upmpdcli-1.5.8/src/mediaserver/cdplugins/uprcl/uprcltags.py     
2021-01-10 19:25:54.000000000 +0100
@@ -136,6 +136,7 @@
     # useful anyway.
     def _trackentriesforstmt(self, stmt, values, pid, offset=0, count=0):
         uplog("trackentries: offset %d count %d" % (offset, count))
+        total = 0
         if offset != 0 or count != 0:
             total = self._stmt_total(stmt, values)
         if total < 1000:
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/mpdcli.cxx 
new/upmpdcli-1.5.8/src/mpdcli.cxx
--- old/upmpdcli-1.5.7/src/mpdcli.cxx   2020-12-30 14:43:57.000000000 +0100
+++ new/upmpdcli-1.5.8/src/mpdcli.cxx   2021-01-10 19:14:33.000000000 +0100
@@ -45,11 +45,6 @@
     std::unique_lock<std::mutex> lock(m_mutex);
 
     regcomp(&m_tpuexpr, "^[[:alpha:]]+://.+", REG_EXTENDED|REG_NOSUB);
-    if (!openconn()) {
-        return;
-    }
-    m_have_addtagid = checkForCommand("addtagid");
-
     g_config->get("onstart", m_onstart);
     g_config->get("onplay", m_onplay);
     g_config->get("onpause", m_onpause);
@@ -61,6 +56,11 @@
     stringToStrings(scratch, m_getexternalvolume);
     m_timeoutms = g_config->getInt("mpdtimeoutms", 2000);
     m_externalvolumecontrol = g_config->getBool("externalvolumecontrol", 
false);
+
+    if (!openconn()) {
+        return;
+    }
+    m_have_addtagid = checkForCommand("addtagid");
     updStatus();
 }
 
@@ -109,7 +109,9 @@
     }
 
     if (mpd_connection_get_error(m_conn) != MPD_ERROR_SUCCESS) {
-        showError("MPDCli::openconn");
+        // Don't call showError() from here it calls us back !
+        LOGERR("MPDCli::openconn: mpd_connection_new failed: "
+               <<  mpd_connection_get_error_message(m_conn) << "\n");
         closeconn();
         return false;
     }
@@ -136,6 +138,7 @@
 bool MPDCli::startEventLoop()
 {
     LOGDEB("MPDCli::startEventLoop\n");
+    std::lock_guard<std::mutex> lck(m_idlemutex);
     if (nullptr == m_idleconn) {
         if (m_idlethread.joinable()) {
             m_idlethread.join();
@@ -150,8 +153,10 @@
 void MPDCli::stopEventLoop()
 {
     LOGDEB("MPDCli::stopEventLoop\n");
+    std::unique_lock<std::mutex> lck(m_idlemutex);
     if (m_idleconn) {
         mpd_send_noidle(m_idleconn);
+        lck.unlock();
         m_idlethread.join();
         m_idlethread = std::thread{};
     }
@@ -197,16 +202,21 @@
 
 bool MPDCli::eventLoop()
 {
-    m_idleconn = mpd_connection_new(m_host.c_str(), m_port, m_timeoutms);
-    if (nullptr == m_idleconn) {
-        LOGERR("MPDCli::eventloop: could not open connection\n");
-        return false;
+    {
+        std::lock_guard<std::mutex> lck(m_idlemutex);
+        m_idleconn = mpd_connection_new(m_host.c_str(), m_port, m_timeoutms);
+        if (nullptr == m_idleconn) {
+            LOGERR("MPDCli::eventloop: could not open connection\n");
+            return false;
+        }
     }
+    MpdStatus::State st;
     {
         std::unique_lock<std::mutex> lock(m_mutex);
         updStatus();
+        st = m_stat.state;
     }
-    pollerCtl(m_stat.state);
+    pollerCtl(st);
     for (;;) {
         enum mpd_idle mask = 
             mpd_run_idle_mask(m_idleconn, (enum mpd_idle)o_idle_mask);
@@ -219,22 +229,29 @@
         {
             std::unique_lock<std::mutex> lock(m_mutex);
             updStatus();
+            st = m_stat.state;
         }
-        pollerCtl(m_stat.state);
+        pollerCtl(st);
+
         {
-            std::unique_lock<std::mutex> lock(m_callbackmutex);
+            std::lock_guard<std::mutex> lock(m_callbackmutex);
             for (auto& sub : m_subs) {
                 if (sub.first & mask) {
                     sub.second(&m_stat);
                 }
             }
         }
+        // Rate-limiting the events we trigger improves big list
+        // insertion performance a lot because it decreases the number
+        // of times we have to update the list
+        std::this_thread::sleep_for(std::chrono::milliseconds(1000));
     }
+    
     pollerCtl(MpdStatus::MPDS_STOP);
-    if (m_idleconn) {
-        mpd_connection_free(m_idleconn);
-        m_idleconn = nullptr;
-    }
+
+    std::lock_guard<std::mutex> lck(m_idlemutex);
+    mpd_connection_free(m_idleconn);
+    m_idleconn = nullptr;
     return false;
 }
 
@@ -247,7 +264,7 @@
             updStatus();
         }
         {
-            std::unique_lock<std::mutex> lock(m_callbackmutex);
+            std::lock_guard<std::mutex> lock(m_callbackmutex);
             for (auto& sub : m_subs) {
                 if (sub.first & MpdPlayerEvt) {
                     sub.second(&m_stat);
@@ -272,6 +289,7 @@
 
 bool MPDCli::subscribe(int mask, evtFunc func)
 {
+    std::lock_guard<std::mutex> lock(m_callbackmutex);
     m_subs.push_back({mask, func});
     return true;
 }
@@ -500,11 +518,12 @@
     return found;
 }
 
-const MpdStatus& MPDCli::getStatus()
+MpdStatus MPDCli::getStatus()
 {
     std::unique_lock<std::mutex> lock(m_mutex);
     // We used to updstatus() here, but the status should now be
     // always up to date because of the idle loop.
+    // We have to return a copy, as we are going to release the lock.
     return m_stat;
 }
 
@@ -1056,7 +1075,8 @@
     }
     
     if (!mpd_response_finish(m_conn)) {
-        LOGERR("MPDCli::getQueueSongs: mpd_list_queue_meta failed"<< endl);
+        showError("MPDCli::getQueueSongs: mpd_list_queue_meta:finish");
+        freeSongs(songs);
         return false;
     }
     LOGDEB("MPDCli::getQueueSongs: " << songs.size() << " songs " << endl);
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/mpdcli.hxx 
new/upmpdcli-1.5.8/src/mpdcli.hxx
--- old/upmpdcli-1.5.7/src/mpdcli.hxx   2020-12-29 10:48:33.000000000 +0100
+++ new/upmpdcli-1.5.8/src/mpdcli.hxx   2021-01-10 19:14:33.000000000 +0100
@@ -27,6 +27,7 @@
 #include <thread>
 #include <mutex>
 #include <condition_variable>
+#include <atomic>
 
 #include "upmpdutils.hxx"
 
@@ -109,7 +110,7 @@
     bool getQueueData(std::vector<UpSong>& vdata);
     bool statSong(UpSong& usong, int pos = -1, bool isId = false);
     UpSong& mapSong(UpSong& usong, struct mpd_song *song);
-    const MpdStatus& getStatus();
+    MpdStatus getStatus();
     // Copy complete mpd state. If seekms is > 0, this is the value to
     // save (sometimes useful if mpd was stopped)
     bool saveState(MpdState& st, int seekms = 0);
@@ -146,6 +147,9 @@
     struct mpd_connection *m_conn{nullptr};
     // Connection for the event loop
     struct mpd_connection *m_idleconn{nullptr};
+    // Protect idleconn: We need to use it from the main thread to
+    // tell the idle thread to stop.
+    std::mutex m_idlemutex;
     // thread to listen to MPD events.
     std::thread m_idlethread;
     // MPD does not report idle events for play time change. We poll
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/ohinfo.cxx 
new/upmpdcli-1.5.8/src/ohinfo.cxx
--- old/upmpdcli-1.5.7/src/ohinfo.cxx   2020-12-30 14:28:30.000000000 +0100
+++ new/upmpdcli-1.5.8/src/ohinfo.cxx   2021-01-10 19:14:33.000000000 +0100
@@ -87,7 +87,7 @@
 void OHInfo::makedetails(string &duration, string& bitrate, 
                          string& bitdepth, string& samplerate)
 {
-    const MpdStatus &mpds =  m_dev->getMpdStatus();
+    const MpdStatus mpds =  m_dev->getMpdStatus();
 
     bool is_song = (mpds.state == MpdStatus::MPDS_PLAY) || 
         (mpds.state == MpdStatus::MPDS_PAUSE);
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/ohmetacache.cxx 
new/upmpdcli-1.5.8/src/ohmetacache.cxx
--- old/upmpdcli-1.5.7/src/ohmetacache.cxx      2020-12-19 09:42:12.000000000 
+0100
+++ new/upmpdcli-1.5.8/src/ohmetacache.cxx      2021-01-10 19:14:33.000000000 
+0100
@@ -47,6 +47,11 @@
 };
 static WorkQueue<SaveCacheTask*> saveQueue("SaveQueue");
 
+void freeSaveCacheTask(SaveCacheTask*& t)
+{
+    delete t;
+}
+
 // Encode uris and values so that they can be decoded (escape %, =, and eol)
 static string encode(const string& in)
 {
@@ -169,6 +174,7 @@
 {
     // Restore is called once at startup, so seize the opportunity to start the
     // save thread
+    saveQueue.setTaskFreeFunc(freeSaveCacheTask);
     if (!saveQueue.start(1, dmcacheSaveWorker, 0)) {
         LOGERR("dmcacheRestore: could not start save thread" << endl);
         return false;
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/ohplaylist.cxx 
new/upmpdcli-1.5.8/src/ohplaylist.cxx
--- old/upmpdcli-1.5.7/src/ohplaylist.cxx       2020-12-30 14:28:30.000000000 
+0100
+++ new/upmpdcli-1.5.8/src/ohplaylist.cxx       2021-01-10 19:14:33.000000000 
+0100
@@ -129,7 +129,14 @@
 static string translateIdArray(const vector<UpSong>& in)
 {
     string out1;
+    out1.reserve(in.size() * 4);
+
+#undef TIA_SHOW_PLAIN_ARRAY
+#ifdef TIA_SHOW_PLAIN_ARRAY
     string sdeb;
+    sdeb.reserve(in.size() * 7);
+#endif // TIA_SHOW_PLAIN_ARRAY
+    
     for (auto us = in.begin(); us != in.end(); us++) {
         unsigned int val = us->mpdid;
         if (val) {
@@ -138,15 +145,19 @@
             out1 += (unsigned char) ((val & 0x0000ff00) >> 8);
             out1 += (unsigned char) ((val & 0x000000ff));
         }
+#ifdef TIA_SHOW_PLAIN_ARRAY
         sdeb += SoapHelp::i2s(val) + " ";
+#endif
     }
-    LOGDEB("OHPlaylist::translateIdArray: current ids: " << sdeb << endl);
+#ifdef TIA_SHOW_PLAIN_ARRAY
+    LOGINF("OHPlaylist::translateIdArray: current ids: " << sdeb << endl);
+#endif
     return base64_encode(out1);
 }
 
 bool OHPlaylist::makeIdArray(string& out)
 {
-    //LOGDEB1("OHPlaylist::makeIdArray\n");
+    LOGDEB1("OHPlaylist::makeIdArray\n");
     const MpdStatus &mpds = m_dev->getMpdStatus();
 
     if (mpds.qvers == m_mpdqvers) {
@@ -298,26 +309,15 @@
     return true;
 }
 
-void OHPlaylist::refreshState()
-{
-    m_mpdqvers = -1;
-    unordered_map<string, string> st;
-    makestate(st);
-}
-
 void OHPlaylist::setActive(bool onoff)
 {
     if (onoff) {
         m_dev->getmpdcli()->clearQueue();
         m_dev->getmpdcli()->restoreState(m_mpdsavedstate);
-        if (!keepconsume)
-            m_dev->getmpdcli()->consume(false);
-        m_dev->getmpdcli()->single(false);
-        refreshState();
         onEvent(nullptr);
         m_active = true;
     } else {
-        m_mpdqvers = -1;
+        std::lock_guard<std::mutex> lock(m_statemutex);
         makestate(m_upnpstate);
         m_dev->getmpdcli()->saveState(m_mpdsavedstate);
         iStop();
@@ -555,6 +555,7 @@
 
 bool OHPlaylist::cacheFind(const string& uri, string& meta)
 {
+    std::lock_guard<std::mutex> lock(m_statemutex);
     auto cached = m_metacache.find(uri);
     if (cached != m_metacache.end()) {
         meta = cached->second;
@@ -567,6 +568,7 @@
 
 bool OHPlaylist::cacheSet(const string& uri, const string& meta)
 {
+    std::lock_guard<std::mutex> lock(m_statemutex);
     LOGDEB1("OHPlaylist::cacheSet: " << uri << " -> " << meta << "\n");
     m_metacache[uri] = meta;
     m_cachedirty = true;
@@ -577,7 +579,6 @@
 // Returns a 800 fault code if the given id is not in the playlist. 
 int OHPlaylist::ohread(const SoapIncoming& sc, SoapOutgoing& data)
 {
-    std::unique_lock<std::mutex> lock(m_statemutex);
     int id;
     bool ok = sc.get("Id", &id);
     if (!ok) {
@@ -628,7 +629,6 @@
 // Any ids not in the playlist are ignored. 
 int OHPlaylist::readList(const SoapIncoming& sc, SoapOutgoing& data)
 {
-    std::unique_lock<std::mutex> lock(m_statemutex);
     string sids;
     bool ok = sc.get("IdList", &sids);
     LOGDEB("OHPlaylist::readList: [" << sids << "]" << endl);
@@ -684,20 +684,6 @@
     return ok ? UPNP_E_SUCCESS : UPNP_E_INTERNAL_ERROR;
 }
 
-bool OHPlaylist::ireadList(const vector<int>& ids, vector<UpSong>& songs)
-{
-    for (auto it = ids.begin(); it != ids.end(); it++) {
-        UpSong song;
-        if (!m_dev->getmpdcli()->statSong(song, *it, true)) {
-            LOGDEB("OHPlaylist::readList:stat failed for " << *it << endl);
-            continue;
-        }
-        songs.push_back(song);
-    }
-    return true;
-}
-
-
 // Adds the given uri and metadata as a new track to the playlist. 
 // Set the AfterId argument to 0 to insert a track at the start of the
 // playlist.
@@ -708,7 +694,6 @@
 int OHPlaylist::insert(const SoapIncoming& sc, SoapOutgoing& data)
 {
     LOGDEB("OHPlaylist::insert" << endl);
-    std::unique_lock<std::mutex> lock(m_statemutex);
     int afterid;
     string uri, metadata;
     bool ok = sc.get("AfterId", &afterid);
@@ -771,7 +756,6 @@
 
     int id = m_dev->getmpdcli()->insertAfterId(uri, afterid, metaformpd);
     if (id != -1) {
-        m_mpdqvers = -1;
         if (newid)
             *newid = id;
         return true;
@@ -803,7 +787,6 @@
         m_dev->getmpdcli()->stop();
     }
     bool ok = m_dev->getmpdcli()->deleteId(id);
-    m_mpdqvers = -1;
     return ok ? UPNP_E_SUCCESS : UPNP_E_INTERNAL_ERROR;
 }
 
@@ -814,7 +797,6 @@
         m_udev->getohpr()->iSetSourceIndexByName("Playlist");
     }
     bool ok = m_dev->getmpdcli()->clearQueue();
-    m_mpdqvers = -1;
     return ok ? UPNP_E_SUCCESS : UPNP_E_INTERNAL_ERROR;
 }
 
@@ -825,12 +807,15 @@
     return UPNP_E_SUCCESS;
 }
 
-
+// Call with state lock held
 bool OHPlaylist::iidArray(string& idarray, int *token)
 {
     LOGDEB("OHPlaylist::idArray (internal)" << endl);
     unordered_map<string, string> st;
-    makestate(st);
+    {
+        std::lock_guard<std::mutex> lock(m_statemutex);
+        makestate(st);
+    }
     idarray = st["IdArray"];
     if (token) {
         if (m_active) {
@@ -860,9 +845,22 @@
     return UPNP_E_INTERNAL_ERROR;
 }
 
+bool OHPlaylist::ireadList(const vector<int>& ids, vector<UpSong>& songs)
+{
+    for (auto it = ids.begin(); it != ids.end(); it++) {
+        UpSong song;
+        if (!m_dev->getmpdcli()->statSong(song, *it, true)) {
+            LOGDEB("OHPlaylist::readList:stat failed for " << *it << endl);
+            continue;
+        }
+        songs.push_back(song);
+    }
+    return true;
+}
 
 bool OHPlaylist::urlMap(unordered_map<int, string>& umap)
 {
+    std::lock_guard<std::mutex> lock(m_statemutex);
     LOGDEB1("OHPlaylist::urlMap\n");
     string sarray; 
     if (iidArray(sarray, 0)) {
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/ohplaylist.hxx 
new/upmpdcli-1.5.8/src/ohplaylist.hxx
--- old/upmpdcli-1.5.7/src/ohplaylist.hxx       2020-12-30 14:28:30.000000000 
+0100
+++ new/upmpdcli-1.5.8/src/ohplaylist.hxx       2021-01-10 19:14:33.000000000 
+0100
@@ -76,7 +76,6 @@
     // Private internal non-soap versions of some of the interface +
     // utility methods
     bool makeIdArray(std::string&);
-    void refreshState();
     bool insertUri(int afterid, const std::string& uri, 
                    const std::string& metadata, int *newid, bool nocheck);
     bool ireadList(const std::vector<int>&, std::vector<UpSong>&);
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/ohservice.hxx 
new/upmpdcli-1.5.8/src/ohservice.hxx
--- old/upmpdcli-1.5.7/src/ohservice.hxx        2020-12-30 14:28:30.000000000 
+0100
+++ new/upmpdcli-1.5.8/src/ohservice.hxx        2021-01-10 19:14:33.000000000 
+0100
@@ -39,19 +39,39 @@
               const std::string& xmlfn, UpMpd *dev, UpMpdOpenHome *udev);
     virtual ~OHService() = default;
 
+    // This is called from the mpd event loop when something
+    // changes. It in turn calls getEventData() and sends the result
+    // to libupnpp::notifyEvent()
     virtual void onEvent(const MpdStatus*);
 
+    // Retrieve the service changed state data. This now normally
+    // called from onEvent(), bit it is kept as a separate entry,
+    // because, we could still also decide not to start the mpd idle
+    // loop and rely instead on polling from libupnpp.
     virtual bool getEventData(bool all, std::vector<std::string>& names, 
                               std::vector<std::string>& values);
+
+    // Translate mpd play/stop... state into openhome transportstate.
     static std::string mpdstatusToTransportState(MpdStatus::State st);
     
 protected:
+
+    // This is implemented by each service to return the current state
+    // of its variables.
     virtual bool makestate(std::unordered_map<std::string, std::string> &) = 0;
-    // State variable storage
+
+    // Storage for the state from the last makestate() call. Used by
+    // the services when reporting current state from actions, and for
+    // diffing with next makestate() values.
     std::unordered_map<std::string, std::string> m_state;
+
+    // Local state protection mutex. Held when calling makestate() from
+    // getEventData(), should also be held by the service code as
+    // appropriate.
+    std::mutex m_statemutex;
+
     UpMpd *m_dev;
     UpMpdOpenHome *m_udev;
-    std::mutex m_statemutex;
     std::string m_tpname;
 };
 
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/upmpd.cxx 
new/upmpdcli-1.5.8/src/upmpd.cxx
--- old/upmpdcli-1.5.7/src/upmpd.cxx    2020-12-30 14:28:30.000000000 +0100
+++ new/upmpdcli-1.5.8/src/upmpd.cxx    2021-01-10 19:14:33.000000000 +0100
@@ -250,7 +250,7 @@
 //    }
 }
 
-const MpdStatus& UpMpd::getMpdStatus()
+MpdStatus UpMpd::getMpdStatus()
 {
     return m_mpdcli->getStatus();
 }
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/upmpd.hxx 
new/upmpdcli-1.5.8/src/upmpd.hxx
--- old/upmpdcli-1.5.7/src/upmpd.hxx    2020-12-19 09:42:12.000000000 +0100
+++ new/upmpdcli-1.5.8/src/upmpd.hxx    2021-01-10 19:14:33.000000000 +0100
@@ -149,7 +149,7 @@
     void startloops();
     void startnoloops();
     
-    const MpdStatus& getMpdStatus();
+    MpdStatus getMpdStatus();
 
     const std::string& getMetaCacheFn() const {
         return m_mcachefn;
diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' 
'--exclude=.svnignore' old/upmpdcli-1.5.7/src/workqueue.h 
new/upmpdcli-1.5.8/src/workqueue.h
--- old/upmpdcli-1.5.7/src/workqueue.h  2020-12-19 09:42:12.000000000 +0100
+++ new/upmpdcli-1.5.8/src/workqueue.h  2021-01-10 19:14:33.000000000 +0100
@@ -72,6 +72,16 @@
         }
     }
 
+    /** Task deleter
+     * If put() is called with the flush option, and the tasks allocate memory,
+     * you need to set this function, which will be called on each task popped 
+     * from the queue. Tasks which go through normally must be freed by the 
+     * worker function.
+     */
+    void setTaskFreeFunc(void (*func)(T&)) {
+        m_taskfreefunc = func;
+    }
+
     /** Start the worker threads.
      *
      * @param nworkers number of threads copies to start.
@@ -120,6 +130,10 @@
         }
         if (flushprevious) {
             while (!m_queue.empty()) {
+                if (m_taskfreefunc) {
+                    T& d = m_queue.front();
+                    m_taskfreefunc(d);
+                }
                 m_queue.pop();
             }
         }
@@ -325,6 +339,7 @@
 #endif
     };
     
+    void (*m_taskfreefunc)(T&){nullptr};
     // Configuration
     std::string m_name;
     size_t m_high;

Reply via email to