Abandon raop_metadata_arg, and defer metadata time setting

src/player.c

@@ -139,6 +139,16 @@ struct icy_artwork
   char *artwork_url;
 };
 
+struct player_metadata
+{
+  int id;
+  uint64_t rtptime;
+  uint64_t offset;
+  int startup;
+
+  struct raop_metadata *rmd;
+};
+
 struct player_command
 {
   pthread_mutex_t lck;
@@ -154,9 +164,9 @@ struct player_command
     void *noarg;
     struct spk_enum *spk_enum;
     struct raop_device *rd;
-    struct raop_metadata *rmd;
     struct player_status *status;
     struct player_source *ps;
+    struct player_metadata *pmd;
     player_status_handler status_handler;
     uint32_t *id_ptr;
     uint64_t *raop_ids;
@@ -580,7 +590,7 @@ static int
 queue_clear(struct player_command *cmd);
 
 static void
-player_metadata_send(struct raop_metadata *rmd);
+player_metadata_send(struct player_metadata *pmd);
 
 static void
 player_laudio_status_cb(enum laudio_state status)
@@ -647,12 +657,12 @@ playcount_inc_cb(void *arg)
 static void
 metadata_prepare_cb(void *arg)
 {
-  struct raop_metadata *rmd;
+  struct player_metadata *pmd = arg;
 
-  rmd = raop_metadata_prepare(arg);
+  pmd->rmd = raop_metadata_prepare(pmd->id);
 
-  if (rmd)
+  if (pmd->rmd)
-    player_metadata_send(rmd);
+    player_metadata_send(pmd);
 }
 
 /* Callback from the worker thread (async operation as it may block) */
@@ -682,37 +692,37 @@ metadata_purge(void)
 static void
 metadata_trigger(struct player_source *ps, int startup)
 {
-  struct raop_metadata_arg rma;
+  struct player_metadata pmd;
 
-  rma.id = ps->id;
+  memset(&pmd, 0, sizeof(struct player_metadata));
-  rma.offset = 0;
+
-  rma.startup = startup;
+  pmd.id = ps->id;
+  pmd.startup = startup;
 
   /* Determine song boundaries, dependent on context */
 
   /* Restart after pause/seek */
   if (ps->stream_start)
     {
-      rma.offset = ps->output_start - ps->stream_start;
+      pmd.offset = ps->output_start - ps->stream_start;
-      rma.rtptime = ps->stream_start;
+      pmd.rtptime = ps->stream_start;
     }
   else if (startup)
     {
-      rma.rtptime = last_rtptime + AIRTUNES_V2_PACKET_SAMPLES;
+      /* Will be set later, right before sending */
     }
   /* Generic case */
   else if (cur_streaming && (cur_streaming->end))
     {
-      rma.rtptime = cur_streaming->end + 1;
+      pmd.rtptime = cur_streaming->end + 1;
     }
   else
     {
-      rma.rtptime = 0;
       DPRINTF(E_LOG, L_PLAYER, "PTOH! Unhandled song boundary case in metadata_trigger()\n");
     }
 
   /* Defer the actual work of preparing the metadata to the worker thread */
-  worker_execute(metadata_prepare_cb, &rma, sizeof(struct raop_metadata_arg), 0);
+  worker_execute(metadata_prepare_cb, &pmd, sizeof(struct player_metadata), 0);
 }
 
 static void
@@ -2175,11 +2185,17 @@ device_remove_family(struct player_command *cmd)
 static int
 metadata_send(struct player_command *cmd)
 {
-  struct raop_metadata *rmd;
+  struct player_metadata *pmd;
 
-  rmd = cmd->arg.rmd;
+  pmd = cmd->arg.pmd;
 
-  raop_metadata_send(rmd);
+  /* Do the setting of rtptime which was deferred in metadata_trigger because we
+   * wanted to wait until we had the actual last_rtptime
+   */
+  if ((pmd->rtptime == 0) && (pmd->startup))
+    pmd->rtptime = last_rtptime + AIRTUNES_V2_PACKET_SAMPLES;
+
+  raop_metadata_send(pmd->rmd, pmd->rtptime, pmd->offset, pmd->startup);
 
   return 0;
 }
@@ -4896,35 +4912,19 @@ player_device_remove(struct raop_device *rd)
 
 /* Thread: worker */
 static void
-player_metadata_send(struct raop_metadata *rmd)
+player_metadata_send(struct player_metadata *pmd)
 {
-  struct player_command *cmd;
+  struct player_command cmd;
-  int ret;
 
-  cmd = (struct player_command *)malloc(sizeof(struct player_command));
+  command_init(&cmd);
-  if (!cmd)
-    {
-      DPRINTF(E_LOG, L_PLAYER, "Could not allocate player_command\n");
 
-      raop_metadata_free(rmd);
+  cmd.func = metadata_send;
-      return;
+  cmd.func_bh = NULL;
-    }
+  cmd.arg.pmd = pmd;
 
-  memset(cmd, 0, sizeof(struct player_command));
+  sync_command(&cmd);
 
-  cmd->nonblock = 1;
+  command_deinit(&cmd);
-
-  cmd->func = metadata_send;
-  cmd->arg.rmd = rmd;
-
-  ret = nonblock_command(cmd);
-  if (ret < 0)
-    {
-      free(cmd);
-      raop_metadata_free(rmd);
-
-      return;
-    }
 }
 
 

src/raop.c

@@ -154,9 +154,6 @@ struct raop_metadata
   uint64_t start;
   uint64_t end;
 
-  uint64_t offset;
-  int startup;
-
   struct raop_metadata *next;
 };
 
