Merge pull request #1610 from owntone/streaming_ffmpeg6

Refactor mp3 streaming/fix for ffmpeg 6

src/Makefile.am

@@ -118,7 +118,7 @@ owntone_SOURCES = main.c \
 	outputs/rtp_common.h outputs/rtp_common.c \
 	outputs/raop.c outputs/airplay.c $(PAIR_AP_SRC) \
 	outputs/airplay_events.c outputs/airplay_events.h \
-	outputs/streaming.c outputs/streaming.h \
+	outputs/streaming.c \
 	outputs/dummy.c outputs/fifo.c outputs/rcp.c \
 	$(ALSA_SRC) $(PULSEAUDIO_SRC) $(CHROMECAST_SRC) \
 	evrtsp/rtsp.c evrtsp/evrtsp.h evrtsp/rtsp-internal.h evrtsp/log.h \

src/httpd_streaming.c

@@ -31,20 +31,25 @@
 #include <event2/buffer.h>
 
 #include "httpd_internal.h"
-#include "outputs/streaming.h"
+#include "player.h"
 #include "logger.h"
 #include "conffile.h"
 
+#define STREAMING_ICY_METALEN_MAX      4080  // 255*16 incl header/footer (16bytes)
+#define STREAMING_ICY_METATITLELEN_MAX 4064  // STREAMING_ICY_METALEN_MAX -16 (not incl header/footer)
+
 struct streaming_session {
   struct httpd_request *hreq;
 
-  int fd;
+  int id;
-  struct event *readev;
+  struct event *audioev;
-  struct evbuffer *readbuf;
+  struct event *metadataev;
+  struct evbuffer *audiobuf;
   size_t bytes_sent;
 
   bool icy_is_requested;
   size_t icy_remaining;
+  char icy_title[STREAMING_ICY_METATITLELEN_MAX];
 };
 
 static struct media_quality streaming_default_quality = {
@@ -54,25 +59,20 @@ static struct media_quality streaming_default_quality = {
   .bit_rate = 128000,
 };
 
+static void
+session_free(struct streaming_session *session);
 
 /* ------------------------------ ICY metadata -------------------------------*/
 
 // To test mp3 and ICY tagm it is good to use:
 //   mpv --display-tags=* http://localhost:3689/stream.mp3
 
-#define STREAMING_ICY_METALEN_MAX      4080  // 255*16 incl header/footer (16bytes)
-#define STREAMING_ICY_METATITLELEN_MAX 4064  // STREAMING_ICY_METALEN_MAX -16 (not incl header/footer)
-
 // As streaming quality goes up, we send more data to the remote client.  With a
 // smaller ICY_METAINT value we have to splice metadata more frequently - on
 // some devices with small input buffers, a higher quality stream and low
 // ICY_METAINT can lead to stuttering as observed on a Roku Soundbridge
 static unsigned short streaming_icy_metaint = 16384;
 
-static pthread_mutex_t streaming_metadata_lck;
-static char streaming_icy_title[STREAMING_ICY_METATITLELEN_MAX];
-
-
 // We know that the icymeta is limited to 1+255*16 (ie 4081) bytes so caller must
 // provide a buf of this size to avoid needless mallocs
 //
@@ -124,7 +124,7 @@ icy_meta_create(uint8_t buf[STREAMING_ICY_METALEN_MAX+1], unsigned *buflen, cons
 }
 
 static void
-icy_meta_splice(struct evbuffer *out, struct evbuffer *in, size_t *icy_remaining)
+icy_meta_splice(struct evbuffer *out, struct evbuffer *in, size_t *icy_remaining, char *title)
 {
   uint8_t meta[STREAMING_ICY_METALEN_MAX + 1];
   unsigned metalen;
@@ -138,9 +138,7 @@ icy_meta_splice(struct evbuffer *out, struct evbuffer *in, size_t *icy_remaining
       *icy_remaining -= consume;
       if (*icy_remaining == 0)
 	{
-	  pthread_mutex_lock(&streaming_metadata_lck);
+	  icy_meta_create(meta, &metalen, title);
-	  icy_meta_create(meta, &metalen, streaming_icy_title);
-	  pthread_mutex_unlock(&streaming_metadata_lck);
 
 	  evbuffer_add(out, meta, metalen);
 	  *icy_remaining = streaming_icy_metaint;
@@ -148,50 +146,6 @@ icy_meta_splice(struct evbuffer *out, struct evbuffer *in, size_t *icy_remaining
     }
 }
 
-// Thread: player. TODO Would be nice to avoid the lock. Consider moving all the
-// ICY tag stuff to streaming.c and make a STREAMING_FORMAT_MP3_ICY?
-static void
-icy_metadata_cb(char *metadata)
-{
-  pthread_mutex_lock(&streaming_metadata_lck);
-  snprintf(streaming_icy_title, sizeof(streaming_icy_title), "%s", metadata);
-  pthread_mutex_unlock(&streaming_metadata_lck);
-}
-
-
-/* ----------------------------- Session helpers ---------------------------- */
-
-static void
-session_free(struct streaming_session *session)
-{
-  if (!session)
-    return;
-
-  if (session->readev)
-    {
-      streaming_session_deregister(session->fd);
-      event_free(session->readev);
-    }
-
-  evbuffer_free(session->readbuf);
-  free(session);
-}
-
-static struct streaming_session *
-session_new(struct httpd_request *hreq, bool icy_is_requested)
-{
-  struct streaming_session *session;
-
-  CHECK_NULL(L_STREAMING, session = calloc(1, sizeof(struct streaming_session)));
-  CHECK_NULL(L_STREAMING, session->readbuf = evbuffer_new());
-
-  session->hreq = hreq;
-  session->icy_is_requested = icy_is_requested;
-  session->icy_remaining = streaming_icy_metaint;
-
-  return session;
-}
-
 
 /* ----------------------------- Event callbacks ---------------------------- */
 
@@ -204,7 +158,7 @@ conn_close_cb(void *arg)
 }
 
 static void
-read_cb(evutil_socket_t fd, short event, void *arg)
+audio_cb(evutil_socket_t fd, short event, void *arg)
 {
   struct streaming_session *session = arg;
   struct httpd_request *hreq;
@@ -212,7 +166,7 @@ read_cb(evutil_socket_t fd, short event, void *arg)
 
   CHECK_NULL(L_STREAMING, hreq = session->hreq);
 
-  len = evbuffer_read(session->readbuf, fd, -1);
+  len = evbuffer_read(session->audiobuf, fd, -1);
   if (len < 0 && errno != EAGAIN)
     {
       DPRINTF(E_INFO, L_STREAMING, "Stopping mp3 streaming to %s:%d\n", session->hreq->peer_address, (int)session->hreq->peer_port);
@@ -223,22 +177,94 @@ read_cb(evutil_socket_t fd, short event, void *arg)
     }
 
   if (session->icy_is_requested)
-    icy_meta_splice(hreq->out_body, session->readbuf, &session->icy_remaining);
+    icy_meta_splice(hreq->out_body, session->audiobuf, &session->icy_remaining, session->icy_title);
   else
-    evbuffer_add_buffer(hreq->out_body, session->readbuf);
+    evbuffer_add_buffer(hreq->out_body, session->audiobuf);
 
   httpd_send_reply_chunk(hreq, NULL, NULL);
 
   session->bytes_sent += len;
 }
 
+static void
+metadata_cb(evutil_socket_t fd, short event, void *arg)
+{
+  struct streaming_session *session = arg;
+  struct evbuffer *evbuf;
+  int len;
+
+  CHECK_NULL(L_STREAMING, evbuf = evbuffer_new());
+
+  len = evbuffer_read(evbuf, fd, -1);
+  if (len < 0)
+    goto out;
+
+  len = sizeof(session->icy_title);
+  evbuffer_remove(evbuf, session->icy_title, len);
+  session->icy_title[len - 1] = '\0';
+
+ out:
+  evbuffer_free(evbuf);
+}
+
+
+/* ----------------------------- Session helpers ---------------------------- */
+
+static void
+session_free(struct streaming_session *session)
+{
+  if (!session)
+    return;
+
+  player_streaming_deregister(session->id);
+
+  if (session->audioev)
+    event_free(session->audioev);
+  if (session->metadataev)
+    event_free(session->metadataev);
+
+  evbuffer_free(session->audiobuf);
+  free(session);
+}
+
+static struct streaming_session *
+session_new(struct httpd_request *hreq, bool icy_is_requested, enum player_format format, struct media_quality quality)
+{
+  struct streaming_session *session;
+  int audio_fd;
+  int metadata_fd;
+
+  CHECK_NULL(L_STREAMING, session = calloc(1, sizeof(struct streaming_session)));
+  CHECK_NULL(L_STREAMING, session->audiobuf = evbuffer_new());
+
+  session->hreq = hreq;
+  session->icy_is_requested = icy_is_requested;
+  session->icy_remaining = streaming_icy_metaint;
+
+  // Ask streaming output module for a fd to read mp3 from
+  session->id = player_streaming_register(&audio_fd, &metadata_fd, format, quality);
+  if (session->id < 0)
+    goto error;
+
+  CHECK_NULL(L_STREAMING, session->audioev = event_new(hreq->evbase, audio_fd, EV_READ | EV_PERSIST, audio_cb, session));
+  event_add(session->audioev, NULL);
+  CHECK_NULL(L_STREAMING, session->metadataev = event_new(hreq->evbase, metadata_fd, EV_READ | EV_PERSIST, metadata_cb, session));
+  event_add(session->metadataev, NULL);
+
+  return session;
+
+ error:
+  session_free(session);
+  return NULL;
+}
+
 
 /* -------------------------- Module implementation ------------------------- */
 
 static int
 streaming_mp3_handler(struct httpd_request *hreq)
 {
-  struct streaming_session *session;
+  struct streaming_session *session = NULL;
   const char *name = cfg_getstr(cfg_getsec(cfg, "library"), "name");
   const char *param;
   bool icy_is_requested;
@@ -253,15 +279,9 @@ streaming_mp3_handler(struct httpd_request *hreq)
       httpd_header_add(hreq->out_headers, "icy-metaint", buf);
     }
 
-  session = session_new(hreq, icy_is_requested);
+  session = session_new(hreq, icy_is_requested, PLAYER_FORMAT_MP3, streaming_default_quality);
   if (!session)
-    return -1;
+    return -1; // Error sent by caller
-
-  // Ask streaming output module for a fd to read mp3 from
-  session->fd = streaming_session_register(STREAMING_FORMAT_MP3, streaming_default_quality);
-
-  CHECK_NULL(L_STREAMING, session->readev = event_new(hreq->evbase, session->fd, EV_READ | EV_PERSIST, read_cb, session));
-  event_add(session->readev, NULL);
 
   httpd_request_close_cb_set(hreq, conn_close_cb, session);
 
@@ -345,9 +365,6 @@ streaming_init(void)
   else
     DPRINTF(E_INFO, L_STREAMING, "Unsupported icy_metaint=%d, supported range: 4096..131072, defaulting to %d\n", val, streaming_icy_metaint);
 
-  CHECK_ERR(L_STREAMING, mutex_init(&streaming_metadata_lck));
-  streaming_metadatacb_register(icy_metadata_cb);
-
   return 0;
 }
 

src/outputs.c

@@ -389,6 +389,43 @@ buffer_drain(struct output_buffer *obuf)
     }
 }
 
