Merge tag 'v0.19.17'

Makefile.am

@@ -539,6 +539,7 @@ libpcm_a_SOURCES = \
 	src/pcm/PcmConvert.cxx src/pcm/PcmConvert.hxx \
 	src/pcm/PcmDop.cxx src/pcm/PcmDop.hxx \
 	src/pcm/Volume.cxx src/pcm/Volume.hxx \
+	src/pcm/Silence.cxx src/pcm/Silence.hxx \
 	src/pcm/PcmMix.cxx src/pcm/PcmMix.hxx \
 	src/pcm/PcmChannels.cxx src/pcm/PcmChannels.hxx \
 	src/pcm/PcmPack.cxx src/pcm/PcmPack.hxx \

NEWS

@@ -54,6 +54,16 @@ ver 0.20 (not yet released)
 * update
   - apply .mpdignore matches to subdirectories
 
+ver 0.19.17 (2016/07/09)
+* decoder
+  - flac: fix assertion failure while seeking
+  - flac: fix stream duration indicator
+  - fix seek problems in several plugins
+* fix spurious seek error "Failed to allocate silence buffer"
+* replay gain: fix "replay_gain_handler mixer" setting
+* DSD: use 0x69 as silence pattern
+* fix use-after-free bug on "close" and "kill"
+
 ver 0.19.16 (2016/06/13)
 * faster seeking
 * fix system include path order

src/client/ClientRead.cxx

@@ -52,8 +52,8 @@ Client::OnSocketInput(void *data, size_t length)
 		break;
 
 	case CommandResult::KILL:
-		Close();
 		partition.instance.Shutdown();
+		Close();
 		return InputResult::CLOSED;
 
 	case CommandResult::FINISH:

src/decoder/DecoderAPI.cxx

@@ -301,7 +301,8 @@ decoder_check_cancel_read(const Decoder *decoder)
 	/* ignore the SEEK command during initialization, the plugin
 	   should handle that after it has initialized successfully */
 	if (dc.command == DecoderCommand::SEEK &&
-	    (dc.state == DecoderState::START || decoder->seeking))
+	    (dc.state == DecoderState::START || decoder->seeking ||
+	     decoder->initial_seek_running))
 		return false;
 
 	return true;

src/decoder/plugins/FlacCommon.cxx

@@ -33,7 +33,7 @@ flac_data::flac_data(Decoder &_decoder,
 		     InputStream &_input_stream)
 	:FlacInput(_input_stream, &_decoder),
 	 initialized(false), unsupported(false),
-	 total_frames(0), first_frame(0), next_frame(0), position(0),
+	 position(0),
 	 decoder(_decoder), input_stream(_input_stream)
 {
 }
@@ -59,6 +59,38 @@ flac_sample_format(unsigned bits_per_sample)
 	}
 }
 
+bool
+flac_data::Initialize(unsigned sample_rate, unsigned bits_per_sample,
+		      unsigned channels, FLAC__uint64 total_frames)
+{
+	assert(!initialized);
+	assert(!unsupported);
+
+	::Error error;
+	if (!audio_format_init_checked(audio_format,
+				       sample_rate,
+				       flac_sample_format(bits_per_sample),
+				       channels, error)) {
+		LogError(error);
+		unsupported = true;
+		return false;
+	}
+
+	frame_size = audio_format.GetFrameSize();
+
+	const auto duration = total_frames > 0
+		? SignedSongTime::FromScale<uint64_t>(total_frames,
+						      audio_format.sample_rate)
+		: SignedSongTime::Negative();
+
+	decoder_initialized(decoder, audio_format,
+			    input_stream.IsSeekable(),
+			    duration);
+
+	initialized = true;
+	return true;
+}
+
 static void
 flac_got_stream_info(struct flac_data *data,
 		     const FLAC__StreamMetadata_StreamInfo *stream_info)