@@ -756,7 +753,7 @@ raop_metadata_prune(uint64_t rtptime)
 
 /* Thread: worker */
 struct raop_metadata *
-raop_metadata_prepare(struct raop_metadata_arg *rma)
+raop_metadata_prepare(int id)
 {
   struct query_params qp;
   struct db_media_file_info dbmfi;
@@ -785,10 +782,10 @@ raop_metadata_prepare(struct raop_metadata_arg *rma)
       goto skip_artwork;
     }
 
-  ret = artwork_get_item(rma->id, 600, 600, rmd->artwork);
+  ret = artwork_get_item(id, 600, 600, rmd->artwork);
   if (ret < 0)
     {
-      DPRINTF(E_INFO, L_RAOP, "Failed to retrieve artwork for file id %d; no artwork will be sent\n", rma->id);
+      DPRINTF(E_INFO, L_RAOP, "Failed to retrieve artwork for file id %d; no artwork will be sent\n", id);
 
       evbuffer_free(rmd->artwork);
       rmd->artwork = NULL;
@@ -805,10 +802,10 @@ raop_metadata_prepare(struct raop_metadata_arg *rma)
   qp.sort = S_NONE;
   qp.filter = filter;
 
-  ret = snprintf(filter, sizeof(filter), "id = %d", rma->id);
+  ret = snprintf(filter, sizeof(filter), "id = %d", id);
   if ((ret < 0) || (ret >= sizeof(filter)))
     {
-      DPRINTF(E_LOG, L_RAOP, "Could not build filter for file id %d; metadata will not be sent\n", rma->id);
+      DPRINTF(E_LOG, L_RAOP, "Could not build filter for file id %d; metadata will not be sent\n", id);
 
       goto out_rmd;
     }
@@ -824,7 +821,7 @@ raop_metadata_prepare(struct raop_metadata_arg *rma)
   ret = db_query_fetch_file(&qp, &dbmfi);
   if (ret < 0)
     {
-      DPRINTF(E_LOG, L_RAOP, "Couldn't fetch file id %d; metadata will not be sent\n", rma->id);
+      DPRINTF(E_LOG, L_RAOP, "Couldn't fetch file id %d; metadata will not be sent\n", id);
 
       goto out_query;
     }
@@ -867,11 +864,9 @@ raop_metadata_prepare(struct raop_metadata_arg *rma)
 
   db_query_end(&qp);
 
-  rmd->start = rma->rtptime;
+  /* raop_metadata_send() will add rtptime to these */
-  rmd->end = rma->rtptime + (duration * 44100UL) / 1000UL;
+  rmd->start = 0;
-
+  rmd->end = (duration * 44100UL) / 1000UL;
-  rmd->startup = rma->startup;
-  rmd->offset = rma->offset;
 
   return rmd;
 
@@ -2162,12 +2157,15 @@ raop_metadata_startup_send(struct raop_session *rs)
 }
 
 void
-raop_metadata_send(struct raop_metadata *rmd)
+raop_metadata_send(struct raop_metadata *rmd, uint64_t rtptime, uint64_t offset, int startup)
 {
   struct raop_session *rs;
   uint32_t delay;
   int ret;
 
+  rmd->start += rtptime;
+  rmd->end += rtptime;
+
   /* Add the rmd to the metadata list */
   if (metadata_tail)
     metadata_tail->next = rmd;
@@ -2185,9 +2183,9 @@ raop_metadata_send(struct raop_metadata *rmd)
       if (!rs->wants_metadata)
 	continue;
 
-      delay = (rmd->startup) ? RAOP_MD_DELAY_STARTUP : RAOP_MD_DELAY_SWITCH;
+      delay = (startup) ? RAOP_MD_DELAY_STARTUP : RAOP_MD_DELAY_SWITCH;
 
-      ret = raop_metadata_send_internal(rs, rmd, rmd->offset, delay);
+      ret = raop_metadata_send_internal(rs, rmd, offset, delay);
       if (ret < 0)
 	{
 	  raop_session_failure(rs);

src/raop.h

@@ -55,14 +55,6 @@ struct raop_device
   struct raop_device *next;
 };
 
-struct raop_metadata_arg
-{
-  int id;
-  uint64_t rtptime;
-  uint64_t offset;
-  int startup;
-};
-
 /* RAOP session state */
 
 /* Session is starting up */
@@ -109,10 +101,10 @@ void
 raop_metadata_prune(uint64_t rtptime);
 
 struct raop_metadata *
-raop_metadata_prepare(struct raop_metadata_arg *rma);
+raop_metadata_prepare(int id);
 
 void
-raop_metadata_send(struct raop_metadata *rmd);
+raop_metadata_send(struct raop_metadata *rmd, uint64_t rtptime, uint64_t offset, int startup);
 
 int
 raop_device_probe(struct raop_device *rd, raop_status_cb cb);