+static struct output_buffer *
+buffer_copy(struct output_buffer *obuf)
+{
+  struct output_buffer *copy;
+  int i;
+
+  if (!obuf)
+    return NULL;
+
+  CHECK_NULL(L_PLAYER, copy = malloc(sizeof(struct output_buffer)));
+
+  memcpy(copy, obuf, sizeof(struct output_buffer));
+
+  for (i = 0; obuf->data[i].buffer; i++)
+    {
+      CHECK_NULL(L_PLAYER, copy->data[i].evbuf = evbuffer_new());
+      evbuffer_add(copy->data[i].evbuf, obuf->data[i].buffer, obuf->data[i].bufsize);
+      copy->data[i].buffer = evbuffer_pullup(copy->data[i].evbuf, -1);
+    }
+
+  return copy;
+}
+
+static void
+buffer_free(struct output_buffer *obuf)
+{
+  int i;
+
+  if (!obuf)
+    return;
+
+  for (i = 0; obuf->data[i].buffer; i++)
+    evbuffer_free(obuf->data[i].evbuf);
+
+  free(obuf);
+}
+
 static void
 device_list_sort(void)
 {
@@ -705,6 +742,18 @@ outputs_metadata_free(struct output_metadata *metadata)
   metadata_free(metadata);
 }
 
