From 5b2374b9495bb9da4b07d03328c1ecac204af8d4 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Sun, 23 Sep 2018 15:26:42 +0200
Subject: [PATCH] player/Thread: calculate `buffered_before_play` based on a
 fixed duration

Previously, there was the setting `buffered_before_play` which
specified a percentage of the audio buffer, defaulting to `10%`.  That
was working well enough for quite some time, until high-quality audio
formats became common.

At 44.1 kHz, 16 bit stereo, MPD collected 2.3 seconds worth of data in
the buffer before starting playback.  With the same default settings
and 192 kHz, 24 bit stereo, that was only 0.27 seconds.

Making this depend on the byte size only leads to high latency at low
quality, and too little data at high quality.  The natural choice
would be to use a duration instead of a byte size, which should give
the same good experience with all audio formats.

Since the `buffered_before_play` configuration setting was not
understood well by users and caused more harm than good, this commit
deprecates it.  It has now no effect.
---
 NEWS                     |  1 +
 src/config/Templates.cxx |  2 +-
 src/player/Thread.cxx    | 18 +++++++++++++++---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 4fbbe18a2..b68853d3d 100644
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,7 @@ ver 0.21 (not yet released)
   - proxy: require libmpdclient 2.9
   - proxy: forward `sort` and `window` to server
 * player
+  - hard-code "buffer_before_play" to 1 second, independent of audio format
   - "one-shot" single mode
 * input
   - curl: download to buffer instead of throttling transfer
diff --git a/src/config/Templates.cxx b/src/config/Templates.cxx
index 3c2d78aae..8df588aca 100644
--- a/src/config/Templates.cxx
+++ b/src/config/Templates.cxx
@@ -54,7 +54,7 @@ const ConfigTemplate config_param_templates[] = {
 	{ "volume_normalization" },
 	{ "samplerate_converter" },
 	{ "audio_buffer_size" },
-	{ "buffer_before_play" },
+	{ "buffer_before_play", false, true },
 	{ "http_proxy_host", false, true },
 	{ "http_proxy_port", false, true },
 	{ "http_proxy_user", false, true },
diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx
index 017e4c5be..cd930c039 100644
--- a/src/player/Thread.cxx
+++ b/src/player/Thread.cxx
@@ -57,6 +57,12 @@
 
 static constexpr Domain player_domain("player");
 
+/**
+ * Start playback as soon as enough data for this duration has been
+ * pushed to the decoder pipe.
+ */
+static constexpr auto buffer_before_play_duration = std::chrono::seconds(1);
+
 class Player {
 	PlayerControl &pc;
 
@@ -80,9 +86,10 @@ class Player {
 
 	/**
 	 * Start playback as soon as this number of chunks has been
-	 * pushed to the decoder pipe.
+	 * pushed to the decoder pipe.  This is calculated based on
+	 * #buffer_before_play_duration.
 	 */
-	const unsigned buffer_before_play;
+	unsigned buffer_before_play;
 
 	/**
 	 * If the decoder pipe gets consumed below this threshold,
@@ -191,7 +198,6 @@ public:
 	Player(PlayerControl &_pc, DecoderControl &_dc,
 	       MusicBuffer &_buffer) noexcept
 		:pc(_pc), dc(_dc), buffer(_buffer),
-		 buffer_before_play(pc.buffered_before_play),
 		 decoder_wakeup_threshold(buffer.GetSize() * 3 / 4)
 	{
 	}
@@ -517,6 +523,12 @@ Player::CheckDecoderStartup() noexcept
 		play_audio_format = dc.out_audio_format;
 		decoder_starting = false;
 
+		const size_t buffer_before_play_size =
+			play_audio_format.TimeToSize(buffer_before_play_duration);
+		buffer_before_play =
+			(buffer_before_play_size + sizeof(MusicChunk::data) - 1)
+			/ sizeof(MusicChunk::data);
+
 		idle_add(IDLE_PLAYER);
 
 		if (pending_seek > SongTime::zero()) {