Rework user rating updates

src/cache.c

@@ -1596,7 +1596,7 @@ cache_init(void)
 
   cmdbase = commands_base_new(evbase_cache, NULL);
 
-  ret = listener_add(cache_daap_listener_cb, LISTENER_DATABASE);
+  ret = listener_add(cache_daap_listener_cb, LISTENER_DATABASE | LISTENER_STICKER);
   if (ret < 0)
     {
       DPRINTF(E_LOG, L_CACHE, "Could not create listener event\n");

src/db.c

@@ -329,7 +329,7 @@ static enum group_type
 db_group_type_bypersistentid(int64_t persistentid);
 
 static int
-db_query_run(char *query, int free, int cache_update);
+db_query_run(char *query, int free, short update_events);
 
 
 char *
@@ -804,7 +804,7 @@ db_purge_cruft(time_t ref)
 
   DPRINTF(E_DBG, L_DB, "Running purge query '%s'\n", query);
 
-  ret = db_query_run(query, 1, 1);
+  ret = db_query_run(query, 1, LISTENER_DATABASE);
   if (ret == 0)
     DPRINTF(E_DBG, L_DB, "Purged %d rows\n", sqlite3_changes(hdl));
 
@@ -1496,7 +1496,7 @@ db_query_end(struct query_params *qp)
  * to update their cache of the library (and of course also of our own cache).
  */
 static int
-db_query_run(char *query, int free, int library_update)
+db_query_run(char *query, int free, short update_events)
 {
   char *errmsg;
   int changes = 0;
@@ -1527,8 +1527,8 @@ db_query_run(char *query, int free, int library_update)
 
   cache_daap_resume();
 
-  if (library_update && changes > 0)
-    library_update_trigger();
+  if (update_events && changes > 0)
+    library_update_trigger(update_events);
 
   return ((ret != SQLITE_OK) ? -1 : 0);
 }
@@ -2392,7 +2392,7 @@ db_file_add(struct media_file_info *mfi)
 
   sqlite3_free(query);
 
-  library_update_trigger();
+  library_update_trigger(LISTENER_DATABASE);
 
   return 0;
 
@@ -2471,7 +2471,7 @@ db_file_update(struct media_file_info *mfi)
 
   sqlite3_free(query);
 
-  library_update_trigger();
+  library_update_trigger(LISTENER_DATABASE);
 
   return 0;
 
@@ -2499,6 +2499,36 @@ db_file_seek_update(int id, uint32_t seek)
 #undef Q_TMPL
 }
 
+int
+db_file_rating_update_byid(uint32_t id, uint32_t rating)
+{
+#define Q_TMPL "UPDATE files SET rating = %d WHERE id = %d;"
+  char *query;
+  int ret;
+
+  query = sqlite3_mprintf(Q_TMPL, rating, id);
+
+  ret = db_query_run(query, 1, LISTENER_STICKER);
+
+  return ((ret < 0) ? -1 : sqlite3_changes(hdl));
+#undef Q_TMPL
+}
+
+int
+db_file_rating_update_byvirtualpath(const char *virtual_path, uint32_t rating)
+{
+#define Q_TMPL "UPDATE files SET rating = %d WHERE virtual_path = %Q;"
+  char *query;
+  int ret;
+
+  query = sqlite3_mprintf(Q_TMPL, rating, virtual_path);
+
+  ret = db_query_run(query, 1, LISTENER_STICKER);
+
+  return ((ret < 0) ? -1 : sqlite3_changes(hdl));
+#undef Q_TMPL
+}
+
 void
 db_file_delete_bypath(const char *path)
 {
@@ -2507,7 +2537,7 @@ db_file_delete_bypath(const char *path)
 
   query = sqlite3_mprintf(Q_TMPL, path);
 
-  db_query_run(query, 1, 1);
+  db_query_run(query, 1, LISTENER_DATABASE);
 #undef Q_TMPL
 }
 