+struct output_buffer *
+outputs_buffer_copy(struct output_buffer *buffer)
+{
+  return buffer_copy(buffer);
+}
+
+void
+outputs_buffer_free(struct output_buffer *buffer)
+{
+  buffer_free(buffer);
+}
+
 /* ---------------------------- Called by player ---------------------------- */
 
 struct output_device *

src/outputs.h

@@ -134,6 +134,7 @@ struct output_device
 
   // Quality of audio output
   struct media_quality quality;
+  int format;
 
   // Address
   char *v4_address;
@@ -141,6 +142,10 @@ struct output_device
   short v4_port;
   short v6_port;
 
+  // Only used for streaming
+  int audio_fd;
+  int metadata_fd;
+
   struct event *stop_timer;
 
   // Opaque pointers to device and session data
@@ -287,6 +292,12 @@ outputs_cb(int callback_id, uint64_t device_id, enum output_device_state);
 void
 outputs_metadata_free(struct output_metadata *metadata);
 
+struct output_buffer *
+outputs_buffer_copy(struct output_buffer *buffer);
+
+void
+outputs_buffer_free(struct output_buffer *buffer);
+
 /* ---------------------------- Called by player ---------------------------- */
 
 // Ownership of *add is transferred, so don't address after calling. Instead you

src/outputs/streaming.c

@@ -28,10 +28,10 @@
 #include <uninorm.h>
 #include <fcntl.h>
 
-#include "streaming.h"
 #include "outputs.h"
 #include "misc.h"
 #include "worker.h"
+#include "player.h"
 #include "transcode.h"
 #include "logger.h"
 #include "db.h"
@@ -44,15 +44,10 @@
  * player, but there are clients, it instead writes silence to the fd.
  */
 
-// Seconds between sending silence when player is idle
+// Seconds between sending a frame of silence when player is idle
 // (to prevent client from hanging up)
 #define STREAMING_SILENCE_INTERVAL 1
 
-// How many bytes of silence we encode with the above interval. There is no
-// particular reason for using this size, just that it seems to have worked for
-// a while.
-#define SILENCE_BUF_SIZE STOB(352, 16, 2)
-
 // The wanted structure represents a particular format and quality that should
 // be produced for one or more sessions. A pipe pair is created for each session
 // for the i/o.
@@ -66,15 +61,20 @@ struct pipepair
 
 struct streaming_wanted
 {
-  int refcount;
+  int num_sessions; // for refcounting
-  struct pipepair pipes[WANTED_PIPES_MAX];
+  struct pipepair audio[WANTED_PIPES_MAX];
+  struct pipepair metadata[WANTED_PIPES_MAX];
 
-  enum streaming_format format;
+  enum player_format format;
-  struct media_quality quality_in;
+  struct media_quality quality;
-  struct media_quality quality_out;
 
+  struct evbuffer *audio_in;
+  struct evbuffer *audio_out;
   struct encode_ctx *xcode_ctx;
-  struct evbuffer *encoded_data;
+
+  int nb_samples;
+  uint8_t *frame_data;
+  size_t frame_size;
 
   struct streaming_wanted *next;
 };
@@ -86,21 +86,17 @@ struct streaming_ctx
   struct timeval silencetv;
   struct media_quality last_quality;
 
+  char title[4064]; // See STREAMING_ICY_METALEN_MAX in http_streaming.c
+
   // seqnum may wrap around so must be unsigned
   unsigned int seqnum;
   unsigned int seqnum_encode_next;
-
-  // callback with new metadata, e.g. for ICY tags
-  void (*metadatacb)(char *metadata);
 };
 
 struct encode_cmdarg
 {
-  uint8_t *buf;
+  struct output_buffer *obuf;
-  size_t bufsize;
-  int samples;
   unsigned int seqnum;
-  struct media_quality quality;
 };
 
 static pthread_mutex_t streaming_wanted_lck;
@@ -116,6 +112,41 @@ extern struct event_base *evbase_player;
 
 /* ------------------------------- Helpers ---------------------------------- */
 
+static struct encode_ctx *
+encoder_setup(enum player_format format, struct media_quality *quality)
+{
+  struct decode_ctx *decode_ctx = NULL;
+  struct encode_ctx *encode_ctx = NULL;
+
+  if (quality->bits_per_sample == 16)
+    decode_ctx = transcode_decode_setup_raw(XCODE_PCM16, quality);
+  else if (quality->bits_per_sample == 24)
+    decode_ctx = transcode_decode_setup_raw(XCODE_PCM24, quality);
+  else if (quality->bits_per_sample == 32)
+    decode_ctx = transcode_decode_setup_raw(XCODE_PCM32, quality);
+
+  if (!decode_ctx)
+    {
+      DPRINTF(E_LOG, L_STREAMING, "Error setting up decoder for quality sr %d, bps %d, ch %d, cannot encode\n",
+	quality->sample_rate, quality->bits_per_sample, quality->channels);
+      goto out;
+    }
+
+  if (format == PLAYER_FORMAT_MP3)
+    encode_ctx = transcode_encode_setup(XCODE_MP3, quality, decode_ctx, NULL, 0, 0);
+
+  if (!encode_ctx)
+    {
+      DPRINTF(E_LOG, L_STREAMING, "Error setting up encoder for quality sr %d, bps %d, ch %d, cannot encode\n",
+	quality->sample_rate, quality->bits_per_sample, quality->channels);
+      goto out;
+    }
+
+ out:
+  transcode_decode_cleanup(&decode_ctx);
+  return encode_ctx;
+}
+
 static int
 pipe_open(struct pipepair *p)
 {
@@ -162,31 +193,62 @@ wanted_free(struct streaming_wanted *w)
     return;
 
   for (int i = 0; i < WANTED_PIPES_MAX; i++)
-    pipe_close(&w->pipes[i]);
+    pipe_close(&w->audio[i]);
+  for (int i = 0; i < WANTED_PIPES_MAX; i++)
+    pipe_close(&w->metadata[i]);
 
   transcode_encode_cleanup(&w->xcode_ctx);
-  evbuffer_free(w->encoded_data);
+  evbuffer_free(w->audio_in);
+  evbuffer_free(w->audio_out);
+  free(w->frame_data);
   free(w);
 }
 
