[streaming] support multiple output formats #982

whatdoineed2do · 2020-05-01T17:38:48Z

closes #971 - support additional streaming formats to mp3

The work consists mostly of rework of all static globals used to track and manage streaming are now moved to a streaming_ctx structure which represents a single streaming format (ie mp3, pcm ~~44.1, pcm 48~~). For each different stream format we have another streaming_ctx structure.

The supported streaming formats are defined in the streaming_ctxs[] array. Streams available at:

Tested this with multiple concurrent mp3/pcm streams (played with mpv) sub/unsub/resub and it seems good. ~~Will leave this in draft for requester @frankusb to help with testing and structural comments~~

There is one bug fix dealing with handling unsupported encoding and a commit to limit the number of client streams - the last one is to address the potential resource starvation in the shared httpd event_base loop

mill1000 · 2020-05-01T19:50:39Z

I know this is not really the place to make a feature request but supporting HLS could be pretty cool!

whatdoineed2do · 2020-05-02T14:39:35Z

@ejurgensen - this should be good to ask for your input on the refactoring to support additonal streaming formats.

I've given this a bit more testing and there's nothing untoward I see from the refactoring.

whatdoineed2do · 2020-05-02T14:44:57Z

src/httpd_streaming.c

-      streaming_not_supported = 1;
+      DPRINTF(E_LOG, L_STREAMING, "Will not be able to stream %s, libav does not support encoding: %d/%d/%d @ %d\n", ctx->name, ctx->quality_out.sample_rate, ctx->quality_out.bits_per_sample, ctx->quality_out.channels, ctx->quality_out.bit_rate);
+      ctx->not_supported = 1;
+      streaming_end(ctx);


The addition of streaming_end(..) is a bug fix - if we don't have this, the sessions list is never cleared down so the player will continue to send data and we reenter here.

whatdoineed2do · 2020-05-09T11:56:09Z

@ejurgensen - do you still want to proceed with this enhancement or comments on the structure here?

I think the original requestor wanted to stream this to their Roku Soundbridge but it appears that the Roku does not support a http/streamed pcm and therefore that particular use case is no longer valid.

It does not negate the work here though as the structure should easily allow additional formats (maybe HLS or aac) to be supported if transcode module supports it.

ejurgensen · 2020-05-19T21:33:14Z

I haven't looked so much into this, but I think more formats is fine, so it's on my todo list.

I am wondering about the choice of pcm, why not at least wav? Pcm has no headers, so the receiver has to know sample rate and format a priori. Maybe that is why it is not working on the Roku?

Wav is also a bit of bandwidth hog, what about flac or some other lossless format?

frankusb · 2020-05-20T00:34:42Z

For my use case, soundbridge, wav is a known option. Windows lossless is also supported by the soundbridge. I recently did some experiments with Plex transcoding from FLAC to wav with the soundbridge over upnp and was unsuccessful there as well. When I finish converting all my music from mp3 to FLAC, I will be circling back around to both forked-daapd and plex experimentation.

…

On Tue, May 19, 2020 at 4:33 PM ejurgensen ***@***.***> wrote: I haven't looked so much into this, but I think more formats is fine, so it's on my todo list. I am wondering about the choice of pcm, why not at least wav? Pcm has no headers, so the receiver has to know sample rate and format a priori. Maybe that is why it is not working on the Roku? Wav is also a bit of bandwidth hog, what about flac or some other lossless format? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#982 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APETTJRNG5GXSPQ6ZDBSR7TRSL3KRANCNFSM4MXH4OCA> .

whatdoineed2do · 2020-05-20T10:47:14Z

@ejurgensen - yes, the PCM/wav was added to help with the original @frankusb request. In fact, the code is sending headers so the name is misleading and I should fix that - as for the Soundbridge not supporting the stream, it might be that SB doesn't support anything other than streamed mp3.

supported Roku streaming formats

The following HTTP-streamed formats are currently supported:

MP3 streams (usually ending in .pls or .m3u)

WMA streams (usually ending in .asx). You will need the 2.5 software installed on the SoundBridge to play these back.

There is preliminary support for the following stream format on most SoundBridge models:

AAC+ streams

