[mpd-devel] [PATCH 1/2] httpd: add a parameter for playing silence

[Ben Boeckel]

Some clients act weird in the face of silence and buffer it rather thay
"playing" it. This causes severe lag when the stream is unpaused.

Stopping clients on mobile devices is also good since it preserves
battery life by allowing them to power down their wireless components
for more power savings.

Signed-off-by: Ben Boeckel <***@gmail.com>
---
doc/mpdconf.example | 1 +
doc/user.xml | 10 ++++++++++
src/output/plugins/httpd/HttpdInternal.hxx | 5 +++++
src/output/plugins/httpd/HttpdOutputPlugin.cxx | 4 +++-
4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/doc/mpdconf.example b/doc/mpdconf.example
index 87d3021..781c3a9 100644
--- a/doc/mpdconf.example
+++ b/doc/mpdconf.example
@@ -275,6 +275,7 @@ input {
# bitrate "128" # do not define if quality is defined
# format "44100:16:1"
# max_clients "0" # optional 0=no limit
+# play_silence "yes" # optional, play silence when paused
#}
#
# An example of a pulseaudio output (streaming to a remote pulseaudio server)
diff --git a/doc/user.xml b/doc/user.xml
index f66f6e3..af6b3af 100644
--- a/doc/user.xml
+++ b/doc/user.xml
@@ -2846,6 +2846,16 @@ buffer_size: 16384</programlisting>
to 0 no limit will apply.
</entry>
</row>
+ <row>
+ <entry>
+ <varname>play_silence</varname>
+ <parameter>PS</parameter>
+ </entry>
+ <entry>
+ If <literal>true</literal>, silence will be played
+ while the stream is paused.
+ </entry>
+ </row>
</tbody>
</tgroup>
</informaltable>
diff --git a/src/output/plugins/httpd/HttpdInternal.hxx b/src/output/plugins/httpd/HttpdInternal.hxx
index 01498df..bb87ed4 100644
--- a/src/output/plugins/httpd/HttpdInternal.hxx
+++ b/src/output/plugins/httpd/HttpdInternal.hxx
@@ -125,6 +125,11 @@ private:
*/
char const *website;

+ /**
+ * True if the plugin should play silence when paused.
+ */
+ bool play_silence;
+
private:
/**
* A linked list containing all clients which are currently
diff --git a/src/output/plugins/httpd/HttpdOutputPlugin.cxx b/src/output/plugins/httpd/HttpdOutputPlugin.cxx
index 765a72d..78a4088 100644
--- a/src/output/plugins/httpd/HttpdOutputPlugin.cxx
+++ b/src/output/plugins/httpd/HttpdOutputPlugin.cxx
@@ -113,6 +113,8 @@ HttpdOutput::Configure(const ConfigBlock &block, Error &error)

clients_max = block.GetBlockValue("max_clients", 0u);

+ play_silence = block.GetBlockValue("play_silence", true);
+
/* set up bind_to_address */

const char *bind_to_address = block.GetBlockValue("bind_to_address");
@@ -481,7 +483,7 @@ httpd_output_pause(AudioOutput *ao)
{
HttpdOutput *httpd = HttpdOutput::Cast(ao);

- if (httpd->LockHasClients()) {
+ if (httpd->play_silence && httpd->LockHasClients()) {
static const char silence[1020] = { 0 };
return httpd_output_play(ao, silence, sizeof(silence),
IgnoreError()) > 0;

[Ben Boeckel]

These are other popular clients. In particular, VLC is available on
mobile devices.

Signed-off-by: Ben Boeckel <***@gmail.com>
---
doc/user.xml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/user.xml b/doc/user.xml
index af6b3af..e88a2eb 100644
--- a/doc/user.xml
+++ b/doc/user.xml
@@ -2787,7 +2787,8 @@ buffer_size: 16384</programlisting>
/ <ulink
url="http://icecast.org/"><application>IceCast</application></ulink>.
HTTP streaming clients like
- <application>mplayer</application> can connect to it.
+ <application>mplayer</application>, <application>VLC</application>,
+ and <application>mpv</application> can connect to it.
</para>

<para>

[Max Kellermann]

Post by Ben Boeckel
These are other popular clients. In particular, VLC is available on
mobile devices.

Merged.

[Ben Boeckel]

Post by Ben Boeckel
Some clients act weird in the face of silence and buffer it rather thay
"playing" it. This causes severe lag when the stream is unpaused.
Stopping clients on mobile devices is also good since it preserves
battery life by allowing them to power down their wireless components
for more power savings.

Bump?

--Ben

[Max Kellermann]

Post by Ben Boeckel
Some clients act weird in the face of silence and buffer it rather thay
"playing" it. This causes severe lag when the stream is unpaused.

There are other clients which time out when not getting any data.
(All audio streaming software sucks (and that includes MPD). No
matter what you do, at least one program breaks.)

Though I find it surprising to buffer silence. I don't want to
support software that is so obviously broken. A streaming client
should not care if it's silence or music that's being received - just
play what you get, damnit!

But I understand your desire to control MPD's behavior when the user
presses pause. Since HTTP streaming is all but a well-defined
protocol, there is no right and no wrong.

Post by Ben Boeckel
+ <row>
+ <entry>
+ <varname>play_silence</varname>
+ <parameter>PS</parameter>
+ </entry>
+ <entry>
+ If <literal>true</literal>, silence will be played
+ while the stream is paused.
+ </entry>
+ </row>

I don't like the option's name, because it's hard to understand what
it does.

Your documentation describes only what it does when configured "yes",
which is the old behavior - but it does not describe the new behavior
at all! What does it do when "no" is configured? And what's the
default?

[Ben Boeckel]

Post by Max Kellermann

Post by Ben Boeckel
Some clients act weird in the face of silence and buffer it rather thay
"playing" it. This causes severe lag when the stream is unpaused.

There are other clients which time out when not getting any data.
(All audio streaming software sucks (and that includes MPD). No
matter what you do, at least one program breaks.)

Agreed. The thing is that when the playlist ends or is cleared (even if
immediately populated, but that may be a client thing and the protocol
really is clear/populate in separate steps), all clients quit anyways,
so I have to protect against it no matter what for such "always-on"
players.

Post by Max Kellermann
Though I find it surprising to buffer silence. I don't want to
support software that is so obviously broken. A streaming client
should not care if it's silence or music that's being received - just
play what you get, damnit!

Well, I'm not 100% sure it is buffering, but it does act weird (the
message mpv outputs is "[cache] Cache is not responding - slow/stuck
network connection?"). I'll try to do some more digging in to what
happens, but mplayer had similar behavior with respect to the silence
stream, so it is likely deep in the historical baggage, maybe even
FFmpeg. Maybe MPD is not sending proper timestamps with the silence (for
an OGG stream; I don't use others to know for those)?

Post by Max Kellermann

Post by Ben Boeckel
+ <row>
+ <entry>
+ <varname>play_silence</varname>
+ <parameter>PS</parameter>
+ </entry>
+ <entry>
+ If <literal>true</literal>, silence will be played
+ while the stream is paused.
+ </entry>
+ </row>

I don't like the option's name, because it's hard to understand what
it does.

Hmm. How about "play_silence_when_paused". Too long? "_on_pause"? No
"play_"?

Post by Max Kellermann
Your documentation describes only what it does when configured "yes",
which is the old behavior - but it does not describe the new behavior
at all! What does it do when "no" is configured? And what's the
default?

Thanks, will add more to it when I get the chance.

--Ben

[Max Kellermann]

Post by Ben Boeckel
Hmm. How about "play_silence_when_paused". Too long? "_on_pause"? No
"play_"?

Maybe just "silence_paused"? That would give a hint on what it's
about.

It's hard to find the perfect name, but "play_silence" sounds bad to
me because it's not about regular "playback".

[Ben Boeckel]

Some clients act weird in the face of silence and buffer it rather thay
"playing" it. This causes severe lag when the stream is unpaused.

Stopping clients on mobile devices is also good since it preserves
battery life by allowing them to power down their wireless components
for more power savings.

Signed-off-by: Ben Boeckel <***@gmail.com>
---
doc/mpdconf.example | 1 +
doc/user.xml | 12 ++++++++++++
src/output/plugins/httpd/HttpdInternal.hxx | 6 ++++++
src/output/plugins/httpd/HttpdOutputPlugin.cxx | 4 +++-
4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/doc/mpdconf.example b/doc/mpdconf.example
index 2609c38c1..85e8b5615 100644
--- a/doc/mpdconf.example
+++ b/doc/mpdconf.example
@@ -275,6 +275,7 @@ input {
# bitrate "128" # do not define if quality is defined
# format "44100:16:1"
# max_clients "0" # optional 0=no limit
+# silence_paused "yes" # optional, send silence (rather than no data) when paused
#}
#
# An example of a pulseaudio output (streaming to a remote pulseaudio server)
diff --git a/doc/user.xml b/doc/user.xml
index 865369c23..38b77d7be 100644
--- a/doc/user.xml
+++ b/doc/user.xml
@@ -3650,6 +3650,18 @@ run</programlisting>
to 0 no limit will apply.
</entry>
</row>
+ <row>
+ <entry>
+ <varname>silence_paused</varname>
+ <parameter>PS</parameter>
+ </entry>
+ <entry>
+ If <literal>true</literal>, silence will be played
+ while the stream is paused. If <literal>false</literal>, the
+ stream will send no data, the same behavior for a stopped
+ stream.
+ </entry>
+ </row>
</tbody>
</tgroup>
</informaltable>
diff --git a/src/output/plugins/httpd/HttpdInternal.hxx b/src/output/plugins/httpd/HttpdInternal.hxx
index 762c5385a..745c90173 100644
--- a/src/output/plugins/httpd/HttpdInternal.hxx
+++ b/src/output/plugins/httpd/HttpdInternal.hxx
@@ -130,6 +130,12 @@ private:
*/
char const *website;

+ /**
+ * True if the plugin should play silence when paused rather than not
+ * sending any data.
+ */
+ bool silence_paused;
+
private:
/**
* A linked list containing all clients which are currently
diff --git a/src/output/plugins/httpd/HttpdOutputPlugin.cxx b/src/output/plugins/httpd/HttpdOutputPlugin.cxx
index b5c48ea8b..4b42fdf9e 100644
--- a/src/output/plugins/httpd/HttpdOutputPlugin.cxx
+++ b/src/output/plugins/httpd/HttpdOutputPlugin.cxx
@@ -70,6 +70,8 @@ HttpdOutput::HttpdOutput(EventLoop &_loop, const ConfigBlock &block)

clients_max = block.GetBlockValue("max_clients", 0u);

+ silence_paused = block.GetBlockValue("silence_paused", true);
+
/* set up bind_to_address */

const char *bind_to_address = block.GetBlockValue("bind_to_address");
@@ -382,7 +384,7 @@ HttpdOutput::Play(const void *chunk, size_t size)
bool
HttpdOutput::Pause()
{
- if (LockHasClients()) {
+ if (silence_paused && LockHasClients()) {
static const char silence[1020] = { 0 };
Play(silence, sizeof(silence));
}