+static int
+pipe_index_find_byreadfd(struct pipepair *p, int readfd)
+{
+  for (int i = 0; i < WANTED_PIPES_MAX; i++, p++)
+    {
+      if (p->readfd == readfd)
+	return i;
+    }
+
+  return -1;
+}
+
 static struct streaming_wanted *
-wanted_new(enum streaming_format format, struct media_quality quality)
+wanted_new(enum player_format format, struct media_quality quality)
 {
   struct streaming_wanted *w;
 
   CHECK_NULL(L_STREAMING, w = calloc(1, sizeof(struct streaming_wanted)));
-  CHECK_NULL(L_STREAMING, w->encoded_data = evbuffer_new());
+  CHECK_NULL(L_STREAMING, w->audio_in = evbuffer_new());
+  CHECK_NULL(L_STREAMING, w->audio_out = evbuffer_new());
+
+  w->xcode_ctx = encoder_setup(format, &quality);
+  if (!w->xcode_ctx)
+    goto error;
 
-  w->quality_out = quality;
   w->format = format;
+  w->quality = quality;
+  w->nb_samples = transcode_encode_query(w->xcode_ctx, "samples_per_frame"); // 1152 for mp3
+  w->frame_size = STOB(w->nb_samples, quality.bits_per_sample, quality.channels);
+
+  CHECK_NULL(L_STREAMING, w->frame_data = malloc(w->frame_size));
 
   for (int i = 0; i < WANTED_PIPES_MAX; i++)
     {
-      w->pipes[i].writefd = -1;
+      w->audio[i].writefd = -1;
-      w->pipes[i].readfd = -1;
+      w->audio[i].readfd = -1;
+      w->metadata[i].writefd = -1;
+      w->metadata[i].readfd = -1;
     }
 
   return w;
+
+ error:
+  wanted_free(w);
+  return NULL;
 }
 
 static void
@@ -215,7 +277,7 @@ wanted_remove(struct streaming_wanted **wanted, struct streaming_wanted *remove)
 }
 
 static struct streaming_wanted *
-wanted_add(struct streaming_wanted **wanted, enum streaming_format format, struct media_quality quality)
+wanted_add(struct streaming_wanted **wanted, enum player_format format, struct media_quality quality)
 {
   struct streaming_wanted *w;
 
@@ -227,13 +289,13 @@ wanted_add(struct streaming_wanted **wanted, enum streaming_format format, struc
 }
 
 static struct streaming_wanted *
-wanted_find_byformat(struct streaming_wanted *wanted, enum streaming_format format, struct media_quality quality)
+wanted_find_byformat(struct streaming_wanted *wanted, enum player_format format, struct media_quality quality)
 {
   struct streaming_wanted *w;
 
   for (w = wanted; w; w = w->next)
     {
-      if (w->format == format && quality_is_equal(&w->quality_out, &quality))
+      if (w->format == format && quality_is_equal(&w->quality, &quality))
 	return w;
     }
 
@@ -247,31 +309,36 @@ wanted_find_byreadfd(struct streaming_wanted *wanted, int readfd)
   int i;
 
   for (w = wanted; w; w = w->next)
-    for (i = 0; i < WANTED_PIPES_MAX; i++)
+    {
-      {
+      i = pipe_index_find_byreadfd(w->audio, readfd);
-	if (w->pipes[i].readfd == readfd)
+      if (i != -1)
-	  return w;
+	return w;
-      }
+    }
 
   return NULL;
 }
 
 static int
-wanted_session_add(struct pipepair *p, struct streaming_wanted *w)
+wanted_session_add(int *audiofd, int *metadatafd, struct streaming_wanted *w)
 {
   int ret;
   int i;
 
   for (i = 0; i < WANTED_PIPES_MAX; i++)
     {
-      if (w->pipes[i].writefd != -1) // In use
+      if (w->audio[i].writefd != -1) // In use
 	continue;
 
-      ret = pipe_open(&w->pipes[i]);
+      ret = pipe_open(&w->audio[i]);
       if (ret < 0)
 	return -1;
 
-      memcpy(p, &w->pipes[i], sizeof(struct pipepair));
+      ret = pipe_open(&w->metadata[i]);
+      if (ret < 0)
+	return -1;
+
+      *audiofd = w->audio[i].readfd;
+      *metadatafd = w->metadata[i].readfd;
       break;
     }
 
@@ -281,133 +348,152 @@ wanted_session_add(struct pipepair *p, struct streaming_wanted *w)
       return -1;
     }
 
-  w->refcount++;
+  w->num_sessions++;
-  DPRINTF(E_DBG, L_STREAMING, "Session register readfd %d, wanted->refcount=%d\n", p->readfd, w->refcount);
+  DPRINTF(E_DBG, L_STREAMING, "Session register audiofd %d, metadatafd %d, wanted->num_sessions=%d\n", *audiofd, *metadatafd, w->num_sessions);
   return 0;
 }
 