@@ -2529,7 +2559,7 @@ db_file_disable_bypath(const char *path, char *strip, uint32_t cookie)
 
   query = sqlite3_mprintf(Q_TMPL, striplen, striplenvpath, disabled, path);
 
-  db_query_run(query, 1, 1);
+  db_query_run(query, 1, LISTENER_DATABASE);
 #undef Q_TMPL
 }
 
@@ -2551,7 +2581,7 @@ db_file_disable_bymatch(const char *path, char *strip, uint32_t cookie)
 
   query = sqlite3_mprintf(Q_TMPL, striplen, striplenvpath, disabled, path);
 
-  db_query_run(query, 1, 1);
+  db_query_run(query, 1, LISTENER_DATABASE);
 #undef Q_TMPL
 }
 
@@ -3161,7 +3191,7 @@ db_groups_cleanup()
 
   db_transaction_begin();
 
-  ret = db_query_run(Q_TMPL_ALBUM, 0, 1);
+  ret = db_query_run(Q_TMPL_ALBUM, 0, LISTENER_DATABASE);
   if (ret < 0)
     {
       db_transaction_rollback();
@@ -3170,7 +3200,7 @@ db_groups_cleanup()
 
   DPRINTF(E_DBG, L_DB, "Removed album group-entries: %d\n", sqlite3_changes(hdl));
 
-  ret = db_query_run(Q_TMPL_ARTIST, 0, 1);
+  ret = db_query_run(Q_TMPL_ARTIST, 0, LISTENER_DATABASE);
   if (ret < 0)
     {
       db_transaction_rollback();
@@ -3558,7 +3588,7 @@ db_directory_disable_bymatch(char *path, char *strip, uint32_t cookie)
 
   query = sqlite3_mprintf(Q_TMPL, striplen, disabled, path, path, path);
 
-  db_query_run(query, 1, 1);
+  db_query_run(query, 1, LISTENER_DATABASE);
 #undef Q_TMPL
 }
 
@@ -3571,7 +3601,7 @@ db_directory_enable_bycookie(uint32_t cookie, char *path)
 
   query = sqlite3_mprintf(Q_TMPL, path, (int64_t)cookie);
 
-  ret = db_query_run(query, 1, 1);
+  ret = db_query_run(query, 1, LISTENER_DATABASE);
 
   return ((ret < 0) ? -1 : sqlite3_changes(hdl));
 #undef Q_TMPL
@@ -3586,7 +3616,7 @@ db_directory_enable_bypath(char *path)
 
   query = sqlite3_mprintf(Q_TMPL, path);
 
-  ret = db_query_run(query, 1, 1);
+  ret = db_query_run(query, 1, LISTENER_DATABASE);
 
   return ((ret < 0) ? -1 : sqlite3_changes(hdl));
 #undef Q_TMPL
@@ -3695,7 +3725,7 @@ db_spotify_purge(void)
 
   for (i = 0; i < (sizeof(queries) / sizeof(queries[0])); i++)
     {
-      ret = db_query_run(queries[i], 0, 1);
+      ret = db_query_run(queries[i], 0, LISTENER_DATABASE);
 
       if (ret == 0)
 	DPRINTF(E_DBG, L_DB, "Processed %d rows\n", sqlite3_changes(hdl));
@@ -3708,7 +3738,7 @@ db_spotify_purge(void)
       DPRINTF(E_LOG, L_DB, "Out of memory for query string\n");
       return;
     }
-  ret = db_query_run(query, 1, 1);
+  ret = db_query_run(query, 1, LISTENER_DATABASE);
 
   if (ret == 0)
     DPRINTF(E_DBG, L_DB, "Disabled spotify directory\n");
@@ -3733,7 +3763,7 @@ db_spotify_pl_delete(int id)
     {
       query = sqlite3_mprintf(queries_tmpl[i], id);
 
-      ret = db_query_run(query, 1, 1);
+      ret = db_query_run(query, 1, LISTENER_DATABASE);
 
       if (ret == 0)
 	DPRINTF(E_DBG, L_DB, "Deleted %d rows\n", sqlite3_changes(hdl));
@@ -3750,7 +3780,7 @@ db_spotify_files_delete(void)
 
   query = sqlite3_mprintf(Q_TMPL);
 
-  ret = db_query_run(query, 1, 1);
+  ret = db_query_run(query, 1, LISTENER_DATABASE);
 
   if (ret == 0)
     DPRINTF(E_DBG, L_DB, "Deleted %d rows\n", sqlite3_changes(hdl));

src/db.h

@@ -559,6 +559,12 @@ db_file_update(struct media_file_info *mfi);
 void
 db_file_seek_update(int id, uint32_t seek);
 
+int
+db_file_rating_update_byid(uint32_t id, uint32_t rating);
+
+int
+db_file_rating_update_byvirtualpath(const char *virtual_path, uint32_t rating);
+
 void
 db_file_delete_bypath(const char *path);
 

src/httpd_dacp.c

@@ -1005,7 +1005,6 @@ dacp_propset_repeatstate(const char *value, struct evkeyvalq *query)
 static void
 dacp_propset_userrating(const char *value, struct evkeyvalq *query)
 {
-  struct media_file_info *mfi;
   const char *param;
   uint32_t itemid;
   uint32_t rating;
@@ -1052,35 +1051,31 @@ dacp_propset_userrating(const char *value, struct evkeyvalq *query)
       return;
     }
 
-  mfi = db_file_fetch_byid(itemid);
+  ret = db_file_rating_update_byid(itemid, rating);
 
   /* If no mfi, it may be because we sent an invalid nowplaying itemid. In this
    * case request the real one from the player and default to that.
    */
-  if (!mfi)
+  if (ret == 0)
     {
       DPRINTF(E_WARN, L_DACP, "Invalid id %d for rating, defaulting to player id\n", itemid);
 
       ret = player_now_playing(&itemid);
-      if ((ret < 0) || !(mfi = db_file_fetch_byid(itemid)))
+      if (ret < 0)
 	{
 	  DPRINTF(E_WARN, L_DACP, "Could not find an id for rating\n");
 
 	  return;
 	}
+
+      ret = db_file_rating_update_byid(itemid, rating);
+      if (ret <= 0)
+      	{
+      	  DPRINTF(E_WARN, L_DACP, "Could not find an id for rating\n");
+
+      	  return;
+      	}
     }
-
-  mfi->rating = rating;
-
-  /* rating is shared as MPD sticker `rating` */
-  listener_notify(LISTENER_STICKER);
-  
-  /* We're not touching any string field in mfi, so it's safe to
-   * skip unicode_fixup_mfi() before the update
-   */
-  db_file_update(mfi);
-
-  free_mfi(mfi, 0);
 }
 
 

src/library.c

@@ -93,6 +93,7 @@ static struct event *updateev;
 // Counts the number of changes made to the database between to DATABASE
 // event notifications
 static unsigned int deferred_update_notifications = 0;
+static short deferred_update_events = 0;
 
 static bool
 handle_deferred_update_notifications(void)
@@ -654,14 +655,18 @@ update_trigger_cb(int fd, short what, void *arg)
 {
   if (handle_deferred_update_notifications())
     {
-      listener_notify(LISTENER_DATABASE);
+      listener_notify(deferred_update_events);
+      deferred_update_events = 0;
     }
 }
 
 static enum command_state
 update_trigger(void *arg, int *retval)
 {
+  short *events = arg;
+
   ++deferred_update_notifications;
+  deferred_update_events |= *events;
 
   // Only add the timer event if the update occurred outside a (init-/re-/fullre-) scan.
   // The scanning functions take care of notifying clients of database changes directly
@@ -780,19 +785,22 @@ library_is_exiting()
  * is emitted with the delay 'library_update_wait'. It is safe to call this function from any thread.
  */
 void
-library_update_trigger(void)
+library_update_trigger(short update_events)
 {
+  short *events;
   int ret;
 
   pthread_t current_thread = pthread_self();
   if (pthread_equal(current_thread, tid_library))
     {
       // We are already running in the library thread, it is safe to directly call update_trigger
-      update_trigger(NULL, &ret);
+      update_trigger(&update_events, &ret);
     }
   else
     {
-      commands_exec_async(cmdbase, update_trigger, NULL);
+      events = malloc(sizeof(short));
+      *events = update_events;
+      commands_exec_async(cmdbase, update_trigger, events);
     }
 }
 

src/library.h

@@ -116,7 +116,7 @@ bool
 library_is_exiting();
 
 void
-library_update_trigger(void);
+library_update_trigger(short update_events);
 
 int
 library_playlist_add(const char *vp_playlist, const char *vp_item);

src/mpd.c

@@ -3423,9 +3423,6 @@ static struct mpd_sticker_command mpd_sticker_handlers[] = {
   { NULL, NULL, 0, 0, 0 },
 };
 
-static void
-mpd_add_idle_events(short event_mask);
-
 /*
  * Command handler function for 'sticker'
  *
@@ -3511,12 +3508,7 @@ mpd_command_sticker(struct evbuffer *evbuf, int argc, char **argv, char **errmsg
 
   if (ret == 0 && mfi && set_rating && mfi->rating != rating)
     {
-      DPRINTF(E_DBG, L_MPD, "STICKER notification for changed rating: %d -> %d\n", mfi->rating, rating);
-      mfi->rating = rating;
-      /* Note, that a DATABASE event is triggered, but the
-       * client actually expects a sticker event, so add it here. */
-      mpd_add_idle_events(LISTENER_STICKER);
-      db_file_update(mfi);
+      db_file_rating_update_byvirtualpath(virtual_path, rating);
     }
 
   free(virtual_path);
@@ -5057,28 +5049,6 @@ mpd_accept_error_cb(struct evconnlistener *listener, void *ctx)
   DPRINTF(E_LOG, L_MPD, "Error occured %d (%s) on the listener.\n", err, evutil_socket_error_to_string(err));
 }
 