@@ -66,22 +98,10 @@ flac_got_stream_info(struct flac_data *data,
 	if (data->initialized || data->unsupported)
 		return;
 
-	Error error;
-	if (!audio_format_init_checked(data->audio_format,
-				       stream_info->sample_rate,
-				       flac_sample_format(stream_info->bits_per_sample),
-				       stream_info->channels, error)) {
-		LogError(error);
-		data->unsupported = true;
-		return;
-	}
-
-	data->frame_size = data->audio_format.GetFrameSize();
-
-	if (data->total_frames == 0)
-		data->total_frames = stream_info->total_samples;
-
-	data->initialized = true;
+	data->Initialize(stream_info->sample_rate,
+			 stream_info->bits_per_sample,
+			 stream_info->channels,
+			 stream_info->total_samples);
 }
 
 void flac_metadata_common_cb(const FLAC__StreamMetadata * block,
@@ -125,28 +145,11 @@ flac_got_first_frame(struct flac_data *data, const FLAC__FrameHeader *header)
 	if (data->unsupported)
 		return false;
 
-	Error error;
-	if (!audio_format_init_checked(data->audio_format,
-				       header->sample_rate,
-				       flac_sample_format(header->bits_per_sample),
-				       header->channels, error)) {
-		LogError(error);
-		data->unsupported = true;
-		return false;
-	}
-
-	data->frame_size = data->audio_format.GetFrameSize();
-
-	const auto duration = SongTime::FromScale<uint64_t>(data->total_frames,
-							    data->audio_format.sample_rate);
-
-	decoder_initialized(data->decoder, data->audio_format,
-			    data->input_stream.IsSeekable(),
-			    duration);
-
-	data->initialized = true;
-
-	return true;
+	return data->Initialize(header->sample_rate,
+				header->bits_per_sample,
+				header->channels,
+				/* unknown duration */
+				0);
 }
 
 FLAC__StreamDecoderWriteStatus