-
 static void
 wanted_session_remove(struct streaming_wanted *w, int readfd)
 {
   int i;
 
-  for (i = 0; i < WANTED_PIPES_MAX; i++)
+  i = pipe_index_find_byreadfd(w->audio, readfd);
-    {
+  if (i < 0)
-      if (w->pipes[i].readfd != readfd)
-	continue;
-
-      pipe_close(&w->pipes[i]);
-      break;
-    }
-
-  if (i == WANTED_PIPES_MAX)
     {
       DPRINTF(E_LOG, L_STREAMING, "Cannot remove streaming session, readfd %d not found\n", readfd);
       return;
     }
 
-  w->refcount--;
+  pipe_close(&w->audio[i]);
-  DPRINTF(E_DBG, L_STREAMING, "Session deregister readfd %d, wanted->refcount=%d\n", readfd, w->refcount);
+  pipe_close(&w->metadata[i]);
+
+  w->num_sessions--;
+  DPRINTF(E_DBG, L_STREAMING, "Session deregister readfd %d, wanted->num_sessions=%d\n", readfd, w->num_sessions);
 }
 
 
 /* ----------------------------- Thread: Worker ----------------------------- */
 
 static int
-encode_reset(struct streaming_wanted *w, struct media_quality quality_in)
+encode_buffer(struct streaming_wanted *w, uint8_t *buf, size_t bufsize)
 {
-  struct media_quality quality_out = w->quality_out;
+  ssize_t remaining_bytes;
-  struct decode_ctx *decode_ctx = NULL;
+  transcode_frame *frame = NULL;
+  int ret;
 
-  transcode_encode_cleanup(&w->xcode_ctx);
+  if (buf)
-
-  if (quality_in.bits_per_sample == 16)
-    decode_ctx = transcode_decode_setup_raw(XCODE_PCM16, &quality_in);
-  else if (quality_in.bits_per_sample == 24)
-    decode_ctx = transcode_decode_setup_raw(XCODE_PCM24, &quality_in);
-  else if (quality_in.bits_per_sample == 32)
-    decode_ctx = transcode_decode_setup_raw(XCODE_PCM32, &quality_in);
-
-  if (!decode_ctx)
     {
-      DPRINTF(E_LOG, L_STREAMING, "Error setting up decoder for input quality sr %d, bps %d, ch %d, cannot MP3 encode\n",
+      evbuffer_add(w->audio_in, buf, bufsize);
-	quality_in.sample_rate, quality_in.bits_per_sample, quality_in.channels);
+    }
-      goto error;
+  else
+    {
+      // buf being null is either a silence timeout or that we could't find the
+      // subscripted quality. In both cases we encode silence.
+      memset(w->frame_data, 0, w->frame_size);
+      evbuffer_add(w->audio_in, w->frame_data, w->frame_size);
     }
 
-  w->quality_in = quality_in;
+  remaining_bytes = evbuffer_get_length(w->audio_in);
-  w->xcode_ctx = transcode_encode_setup(XCODE_MP3, &quality_out, decode_ctx, NULL, 0, 0);
+
-  if (!w->xcode_ctx)
+  // Read and encode from 'audio_in' in chunks of 'frame_size' bytes
+  while (remaining_bytes > w->frame_size)
     {
-      DPRINTF(E_LOG, L_STREAMING, "Error setting up encoder for output quality sr %d, bps %d, ch %d, cannot MP3 encode\n",
+      ret = evbuffer_remove(w->audio_in, w->frame_data, w->frame_size);
-	quality_out.sample_rate, quality_out.bits_per_sample, quality_out.channels);
+      if (ret != w->frame_size)
-      goto error;
+	{
+	  DPRINTF(E_LOG, L_STREAMING, "Bug! Couldn't read a frame of %zu bytes (format %d)\n", w->frame_size, w->format);
+	  goto error;
+	}
+
+      remaining_bytes -= w->frame_size;
+
+      frame = transcode_frame_new(w->frame_data, w->frame_size, w->nb_samples, &w->quality);
+      if (!frame)
+	{
+	  DPRINTF(E_LOG, L_STREAMING, "Could not convert raw PCM to frame (format %d)\n", w->format);
+	  goto error;
+	}
+
+      ret = transcode_encode(w->audio_out, w->xcode_ctx, frame, 0);
+      if (ret < 0)
+	{
+	  DPRINTF(E_LOG, L_STREAMING, "Encoding error (format %d)\n", w->format);
+	  goto error;
+	}
+
+      transcode_frame_free(frame);
     }
 
-  transcode_decode_cleanup(&decode_ctx);
   return 0;
 
  error:
-  transcode_decode_cleanup(&decode_ctx);
+  transcode_frame_free(frame);
   return -1;
 }
 
-static int
-encode_frame(struct streaming_wanted *w, struct media_quality quality_in, transcode_frame *frame)
-{
-  int ret;
-
-  if (!w->xcode_ctx || !quality_is_equal(&quality_in, &w->quality_in))
-    {
-      DPRINTF(E_DBG, L_STREAMING, "Resetting transcode context\n");
-      if (encode_reset(w, quality_in) < 0)
-	return -1;
-    }
-
-  ret = transcode_encode(w->encoded_data, w->xcode_ctx, frame, 0);
-  if (ret < 0)
-    {
-      return -1;
-    }
-
-  return 0;
-}
-
 static void