-static void
-mpd_add_idle_events(short event_mask)
-{
-  struct mpd_client_ctx *client;
-  int i;
-
-  DPRINTF(E_DBG, L_MPD, "Add idle events (untriggered): %d\n", event_mask);
-
-  i = 0;
-  client = mpd_clients;
-  while (client)
-    {
-      client->events |= event_mask;
-      if (client->is_idle)
-	{
-	  client->events &= client->idle_events;
-	}
-      client = client->next;
-      i++;
-    }
-}
-
 static int
 mpd_notify_idle_client(struct mpd_client_ctx *client_ctx, short events)
 {
@@ -5154,21 +5124,11 @@ static void
 mpd_listener_cb(short event_mask)
 {
   short *ptr;
-  pthread_t current_thread = pthread_self();
 
-  if (pthread_equal(current_thread, tid_mpd))
-    {
-      int ret;
-      DPRINTF(E_DBG, L_MPD, "Immediate listener callback called with event type %d.\n", event_mask);
-      mpd_notify_idle(&event_mask, &ret);
-    }
-  else
-    {
-      ptr = (short *)malloc(sizeof(short));
-      *ptr = event_mask;
-      DPRINTF(E_DBG, L_MPD, "Asynchronous listener callback called with event type %d.\n", event_mask);
-      commands_exec_async(cmdbase, mpd_notify_idle, ptr);
-    }
+  ptr = (short *)malloc(sizeof(short));
+  *ptr = event_mask;
+  DPRINTF(E_DBG, L_MPD, "Asynchronous listener callback called with event type %d.\n", event_mask);
+  commands_exec_async(cmdbase, mpd_notify_idle, ptr);
 }
 
 /*