However the primary focus of this PR should really be about the structure/refactor of the streaming module to be able to support other streaming formats. I can drop the PCM stream from this PR (it's just dropping it out of streaming_ctx streaming_ctxs[]) and we can add that back in another PR to support the different streamable formats (flac, opus...) once these are added to transcode

frankusb · 2020-05-20T13:19:16Z

A couple other thoughts. Roku Soundbridge also supports Apple Lossless (ALAC).

During my Plex UPnP experiment, I noticed that when the Soundbridge requested a file to play, Plex responded with an http link. So I think that will be my next experiment. Put a WAV file on Plex and see if it provides it via http and if the Soundbridge can play it.

whatdoineed2do · 2020-05-20T17:01:11Z

@frankusb - can you try to setup Plex to stream to your roku

mp3 (your control case)
WMA / .asx
AAC (or AAC HE aka AAC+ I believe)
wav

The first two are apparently supported based on Roku's old old webite link (https://soundbridge.roku.com/soundbridge/internet.php). If you can serve the top 2 to the roku but not the wav, it'd suggest the Roku doesnt support that streamming format.

frankusb · 2020-05-20T20:14:13Z

@whatdoineed2do, my results are.

mp3 plays fine with Plex
WMAV2 @ 320kbps plays fine with Plex
AAC attempts to play but immediately gives an option to stop or play next
WAV plays but it's white noise. Oddly the Soundbridge labels it 48kHz but Plex recognizes its 44.1kHz
ALAC seems to be filtered by the Soundbridge, it has an x by the Song Title, same as FLAC

whatdoineed2do · 2020-05-21T11:07:16Z

@ejurgensen - I've split this PR off into structural change so that we're not relying on globals/can add other streaming formats along with some additional commits to add streaming support for wav and flac - the latter of which we can drop easily out of this PR if you feel like that needs a little more attetion.

If you can take a look this'll be great.

@frankusb - thanks, it does suggest that the SB doesnt like those other formats (non MP3/wma) as per the link I shared.

frankusb · 2020-06-02T21:38:35Z

Good news, I got WAV playing from Plex today. The Soundbridge shows Kind: WAV audio file while playing back.

I believe the key was adding:

<DlnaMediaProfiles> <DlnaMusicProfile container="wav" mimeType="audio/wav" /> </DlnaMediaProfiles>

The Plex log below

Jun 02, 2020 16:32:06.038 [0x7f80dbfff700] DEBUG - Proxied GET to http://127.0.0.1:32400/library/parts/124454/1591130247/file.wav?X-Plex-Token=xxxxxxxxxxxxxxxxxxxx: HTTP/1.1 200
Jun 02, 2020 16:32:06.038 [0x7f80dbfff700] VERBOSE - Proxy response headers: X-Plex-Protocol: 1.0; ETag: "31862d4588b5dc9bd054fc9574131efd5a83e840"; Last-Modified: Tue, 02 Jun 2020 21:28:49 GMT; Accept-Ranges: bytes; Connection: Keep-Alive; Keep-Alive: timeout=20; Content-Length: 45472072; Content-Type: audio/wav; Date: Tue, 02 Jun 2020 21:32:06 GMT
Jun 02, 2020 16:32:06.038 [0x7f80dbfff700] DEBUG - Responding HTTP/1.1 200
Jun 02, 2020 16:32:06.038 [0x7f80dbfff700] VERBOSE - Response headers: Content-Length: 45472072; Content-Type: audio/wav; Date: Tue, 02 Jun 2020 21:32:06 GMT; Accept-Ranges: bytes

ejurgensen

I've had a look at this pr, but didn't get to go through it all. There is one basic issue, which is that the change involves having the streaming output do it's own transcoding, and output modules are not supposed to do that. They should leave this to the output via the subscription function. This has the advantage that the output middle layer takes care that no double transcoding is done. So you need to refactor this PR to make use of this. Perhaps you should make an entirely new PR, and just close this one. Some of my other comments probably won't apply either after such a refactor.

ejurgensen · 2020-06-08T09:34:19Z

src/httpd_streaming.c


-// Means we're not able to encode to mp3
-static bool streaming_not_supported;
+static struct streaming_ctx streaming_ctxs[] = {


Below you only need to define the stuff that is non-zero

ejurgensen · 2020-06-08T09:43:58Z

src/httpd_streaming.c

  struct streaming_session *this;
  struct streaming_session *session;
  struct streaming_session *prev;
  char *address;
  ev_uint16_t port;

-  this = (struct streaming_session *)arg;
+  this = ctx->sessions;


Not used any more (plus it seems confusing that "this" is the head of the list, not the actual session)

ejurgensen · 2020-06-08T09:44:14Z

src/httpd_streaming.c

-  pthread_mutex_lock(&streaming_sessions_lck);
-  if (!streaming_sessions)
+  pthread_mutex_lock(&ctx->sessions_lck);
+  if (!ctx->sessions)


I think there might be something wrong here. Before "this" pointed to the session, but now it looks like it points to the head of the list - so is the name still correct? Also why is "this" being set, but not used in the if test above? Plus the comment says that "this" may be a dangling pointer, and in that case (!ctx->sessions) will not be true, so in that context the comment doesn't make sense any more.

ejurgensen · 2020-06-08T09:50:07Z

src/httpd_streaming.c

  else
    prev->next = session->next;

+  ++(ctx->available_sessions);


Counters like this are dangerous, they easily go out of sync without any clear signs. When operating with a fixed max number of sessions it is maybe better to have corresponding fixed array size of ctx->sessions (so not a linked list). When you want a new session you look for a free slot, and when a session ends you zero a slot.

ejurgensen · 2020-06-08T09:56:44Z

src/httpd_streaming.c

+
+  while (ctx->name)
+    {
+      pthread_mutex_lock(&ctx->sessions_lck);


You cannot lock here, since it can block the player thread. I think you need to keep the simple global variable here.

ejurgensen · 2020-06-08T10:07:37Z

src/httpd_streaming.c

+  const enum transcode_profile xcode;
+
+  pthread_mutex_t sessions_lck;
+  struct streaming_session *sessions;


The data model seems to be two way, since streaming_session->ctx points at a streaming_ctx, and streaming_ctx->sessions points at a list of sessions. That means ownership is somewhat unclear. I think you should consider a more simple model where you don't point both ways. I would think that you should have streaming_session->ctx, but not the other way around.

ejurgensen · 2020-06-08T10:13:35Z

src/httpd_streaming.c

-      ret = write(streaming_meta[1], &obuf->data[0].quality, sizeof(struct media_quality));
+      pthread_mutex_lock(&ctx->sessions_lck);
+      if (!ctx->sessions)
+	goto next;


Just unlock and continue, don't use goto for this

whatdoineed2do · 2020-06-11T12:07:06Z

Let me close this and have another look when I free up. In the meantime, I'll put in the oneliner fix for when libav doesnt support MP3 encoding since currently it doesnt drop the client leading to the server always trying to get a decoding ctx

whatdoineed2do force-pushed the streaming-multiformat branch 2 times, most recently from bc2de86 to 4104639 Compare May 1, 2020 18:48

whatdoineed2do commented May 2, 2020

View reviewed changes

whatdoineed2do marked this pull request as ready for review May 3, 2020 12:16

whatdoineed2do/Ray added 4 commits May 20, 2020 19:46

[streaming] refactor to allow supporting multiple output formats

a5b34b2

[streaming] drop client connections if stream/encoding unsupported

ea30311

[streaming] allow limitting of total streaming sessions

e67db2c

[streaming] support wav/pcm streaming

74e89ab

whatdoineed2do force-pushed the streaming-multiformat branch from a80518e to 74e89ab Compare May 20, 2020 18:57

whatdoineed2do/Ray added 3 commits May 20, 2020 20:48

[xcode] additional info on bug on mismatch

3e213e7

[xcode] support flac encode

f98385c

[streaming] support flac streaming

092e532

whatdoineed2do force-pushed the streaming-multiformat branch from 6c546f3 to 092e532 Compare May 20, 2020 20:02

ejurgensen reviewed Jun 8, 2020

View reviewed changes

whatdoineed2do closed this Jun 11, 2020

frankusb mentioned this pull request Mar 3, 2022

FLAC network streaming #1419

Open

whatdoineed2do mentioned this pull request May 5, 2022

[streaming] refactor to use outputs_quality_subscribe #1467

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[streaming] support multiple output formats #982

[streaming] support multiple output formats #982

whatdoineed2do commented May 1, 2020 •

edited

Loading

mill1000 commented May 1, 2020

whatdoineed2do commented May 2, 2020

whatdoineed2do May 2, 2020

whatdoineed2do commented May 9, 2020

ejurgensen commented May 19, 2020

frankusb commented May 20, 2020 via email

whatdoineed2do commented May 20, 2020 •

edited

Loading

frankusb commented May 20, 2020

whatdoineed2do commented May 20, 2020

frankusb commented May 20, 2020

whatdoineed2do commented May 21, 2020

frankusb commented Jun 2, 2020

ejurgensen left a comment

ejurgensen Jun 8, 2020

ejurgensen Jun 8, 2020

ejurgensen Jun 8, 2020

ejurgensen Jun 8, 2020

ejurgensen Jun 8, 2020

ejurgensen Jun 8, 2020

ejurgensen Jun 8, 2020

whatdoineed2do commented Jun 11, 2020

[streaming] support multiple output formats #982

[streaming] support multiple output formats #982

Conversation

whatdoineed2do commented May 1, 2020 • edited Loading

mill1000 commented May 1, 2020

whatdoineed2do commented May 2, 2020

Choose a reason for hiding this comment

whatdoineed2do commented May 9, 2020

ejurgensen commented May 19, 2020

frankusb commented May 20, 2020 via email

whatdoineed2do commented May 20, 2020 • edited Loading

frankusb commented May 20, 2020

whatdoineed2do commented May 20, 2020

frankusb commented May 20, 2020

whatdoineed2do commented May 21, 2020

frankusb commented Jun 2, 2020

ejurgensen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whatdoineed2do commented Jun 11, 2020

whatdoineed2do commented May 1, 2020 •

edited

Loading

whatdoineed2do commented May 20, 2020 •

edited

Loading