-encode_write(uint8_t *buf, size_t buflen, struct streaming_wanted *w, struct pipepair *p)
+encode_and_write(int *failed_pipe_readfd, struct streaming_wanted *w, struct output_buffer *obuf)
 {
+  uint8_t *buf;
+  size_t bufsize;
+  size_t len;
   int ret;
+  int i;
 
-  if (p->writefd < 0)
+  for (i = 0, buf = NULL, bufsize = 0; obuf && obuf->data[i].buffer; i++)
-    return;
+    {
+      if (!quality_is_equal(&obuf->data[i].quality, &w->quality))
+	continue;
 
-  ret = write(p->writefd, buf, buflen);
+      buf = obuf->data[i].buffer;
+      bufsize = obuf->data[i].bufsize;
+    }
+
+  // If encoding fails we should kill the sessions, which for thread safety
+  // and to avoid deadlocks has to be done later with player_streaming_deregister()
+  ret = encode_buffer(w, buf, bufsize);
   if (ret < 0)
     {
-      DPRINTF(E_LOG, L_STREAMING, "Error writing to stream pipe %d (format %d): %s\n", p->writefd, w->format, strerror(errno));
+      for (i = 0; i < WANTED_PIPES_MAX; i++)
-      wanted_session_remove(w, p->readfd);
+	{
+	  if (w->audio[i].writefd != -1)
+	    *failed_pipe_readfd = w->audio[i].readfd;
+	}
+
+      return;
     }
+
+  len = evbuffer_get_length(w->audio_out);
+  if (len == 0)
+    {
+      return;
+    }
+
+  buf = evbuffer_pullup(w->audio_out, -1);
+  for (i = 0; i < WANTED_PIPES_MAX; i++)
+    {
+      if (w->audio[i].writefd == -1)
+	continue;
+
+      ret = write(w->audio[i].writefd, buf, len);
+      if (ret < 0)
+	{
+	  DPRINTF(E_LOG, L_STREAMING, "Error writing to stream pipe %d (format %d): %s\n", w->audio[i].writefd, w->format, strerror(errno));
+	  *failed_pipe_readfd = w->audio[i].readfd;
+	}
+    }
+
+  evbuffer_drain(w->audio_out, -1);
 }
 
 static void
 encode_data_cb(void *arg)
 {
   struct encode_cmdarg *ctx = arg;
-  transcode_frame *frame;
+  struct output_buffer *obuf = ctx->obuf;
   struct streaming_wanted *w;
-  struct streaming_wanted *next;
+  int failed_pipe_readfd = -1;
-  uint8_t *buf;
-  size_t len;
-  int ret;
-  int i;
-
-  frame = transcode_frame_new(ctx->buf, ctx->bufsize, ctx->samples, &ctx->quality);
-  if (!frame)
-    {
-      DPRINTF(E_LOG, L_STREAMING, "Could not convert raw PCM to frame\n");
-      goto out;
-    }
 
   pthread_mutex_lock(&streaming_wanted_lck);
 
@@ -415,43 +501,59 @@ encode_data_cb(void *arg)
   while (ctx->seqnum != streaming.seqnum_encode_next)
     pthread_cond_wait(&streaming_sequence_cond, &streaming_wanted_lck);
 
-  for (w = streaming.wanted; w; w = next)
+  for (w = streaming.wanted; w; w = w->next)
-    {
+    encode_and_write(&failed_pipe_readfd, w, obuf);
-      next = w->next;
-
-      ret = encode_frame(w, ctx->quality, frame);
-      if (ret < 0)
-	wanted_remove(&streaming.wanted, w); // This will close all the fds, so readers get an error
-
-      len = evbuffer_get_length(w->encoded_data);
-      if (len == 0)
-	continue;
-
-      buf = evbuffer_pullup(w->encoded_data, -1);
-
-      for (i = 0; i < WANTED_PIPES_MAX; i++)
-	encode_write(buf, len, w, &w->pipes[i]);
-
-      evbuffer_drain(w->encoded_data, -1);
-
-      if (w->refcount == 0)
-	wanted_remove(&streaming.wanted, w);
-    }
 
   streaming.seqnum_encode_next++;
   pthread_cond_broadcast(&streaming_sequence_cond);
   pthread_mutex_unlock(&streaming_wanted_lck);
 
- out:
+  outputs_buffer_free(ctx->obuf);
-  transcode_frame_free(frame);
+
-  free(ctx->buf);
+  // We have to do this after letting go of the lock or we will deadlock. This
+  // unfortunate method means we can only fail one session (pipe) each pass.
+  if (failed_pipe_readfd >= 0)
+    player_streaming_deregister(failed_pipe_readfd);
+}
+
+static void
+metadata_write(struct streaming_wanted *w, int readfd, const char *metadata)
+{
+  size_t metadata_size;
+  int i;
+  int ret;
+
+  for (i = 0; i < WANTED_PIPES_MAX; i++)
+    {
+      if (w->metadata[i].writefd == -1)
+	continue;
+      if (readfd >= 0 && w->metadata[i].readfd != readfd)
+	continue;
+
+      metadata_size = strlen(metadata) + 1;
+      ret = write(w->metadata[i].writefd, metadata, metadata_size);
+      if (ret < 0)
+	DPRINTF(E_WARN, L_STREAMING, "Error writing metadata '%s' to fd %d\n", metadata, w->metadata[i].writefd);
+    }
+}
+
+static void
+metadata_startup_cb(void *arg)
+{
+  int *metadata_fd = arg;
+  struct streaming_wanted *w;
+
+  pthread_mutex_lock(&streaming_wanted_lck);
+  for (w = streaming.wanted; w; w = w->next)
+    metadata_write(w, *metadata_fd, streaming.title);
+  pthread_mutex_unlock(&streaming_wanted_lck);
 }
 
 static void *
 streaming_metadata_prepare(struct output_metadata *metadata)
 {
   struct db_queue_item *queue_item;
-  char *title;
+  struct streaming_wanted *w;
 
   queue_item = db_queue_fetch_byitemid(metadata->item_id);
   if (!queue_item)
@@ -460,131 +562,113 @@ streaming_metadata_prepare(struct output_metadata *metadata)
       return NULL;
     }
 
-  title = safe_asprintf("%s - %s", queue_item->title, queue_item->artist);
+  pthread_mutex_lock(&streaming_wanted_lck);
+  // Save it here, we might need it later if a new session starts up
+  snprintf(streaming.title, sizeof(streaming.title), "%s - %s", queue_item->title, queue_item->artist);
+
+  for (w = streaming.wanted; w; w = w->next)
+    metadata_write(w, -1, streaming.title);
+  pthread_mutex_unlock(&streaming_wanted_lck);
+
   free_queue_item(queue_item, 0);
-
+  return NULL;
-  return title;
 }
 
 