@@ -155,7 +158,6 @@ flac_common_write(struct flac_data *data, const FLAC__Frame * frame,
 		  FLAC__uint64 nbytes)
 {
 	void *buffer;
-	unsigned bit_rate;
 
 	if (!data->initialized && !flac_got_first_frame(data, &frame->header))
 		return FLAC__STREAM_DECODER_WRITE_STATUS_ABORT;
@@ -167,16 +169,12 @@ flac_common_write(struct flac_data *data, const FLAC__Frame * frame,
 		     data->audio_format.format, buf,
 		     0, frame->header.blocksize);
 
-	if (nbytes > 0)
-		bit_rate = nbytes * 8 * frame->header.sample_rate /
+	unsigned bit_rate = nbytes * 8 * frame->header.sample_rate /
 		(1000 * frame->header.blocksize);
-	else
-		bit_rate = 0;
 
 	auto cmd = decoder_data(data->decoder, data->input_stream,
 				buffer, buffer_size,
 				bit_rate);
-	data->next_frame += frame->header.blocksize;
 	switch (cmd) {
 	case DecoderCommand::NONE:
 	case DecoderCommand::START:

src/decoder/plugins/FlacCommon.hxx

@@ -55,23 +55,9 @@ struct flac_data : public FlacInput {
 	AudioFormat audio_format;
 
 	/**
-	 * The total number of frames in this song.  The decoder
-	 * plugin may initialize this attribute to override the value
-	 * provided by libFLAC (e.g. for sub songs from a CUE sheet).
+	 * End of last frame's position within the stream.  This is
+	 * used for bit rate calculations.
 	 */
-	FLAC__uint64 total_frames;
-
-	/**
-	 * The number of the first frame in this song.  This is only
-	 * non-zero if playing sub songs from a CUE sheet.
-	 */
-	FLAC__uint64 first_frame;
-
-	/**
-	 * The number of the next frame which is going to be decoded.
-	 */
-	FLAC__uint64 next_frame;
-
 	FLAC__uint64 position;
 
 	Decoder &decoder;
@@ -80,6 +66,12 @@ struct flac_data : public FlacInput {
 	Tag tag;
 
 	flac_data(Decoder &decoder, InputStream &input_stream);
+
+	/**
+	 * Wrapper for decoder_initialized().
+	 */
+	bool Initialize(unsigned sample_rate, unsigned bits_per_sample,
+			unsigned channels, FLAC__uint64 total_frames);
 };
 
 void flac_metadata_common_cb(const FLAC__StreamMetadata * block,

src/decoder/plugins/FlacDecoderPlugin.cxx

@@ -133,26 +133,16 @@ flac_decoder_new(void)
 }
 
 static bool
-flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd,
-			FLAC__uint64 duration)
+flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd)
 {
-	data->total_frames = duration;
-
 	if (!FLAC__stream_decoder_process_until_end_of_metadata(sd)) {
+		if (FLAC__stream_decoder_get_state(sd) != FLAC__STREAM_DECODER_END_OF_STREAM)
 			LogWarning(flac_domain, "problem reading metadata");
 		return false;
 	}
 
 	if (data->initialized) {
 		/* done */
-
-		const auto duration2 =
-			SongTime::FromScale<uint64_t>(data->total_frames,
-						      data->audio_format.sample_rate);
-
-		decoder_initialized(data->decoder, data->audio_format,
-				    data->input_stream.IsSeekable(),
-				    duration2);
 		return true;
 	}
 
@@ -168,13 +158,10 @@ flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd,
 }
 
 static void
-flac_decoder_loop(struct flac_data *data, FLAC__StreamDecoder *flac_dec,
-		  FLAC__uint64 t_start, FLAC__uint64 t_end)
+flac_decoder_loop(struct flac_data *data, FLAC__StreamDecoder *flac_dec)
 {
 	Decoder &decoder = data->decoder;
 
-	data->first_frame = t_start;
-
 	while (true) {
 		DecoderCommand cmd;
 		if (!data->tag.IsEmpty()) {
@@ -185,24 +172,49 @@ flac_decoder_loop(struct flac_data *data, FLAC__StreamDecoder *flac_dec,
 			cmd = decoder_get_command(decoder);
 
 		if (cmd == DecoderCommand::SEEK) {
-			FLAC__uint64 seek_sample = t_start +
+			FLAC__uint64 seek_sample =
 				decoder_seek_where_frame(decoder);
-			if (seek_sample >= t_start &&
-			    (t_end == 0 || seek_sample <= t_end) &&
-			    FLAC__stream_decoder_seek_absolute(flac_dec, seek_sample)) {
-				data->next_frame = seek_sample;
+			if (FLAC__stream_decoder_seek_absolute(flac_dec, seek_sample)) {
 				data->position = 0;
 				decoder_command_finished(decoder);
 			} else
 				decoder_seek_error(decoder);
-		} else if (cmd == DecoderCommand::STOP ||
-			   FLAC__stream_decoder_get_state(flac_dec) == FLAC__STREAM_DECODER_END_OF_STREAM)
+		} else if (cmd == DecoderCommand::STOP)
 			break;
 
-		if (t_end != 0 && data->next_frame >= t_end)
-			/* end of this sub track */
+		switch (FLAC__stream_decoder_get_state(flac_dec)) {
+		case FLAC__STREAM_DECODER_SEARCH_FOR_METADATA:
+		case FLAC__STREAM_DECODER_READ_METADATA:
+		case FLAC__STREAM_DECODER_SEARCH_FOR_FRAME_SYNC:
+		case FLAC__STREAM_DECODER_READ_FRAME:
+			/* continue decoding */
+			break;
+
+		case FLAC__STREAM_DECODER_END_OF_STREAM:
+			/* regular end of stream */
+			return;
+
+		case FLAC__STREAM_DECODER_SEEK_ERROR:
+			/* try to recover from seek error */
+			if (!FLAC__stream_decoder_flush(flac_dec)) {
+				LogError(flac_domain, "FLAC__stream_decoder_flush() failed");
+				return;
+			}
+
 			break;
 
+		case FLAC__STREAM_DECODER_OGG_ERROR:
+		case FLAC__STREAM_DECODER_ABORTED:
+		case FLAC__STREAM_DECODER_MEMORY_ALLOCATION_ERROR:
+			/* an error, fatal enough for us to abort the
+			   decoder */
+			return;
+
+		case FLAC__STREAM_DECODER_UNINITIALIZED:
+			/* we shouldn't see this, ever - bail out */
+			return;
+		}
+
 		if (!FLAC__stream_decoder_process_single(flac_dec) &&
 		    decoder_get_command(decoder) == DecoderCommand::NONE) {
 			/* a failure that was not triggered by a
@@ -251,6 +263,24 @@ stream_init(FLAC__StreamDecoder *flac_dec, struct flac_data *data, bool is_ogg)
 		: stream_init_flac(flac_dec, data);
 }
 
+static bool
+FlacInitAndDecode(struct flac_data &data, FLAC__StreamDecoder *sd, bool is_ogg)
+{
+	auto init_status = stream_init(sd, &data, is_ogg);
+	if (init_status != FLAC__STREAM_DECODER_INIT_STATUS_OK) {
+		LogWarning(flac_domain,
+			   FLAC__StreamDecoderInitStatusString[init_status]);
+		return false;
+	}
+
+	bool result = flac_decoder_initialize(&data, sd);
+	if (result)
+		flac_decoder_loop(&data, sd);
+
+	FLAC__stream_decoder_finish(sd);
+	return result;
+}
+
 static void
 flac_decode_internal(Decoder &decoder,
 		     InputStream &input_stream,
@@ -264,24 +294,8 @@ flac_decode_internal(Decoder &decoder,
 
 	struct flac_data data(decoder, input_stream);
 
-	FLAC__StreamDecoderInitStatus status =
-		stream_init(flac_dec, &data, is_ogg);
-	if (status != FLAC__STREAM_DECODER_INIT_STATUS_OK) {
-		FLAC__stream_decoder_delete(flac_dec);
-		LogWarning(flac_domain,
-			   FLAC__StreamDecoderInitStatusString[status]);
-		return;
-	}
-
-	if (!flac_decoder_initialize(&data, flac_dec, 0)) {
-		FLAC__stream_decoder_finish(flac_dec);
-		FLAC__stream_decoder_delete(flac_dec);
-		return;
-	}
-
-	flac_decoder_loop(&data, flac_dec, 0, 0);
+	FlacInitAndDecode(data, flac_dec, is_ogg);
 
-	FLAC__stream_decoder_finish(flac_dec);
 	FLAC__stream_decoder_delete(flac_dec);
 }
 

src/event/BufferedSocket.cxx

@@ -118,9 +118,15 @@ BufferedSocket::OnSocketReady(unsigned flags)
 	if (flags & READ) {
 		assert(!input.IsFull());
 
-		if (!ReadToBuffer() || !ResumeInput())
+		if (!ReadToBuffer())
 			return false;
 
+		if (!ResumeInput())
+			/* we must return "true" here or
+			   SocketMonitor::Dispatch() will call
+			   Cancel() on a freed object */
+			return true;
+
 		if (!input.IsFull())
 			ScheduleRead();
 	}

src/filter/plugins/ReplayGainFilterPlugin.cxx

@@ -152,8 +152,6 @@ ReplayGainFilter::Update()
 		volume = pcm_float_to_volume(scale);
 	}
 
-	pv.SetVolume(volume);
-
 	if (mixer != nullptr) {
 		/* update the hardware mixer volume */
 
@@ -164,7 +162,8 @@ ReplayGainFilter::Update()
 		Error error;
 		if (!mixer_set_volume(mixer, _volume, error))
 			LogError(error, "Failed to update hardware mixer");
-	}
+	} else
+		pv.SetVolume(volume);
 }
 
 static PreparedFilter *
@@ -189,7 +188,9 @@ PreparedReplayGainFilter::Open(AudioFormat &af, gcc_unused Error &error)
 ConstBuffer<void>
 ReplayGainFilter::FilterPCM(ConstBuffer<void> src, gcc_unused Error &error)
 {
-	return pv.Apply(src);
+	return mixer != nullptr
+		? src
+		: pv.Apply(src);
 }
 
 const struct filter_plugin replay_gain_filter_plugin = {

src/filter/plugins/RouteFilterPlugin.cxx

@@ -47,9 +47,11 @@
 #include "filter/FilterInternal.hxx"
 #include "filter/FilterRegistry.hxx"
 #include "pcm/PcmBuffer.hxx"
+#include "pcm/Silence.hxx"
 #include "util/StringUtil.hxx"
 #include "util/Error.hxx"
 #include "util/ConstBuffer.hxx"
+#include "util/WritableBuffer.hxx"
 
 #include <algorithm>
 #include <array>
@@ -272,9 +274,8 @@ RouteFilter::FilterPCM(ConstBuffer<void> src, gcc_unused Error &error)
 			    (unsigned)sources[c] >= input_format.channels) {
 				// No source for this destination output,
 				// give it zeroes as input
-				memset(chan_destination,
-					0x00,
-					bytes_per_frame_per_channel);
+				PcmSilence({chan_destination, bytes_per_frame_per_channel},
+					   input_format.format);
 			} else {
 				// Get the data from channel sources[c]
 				// and copy it to the output

src/output/plugins/ShoutOutputPlugin.cxx

@@ -266,7 +266,6 @@ ShoutOutput::Configure(const ConfigBlock &block, Error &error)
 
 	{
 		char temp[11];
-		memset(temp, 0, sizeof(temp));
 
 		snprintf(temp, sizeof(temp), "%u", audio_format.channels);
 		shout_set_audio_info(shout_conn, SHOUT_AI_CHANNELS, temp);

src/pcm/Silence.cxx

+/*
+ * Copyright (C) 2003-2016 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "config.h"
+#include "Silence.hxx"
+#include "AudioFormat.hxx"
+#include "util/WritableBuffer.hxx"
+
+#include <string.h>
+
+void
+PcmSilence(WritableBuffer<void> dest, SampleFormat format)
+{
+	uint8_t pattern = 0;
+	if (format == SampleFormat::DSD)
+		pattern = 0x69;
+
+	memset(dest.data, pattern, dest.size);
+}

src/pcm/Silence.hxx

+/*
+ * Copyright (C) 2003-2016 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_PCM_SILENCE_HXX
+#define MPD_PCM_SILENCE_HXX
+
+#include "check.h"
+
+#include <stdint.h>
+
+template<typename T> struct WritableBuffer;
+enum class SampleFormat : uint8_t;
+
+/**
+ * Fill the given buffer with the format-specific silence pattern.
+ */
+void
+PcmSilence(WritableBuffer<void> dest, SampleFormat format);
+
+#endif

src/pcm/Volume.cxx

@@ -19,9 +19,11 @@
 
 #include "config.h"
 #include "Volume.hxx"
+#include "Silence.hxx"
 #include "Domain.hxx"
 #include "Traits.hxx"
 #include "util/ConstBuffer.hxx"
+#include "util/WritableBuffer.hxx"
 #include "util/Error.hxx"
 
 #include "PcmDither.cxx" // including the .cxx file to get inlined templates
@@ -133,9 +135,7 @@ PcmVolume::Apply(ConstBuffer<void> src)
 
 	if (volume == 0) {
 		/* optimized special case: 0% volume = memset(0) */
-		/* TODO: is this valid for all sample formats? What
-		   about floating point? */
-		memset(data, 0, src.size);
+		PcmSilence({data, src.size}, format);
 		return { data, src.size };
 	}
 

src/player/Thread.cxx

@@ -25,6 +25,7 @@
 #include "MusicPipe.hxx"
 #include "MusicBuffer.hxx"
 #include "MusicChunk.hxx"
+#include "pcm/Silence.hxx"
 #include "DetachedSong.hxx"
 #include "CrossFade.hxx"
 #include "Control.hxx"
@@ -532,8 +533,12 @@ Player::SendSilence()
 
 	MusicChunk *chunk = buffer.Allocate();
 	if (chunk == nullptr) {
-		LogError(player_domain, "Failed to allocate silence buffer");
-		return false;
+		/* this is non-fatal, because this means that the
+		   decoder has filled to buffer completely meanwhile;
+		   by ignoring the error, we work around this race
+		   condition */
+		LogDebug(player_domain, "Failed to allocate silence buffer");
+		return true;
 	}
 
 #ifndef NDEBUG
@@ -547,7 +552,7 @@ Player::SendSilence()
 
 	chunk->time = SignedSongTime::Negative(); /* undefined time stamp */
 	chunk->length = num_frames * frame_size;
-	memset(chunk->data, 0, chunk->length);
+	PcmSilence({chunk->data, chunk->length}, play_audio_format.format);
 
 	Error error;
 	if (!pc.outputs.Play(chunk, error)) {