-/* ----------------------------- Thread: httpd ------------------------------ */
-
-int
-streaming_session_register(enum streaming_format format, struct media_quality quality)
-{
-  struct streaming_wanted *w;
-  struct pipepair pipe;
-  int ret;
-
-  pthread_mutex_lock(&streaming_wanted_lck);
-  w = wanted_find_byformat(streaming.wanted, format, quality);
-  if (!w)
-    w = wanted_add(&streaming.wanted, format, quality);
-
-  ret = wanted_session_add(&pipe, w);
-  if (ret < 0)
-    pipe.readfd = -1;
-
-  pthread_mutex_unlock(&streaming_wanted_lck);
-
-  return pipe.readfd;
-}
-
-void
-streaming_session_deregister(int readfd)
-{
-  struct streaming_wanted *w;
-
-  pthread_mutex_lock(&streaming_wanted_lck);
-  w = wanted_find_byreadfd(streaming.wanted, readfd);
-  if (!w)
-    goto out;
-
-  wanted_session_remove(w, readfd);
-
-  if (w->refcount == 0)
-    wanted_remove(&streaming.wanted, w);
-
- out:
-  pthread_mutex_unlock(&streaming_wanted_lck);
-}
-
-// Not thread safe, but only called once during httpd init
-void
-streaming_metadatacb_register(streaming_metadatacb cb)
-{
-  streaming.metadatacb = cb;
-}
-
 /* ----------------------------- Thread: Player ----------------------------- */
 
 static void
-encode_worker_invoke(uint8_t *buf, size_t bufsize, int samples, struct media_quality quality)
+streaming_write(struct output_buffer *obuf)
 {
   struct encode_cmdarg ctx;
 
-  if (quality.channels == 0)
+  // No lock since this is just an early exit, it doesn't need to be accurate
-    {
+  if (!streaming.wanted)
-      DPRINTF(E_LOG, L_STREAMING, "Streaming quality is zero (%d/%d/%d)\n",
+    return;
-	quality.sample_rate, quality.bits_per_sample, quality.channels);
-      return;
-    }
 
-  CHECK_NULL(L_STREAMING, ctx.buf = malloc(bufsize));
+  // We don't want to block the player, so we can't lock to access
-  memcpy(ctx.buf, buf, bufsize);
+  // streaming.wanted and find which qualities we need. So we just copy it all
-  ctx.bufsize = bufsize;
+  // and pass it to a worker thread that can lock and check what is wanted, and
-  ctx.samples = samples;
+  // also can encode without holding the player.
-  ctx.quality = quality;
+  ctx.obuf = outputs_buffer_copy(obuf);
   ctx.seqnum = streaming.seqnum;
 
   streaming.seqnum++;
 
   worker_execute(encode_data_cb, &ctx, sizeof(struct encode_cmdarg), 0);
-}
-
-static void
-silenceev_cb(evutil_socket_t fd, short event, void *arg)
-{
-  uint8_t silence[SILENCE_BUF_SIZE] = { 0 };
-  int samples;
-
-  // No lock since this is just an early exit, it doesn't need to be accurate
-  if (!streaming.wanted)
-    return;
-
-  samples = BTOS(SILENCE_BUF_SIZE, streaming.last_quality.bits_per_sample, streaming.last_quality.channels);
-
-  encode_worker_invoke(silence, SILENCE_BUF_SIZE, samples, streaming.last_quality);
-
-  evtimer_add(streaming.silenceev, &streaming.silencetv);
-}
-
-static void
-streaming_write(struct output_buffer *obuf)
-{
-  // No lock since this is just an early exit, it doesn't need to be accurate
-  if (!streaming.wanted)
-    return;
-
-  encode_worker_invoke(obuf->data[0].buffer, obuf->data[0].bufsize, obuf->data[0].samples, obuf->data[0].quality);
-
-  streaming.last_quality = obuf->data[0].quality;
 
   // In case this is the last player write() we want to start streaming silence
   evtimer_add(streaming.silenceev, &streaming.silencetv);
 }
 
+static void
+silenceev_cb(evutil_socket_t fd, short event, void *arg)
+{
+  streaming_write(NULL);
+}
+
 static void
 streaming_metadata_send(struct output_metadata *metadata)
 {
-  char *title = metadata->priv;
+  // Nothing to do, metadata_prepare() did all we needed in a worker thread
+}
 
-  // Calls back to httpd_streaming to update the title
+// Since this is streaming and there is no actual device, we will be called with
-  if (streaming.metadatacb)
+// a dummy/ad hoc device that's not part in the speaker list. We don't need to
-    streaming.metadatacb(title);
+// make any callback so can ignore callback_id.
+static int
+streaming_start(struct output_device *device, int callback_id)
+{
+  struct streaming_wanted *w;
+  int ret;
 
-  free(title);
+  pthread_mutex_lock(&streaming_wanted_lck);
-  outputs_metadata_free(metadata);
+  w = wanted_find_byformat(streaming.wanted, device->format, device->quality);
+  if (!w)
+    w = wanted_add(&streaming.wanted, device->format, device->quality);
+  ret = wanted_session_add(&device->audio_fd, &device->metadata_fd, w);
+  if (ret < 0)
+    goto error;
+  pthread_mutex_unlock(&streaming_wanted_lck);
+
+  worker_execute(metadata_startup_cb, &(device->metadata_fd), sizeof(device->metadata_fd), 0);
+
+  outputs_quality_subscribe(&device->quality);
+
+  device->id = device->audio_fd;
+  return 0;
+
+ error:
+  if (w->num_sessions == 0)
+    wanted_remove(&streaming.wanted, w);
+  pthread_mutex_unlock(&streaming_wanted_lck);
+  return -1;
+}
+
+// Since this is streaming and there is no actual device, we will be called with
+// a dummy/ad hoc device that's not part in the speaker list. We don't need to
+// make any callback so can ignore callback_id.
+static int
+streaming_stop(struct output_device *device, int callback_id)
+{
+  struct streaming_wanted *w;
+
+  pthread_mutex_lock(&streaming_wanted_lck);
+  w = wanted_find_byreadfd(streaming.wanted, device->id);
+  if (!w)
+    goto error;
+  device->quality = w->quality;
+  wanted_session_remove(w, device->id);
+  if (w->num_sessions == 0)
+    wanted_remove(&streaming.wanted, w);
+  pthread_mutex_unlock(&streaming_wanted_lck);
+
+  outputs_quality_unsubscribe(&device->quality);
+  return 0;
+
+ error:
+  pthread_mutex_unlock(&streaming_wanted_lck);
+  return -1;
 }
 
 static int
@@ -606,13 +690,16 @@ streaming_deinit(void)
 
 struct output_definition output_streaming =
 {
-  .name = "mp3 streaming",
+  .name = "streaming",
   .type = OUTPUT_TYPE_STREAMING,
   .priority = 0,
   .disabled = 0,
   .init = streaming_init,
   .deinit = streaming_deinit,
   .write = streaming_write,
+  .device_start = streaming_start,
+  .device_probe = streaming_start,
+  .device_stop = streaming_stop,
   .metadata_prepare = streaming_metadata_prepare,
   .metadata_send = streaming_metadata_send,
 };

src/outputs/streaming.h

@@ -1,23 +0,0 @@
-
-#ifndef __STREAMING_H__
-#define __STREAMING_H__
-
-#include "misc.h" // struct media_quality
-
-typedef void (*streaming_metadatacb)(char *metadata);
-
-enum streaming_format
-{
-  STREAMING_FORMAT_MP3,
-};
-
-int
-streaming_session_register(enum streaming_format format, struct media_quality quality);
-
-void
-streaming_session_deregister(int readfd);
-
-void
-streaming_metadatacb_register(streaming_metadatacb cb);
-
-#endif /* !__STREAMING_H__ */

src/player.c

@@ -151,6 +151,12 @@ struct speaker_attr_param
   bool prevent_playback;
   bool busy;
 
+  struct media_quality quality;
+  enum player_format format;
+
+  int audio_fd;
+  int metadata_fd;
+
   const char *pin;
 };
 
@@ -2883,6 +2889,48 @@ speaker_start_all(void *arg, int *retval)
   return COMMAND_END;
 }
 
+// This is borderline misuse of the outputs_device interface, but the purpose is
+// to register streaming session info with outputs/streaming.c via the player
+// thread. It must be the player thread because session setup requires that
+// outputs_quality_subscribe() is called, and by design it isn't thread safe.
+static enum command_state
+streaming_register(void *arg, int *retval)
+{
+  struct speaker_attr_param *param = arg;
+  struct output_device device =
+  {
+    .type = OUTPUT_TYPE_STREAMING,
+    .type_name = "streaming",
+    .name = "streaming",
+    .quality = param->quality,
+    .format = param->format,
+  };
+
+  *retval = outputs_device_start(&device, NULL, false);
+
+  param->spk_id = device.id;
+  param->audio_fd = device.audio_fd;
+  param->metadata_fd = device.metadata_fd;
+  return COMMAND_END;
+}
+
+static enum command_state
+streaming_deregister(void *arg, int *retval)
+{
+  struct speaker_attr_param *param = arg;
+  struct output_device device =
+  {
+    .type = OUTPUT_TYPE_STREAMING,
+    .type_name = "streaming",
+    .name = "streaming",
+    .id = param->spk_id,
+    .session = "dummy", // to pass check in outputs_device_stop()
+  };
+
+  *retval = outputs_device_stop(&device, NULL);
+  return COMMAND_END;
+}
+
 static enum command_state
 volume_set(void *arg, int *retval)
 {
@@ -3138,7 +3186,7 @@ player_get_status(struct player_status *status)
 }
 
 
-/* --------------------------- Thread: httpd (DACP) ------------------------- */
+/* ------------------------------ Thread: httpd ----------------------------- */
 
 /*
  * Stores the now playing media item dbmfi-id in the given id pointer.
@@ -3424,6 +3472,37 @@ player_speaker_authorize(uint64_t id, const char *pin)
   return ret;
 }
 
+int
+player_streaming_register(int *audio_fd, int *metadata_fd, enum player_format format, struct media_quality quality)
+{
+  struct speaker_attr_param param;
+  int ret;
+
+  param.format = format;
+  param.quality = quality;
+
+  ret = commands_exec_sync(cmdbase, streaming_register, NULL, &param);
+  if (ret < 0)
+    return ret;
+
+  *audio_fd = param.audio_fd;
+  *metadata_fd = param.metadata_fd;
+  return param.spk_id;
+}
+
+int
+player_streaming_deregister(int id)
+{
+  struct speaker_attr_param param;
+  int ret;
+
+  param.spk_id = id;
+
+  ret = commands_exec_sync(cmdbase, streaming_deregister, NULL, &param);
+
+  return ret;
+}
+
 int
 player_volume_set(int vol)
 {

src/player.h

@@ -6,6 +6,7 @@
 #include <stdint.h>
 
 #include "db.h"
+#include "misc.h" // for struct media_quality
 
 // Maximum number of previously played songs that are remembered
 #define MAX_HISTORY_COUNT 20
@@ -27,6 +28,10 @@ enum player_seek_mode {
   PLAYER_SEEK_RELATIVE = 2,
 };
 
+enum player_format {
+  PLAYER_FORMAT_MP3,
+};
+
 struct player_speaker_info {
   uint64_t id;
   uint32_t active_remote;
@@ -117,6 +122,12 @@ player_speaker_resurrect(void *arg);
 int
 player_speaker_authorize(uint64_t id, const char *pin);
 
+int
+player_streaming_register(int *audio_fd, int *metadata_fd, enum player_format format, struct media_quality quality);
+
+int
+player_streaming_deregister(int id);
+
 int
 player_playback_start(void);
 

src/transcode.c

@@ -2202,6 +2202,11 @@ transcode_encode_query(struct encode_ctx *ctx, const char *query)
 	return ctx->audio_stream.stream->codecpar->channels;
 #endif
     }
+  else if (strcmp(query, "samples_per_frame") == 0)
+    {
+      if (ctx->audio_stream.stream)
+	return ctx->audio_stream.stream->codecpar->frame_size;
+    }
 
   return -1;
 }