From 8dc3f3b21a257cfbdfc00594543e4fed284f8466 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 1 Jul 2016 21:16:14 +0200 Subject: [PATCH 01/24] configure.ac: prepare for 0.19.17 --- NEWS | 2 ++ configure.ac | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 73d22444e..2649ac44c 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,5 @@ +ver 0.19.17 (not yet released) + ver 0.19.16 (2016/06/13) * faster seeking * fix system include path order diff --git a/configure.ac b/configure.ac index 35ecb7b19..b5d59c216 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ(2.60) -AC_INIT(mpd, 0.19.16, musicpd-dev-team@lists.sourceforge.net) +AC_INIT(mpd, 0.19.17, musicpd-dev-team@lists.sourceforge.net) VERSION_MAJOR=0 VERSION_MINOR=19 -VERSION_REVISION=16 +VERSION_REVISION=17 VERSION_EXTRA=0 AC_CONFIG_SRCDIR([src/Main.cxx]) From 072e39c9cf449ccb8d7be301768a769f479c2875 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 1 Jul 2016 21:17:52 +0200 Subject: [PATCH 02/24] filter/ReplayGain: skip PcmVolume if a mixer is set Previously, volume was applied twice: once by PcmVolume, and again by the hardware mixer. --- NEWS | 1 + src/filter/plugins/ReplayGainFilterPlugin.cxx | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 2649ac44c..43190a367 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ ver 0.19.17 (not yet released) +* replay gain: fix "replay_gain_handler mixer" setting ver 0.19.16 (2016/06/13) * faster seeking diff --git a/src/filter/plugins/ReplayGainFilterPlugin.cxx b/src/filter/plugins/ReplayGainFilterPlugin.cxx index f76e48e37..b6f574bd0 100644 --- a/src/filter/plugins/ReplayGainFilterPlugin.cxx +++ b/src/filter/plugins/ReplayGainFilterPlugin.cxx @@ -134,8 +134,6 @@ ReplayGainFilter::Update() volume = pcm_float_to_volume(scale); } - pv.SetVolume(volume); - if (mixer != nullptr) { /* update the hardware mixer volume */ @@ -146,7 +144,8 @@ ReplayGainFilter::Update() Error error; if (!mixer_set_volume(mixer, _volume, error)) LogError(error, "Failed to update hardware mixer"); - } + } else + pv.SetVolume(volume); } static Filter * @@ -174,7 +173,9 @@ ReplayGainFilter::Close() ConstBuffer ReplayGainFilter::FilterPCM(ConstBuffer src, gcc_unused Error &error) { - return pv.Apply(src); + return mixer != nullptr + ? src + : pv.Apply(src); } const struct filter_plugin replay_gain_filter_plugin = { From ba8e579e9b8126a694d1125e001acddbeb494299 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 1 Jul 2016 21:22:21 +0200 Subject: [PATCH 03/24] pcm/Volume: use 0x69 to generate DSD silence --- src/pcm/Volume.cxx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pcm/Volume.cxx b/src/pcm/Volume.cxx index b12d8fd41..37f90c582 100644 --- a/src/pcm/Volume.cxx +++ b/src/pcm/Volume.cxx @@ -134,9 +134,11 @@ PcmVolume::Apply(ConstBuffer 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); + uint8_t pattern = 0; + if (format == SampleFormat::DSD) + pattern = 0x69; + + memset(data, pattern, src.size); return { data, src.size }; } From 5eb0cbc887c2b68d8ccc1057b0da44aa69ccde9b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 17:44:45 +0200 Subject: [PATCH 04/24] PlayerThread: make chunk allocation error non-fatal in SendSilence() Fixes abort after seeking on fast machines. --- NEWS | 1 + src/PlayerThread.cxx | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 43190a367..4ef12c30e 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,5 @@ ver 0.19.17 (not yet released) +* fix spurious seek error "Failed to allocate silence buffer" * replay gain: fix "replay_gain_handler mixer" setting ver 0.19.16 (2016/06/13) diff --git a/src/PlayerThread.cxx b/src/PlayerThread.cxx index 30e509b3b..a2bb528b0 100644 --- a/src/PlayerThread.cxx +++ b/src/PlayerThread.cxx @@ -486,8 +486,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 From b8097eaf2e03696e78a59c6c47f5bd4ca539cbbd Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 17:52:24 +0200 Subject: [PATCH 05/24] pcm/Volume: move silence pattern to Silence.cxx --- Makefile.am | 1 + src/pcm/Silence.cxx | 35 +++++++++++++++++++++++++++++++++++ src/pcm/Silence.hxx | 36 ++++++++++++++++++++++++++++++++++++ src/pcm/Volume.cxx | 8 +++----- 4 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 src/pcm/Silence.cxx create mode 100644 src/pcm/Silence.hxx diff --git a/Makefile.am b/Makefile.am index 91f0f326c..7cd68f86e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -467,6 +467,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 \ diff --git a/src/pcm/Silence.cxx b/src/pcm/Silence.cxx new file mode 100644 index 000000000..c8f67f2b0 --- /dev/null +++ b/src/pcm/Silence.cxx @@ -0,0 +1,35 @@ +/* + * 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 + +void +PcmSilence(WritableBuffer dest, SampleFormat format) +{ + uint8_t pattern = 0; + if (format == SampleFormat::DSD) + pattern = 0x69; + + memset(dest.data, pattern, dest.size); +} diff --git a/src/pcm/Silence.hxx b/src/pcm/Silence.hxx new file mode 100644 index 000000000..5fcf99316 --- /dev/null +++ b/src/pcm/Silence.hxx @@ -0,0 +1,36 @@ +/* + * 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 + +template struct WritableBuffer; +enum class SampleFormat : uint8_t; + +/** + * Fill the given buffer with the format-specific silence pattern. + */ +void +PcmSilence(WritableBuffer dest, SampleFormat format); + +#endif diff --git a/src/pcm/Volume.cxx b/src/pcm/Volume.cxx index 37f90c582..6d28a9b36 100644 --- a/src/pcm/Volume.cxx +++ b/src/pcm/Volume.cxx @@ -19,10 +19,12 @@ #include "config.h" #include "Volume.hxx" +#include "Silence.hxx" #include "Domain.hxx" #include "PcmUtils.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 @@ -134,11 +136,7 @@ PcmVolume::Apply(ConstBuffer src) if (volume == 0) { /* optimized special case: 0% volume = memset(0) */ - uint8_t pattern = 0; - if (format == SampleFormat::DSD) - pattern = 0x69; - - memset(data, pattern, src.size); + PcmSilence({data, src.size}, format); return { data, src.size }; } From 104075f3e009a75d3d6d5fc880dd4ba44a143216 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 17:55:59 +0200 Subject: [PATCH 06/24] PlayerThread: use PcmSilence() in SendSilence() No change for regular PCM, but DSD uses 0x69 now. --- NEWS | 1 + src/PlayerThread.cxx | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 4ef12c30e..3260b0142 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.19.17 (not yet released) * fix spurious seek error "Failed to allocate silence buffer" * replay gain: fix "replay_gain_handler mixer" setting +* DSD: use 0x69 as silence pattern ver 0.19.16 (2016/06/13) * faster seeking diff --git a/src/PlayerThread.cxx b/src/PlayerThread.cxx index a2bb528b0..69bd0881e 100644 --- a/src/PlayerThread.cxx +++ b/src/PlayerThread.cxx @@ -25,6 +25,7 @@ #include "MusicPipe.hxx" #include "MusicBuffer.hxx" #include "MusicChunk.hxx" +#include "pcm/Silence.hxx" #include "DetachedSong.hxx" #include "system/FatalError.hxx" #include "CrossFade.hxx" @@ -505,7 +506,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)) { From 58487e484ffdfb93c736ba15cabe266df21e32d0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 17:58:42 +0200 Subject: [PATCH 07/24] filter/route: use PcmSilence() --- src/filter/plugins/RouteFilterPlugin.cxx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/filter/plugins/RouteFilterPlugin.cxx b/src/filter/plugins/RouteFilterPlugin.cxx index 4094119f2..e8bed6efb 100644 --- a/src/filter/plugins/RouteFilterPlugin.cxx +++ b/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 @@ -266,9 +268,8 @@ RouteFilter::FilterPCM(ConstBuffer 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 From 1c7de0b4ac7b3d7e6ebdde2fbbc8a908e32ac0a6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 18:02:35 +0200 Subject: [PATCH 08/24] output/shout: remove pointless memset() call --- src/output/plugins/ShoutOutputPlugin.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/output/plugins/ShoutOutputPlugin.cxx b/src/output/plugins/ShoutOutputPlugin.cxx index b51f7ed82..406be8daf 100644 --- a/src/output/plugins/ShoutOutputPlugin.cxx +++ b/src/output/plugins/ShoutOutputPlugin.cxx @@ -247,7 +247,6 @@ ShoutOutput::Configure(const config_param ¶m, 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); From faf2eeaa99f8fda3afffc13720cee3d65e845264 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 19:27:40 +0200 Subject: [PATCH 09/24] decoder/flac: evaluate all possible FLAC__stream_decoder_get_state() values Stop after all fatal errors. This fixes assertion failures in libFLAC. --- NEWS | 2 ++ src/decoder/plugins/FlacDecoderPlugin.cxx | 28 +++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 3260b0142..8ae416442 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.19.17 (not yet released) +* decoder + - flac: fix assertion failure while seeking * fix spurious seek error "Failed to allocate silence buffer" * replay gain: fix "replay_gain_handler mixer" setting * DSD: use 0x69 as silence pattern diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index eea813401..003e98975 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -194,10 +194,34 @@ flac_decoder_loop(struct flac_data *data, FLAC__StreamDecoder *flac_dec, 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; + 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_OGG_ERROR: + case FLAC__STREAM_DECODER_SEEK_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 (t_end != 0 && data->next_frame >= t_end) /* end of this sub track */ break; From f9130f42a2c49d4436cc0f2c6803fd2b31861486 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 5 Jul 2016 19:29:22 +0200 Subject: [PATCH 10/24] decoder/flac: try to recover from seek error() libFLAC API documentation suggests that FLAC__stream_decoder_flush() should be called to recover from FLAC__STREAM_DECODER_SEEK_ERROR. --- src/decoder/plugins/FlacDecoderPlugin.cxx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index 003e98975..c40bd2243 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -209,8 +209,16 @@ flac_decoder_loop(struct flac_data *data, FLAC__StreamDecoder *flac_dec, /* regular end of stream */ return; - case FLAC__STREAM_DECODER_OGG_ERROR: 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 From 6f59d71e0767a3fb84ff9b63555082938c7f4402 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 6 Jul 2016 15:37:11 +0200 Subject: [PATCH 11/24] decoder/API: check initial_seek_running in _check_cancel_read() The "seeking" flag is not set for the initial seek, and so decoder_read() could be canceled when another SEEK was emitted during initial seek. This fixes several seek problems, for example the one reported for the FLAC decoder plugin: https://bugs.musicpd.org/view.php?id=4552 --- NEWS | 1 + src/decoder/DecoderAPI.cxx | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 8ae416442..dc81a79c4 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.19.17 (not yet released) * decoder - flac: fix assertion failure while seeking + - 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 diff --git a/src/decoder/DecoderAPI.cxx b/src/decoder/DecoderAPI.cxx index 941d3a70d..d7c9bc7ee 100644 --- a/src/decoder/DecoderAPI.cxx +++ b/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; From b46cf57d983e559ebd29c4f0749914f3714d8b75 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 7 Jul 2016 13:52:20 +0200 Subject: [PATCH 12/24] event/BufferedSocket: OnSocketReady() returns true after close Fixes use-after-free bug (https://bugs.musicpd.org/view.php?id=4548). --- NEWS | 1 + src/event/BufferedSocket.cxx | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index dc81a79c4..58edf3f64 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,7 @@ ver 0.19.17 (not yet released) * 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" ver 0.19.16 (2016/06/13) * faster seeking diff --git a/src/event/BufferedSocket.cxx b/src/event/BufferedSocket.cxx index 939824baa..1891f18bb 100644 --- a/src/event/BufferedSocket.cxx +++ b/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(); } From e6389ff5a169a9653f3a2dfa6d0f789f21a4a453 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 7 Jul 2016 13:52:33 +0200 Subject: [PATCH 13/24] client/ClientRead: call Break() before Close() Referencing the attribute "partition" is illegal after Close(), because Close() deletes "this". --- NEWS | 2 +- src/client/ClientRead.cxx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 58edf3f64..bd547f42f 100644 --- a/NEWS +++ b/NEWS @@ -5,7 +5,7 @@ ver 0.19.17 (not yet released) * 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" +* fix use-after-free bug on "close" and "kill" ver 0.19.16 (2016/06/13) * faster seeking diff --git a/src/client/ClientRead.cxx b/src/client/ClientRead.cxx index 9cfb1271f..232da8d62 100644 --- a/src/client/ClientRead.cxx +++ b/src/client/ClientRead.cxx @@ -52,8 +52,8 @@ Client::OnSocketInput(void *data, size_t length) break; case CommandResult::KILL: - Close(); partition.instance.event_loop->Break(); + Close(); return InputResult::CLOSED; case CommandResult::FINISH: From 70367d70c8a6a236ee1deff879ebe148e4d51edf Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 21:59:30 +0200 Subject: [PATCH 14/24] decoder/flac: remove obsolete sub-song support This is obsolete because it has been moved to the MPD core. --- src/decoder/plugins/FlacCommon.cxx | 3 +-- src/decoder/plugins/FlacCommon.hxx | 11 ----------- src/decoder/plugins/FlacDecoderPlugin.cxx | 18 ++++-------------- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/decoder/plugins/FlacCommon.cxx b/src/decoder/plugins/FlacCommon.cxx index e86f85569..18678a90a 100644 --- a/src/decoder/plugins/FlacCommon.cxx +++ b/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), + total_frames(0), position(0), decoder(_decoder), input_stream(_input_stream) { } @@ -176,7 +176,6 @@ flac_common_write(struct flac_data *data, const FLAC__Frame * frame, 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: diff --git a/src/decoder/plugins/FlacCommon.hxx b/src/decoder/plugins/FlacCommon.hxx index 34ce0a3fc..49af9f560 100644 --- a/src/decoder/plugins/FlacCommon.hxx +++ b/src/decoder/plugins/FlacCommon.hxx @@ -61,17 +61,6 @@ struct flac_data : public FlacInput { */ 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; diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index c40bd2243..bb9ebbea8 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -167,13 +167,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()) { @@ -184,12 +181,9 @@ 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 @@ -230,10 +224,6 @@ flac_decoder_loop(struct flac_data *data, FLAC__StreamDecoder *flac_dec, return; } - if (t_end != 0 && data->next_frame >= t_end) - /* end of this sub track */ - break; - if (!FLAC__stream_decoder_process_single(flac_dec) && decoder_get_command(decoder) == DecoderCommand::NONE) { /* a failure that was not triggered by a @@ -310,7 +300,7 @@ flac_decode_internal(Decoder &decoder, return; } - flac_decoder_loop(&data, flac_dec, 0, 0); + flac_decoder_loop(&data, flac_dec); FLAC__stream_decoder_finish(flac_dec); FLAC__stream_decoder_delete(flac_dec); From 2ca8d69126f1e40f1b5079ff11be5b7dccb5cf4c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:20:16 +0200 Subject: [PATCH 15/24] decoder/flac: document flac_data::position --- src/decoder/plugins/FlacCommon.hxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/decoder/plugins/FlacCommon.hxx b/src/decoder/plugins/FlacCommon.hxx index 49af9f560..f38c9aacc 100644 --- a/src/decoder/plugins/FlacCommon.hxx +++ b/src/decoder/plugins/FlacCommon.hxx @@ -61,6 +61,10 @@ struct flac_data : public FlacInput { */ FLAC__uint64 total_frames; + /** + * End of last frame's position within the stream. This is + * used for bit rate calculations. + */ FLAC__uint64 position; Decoder &decoder; From 7f36923eb4e9d7841660ce38d381b7cfc2e09208 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:32:23 +0200 Subject: [PATCH 16/24] decoder/flac: pass SignedSongTime to decoder_initialized() --- src/decoder/plugins/FlacDecoderPlugin.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index bb9ebbea8..d89d116a7 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -146,8 +146,8 @@ flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd, /* done */ const auto duration2 = - SongTime::FromScale(data->total_frames, - data->audio_format.sample_rate); + SignedSongTime::FromScale(data->total_frames, + data->audio_format.sample_rate); decoder_initialized(data->decoder, data->audio_format, data->input_stream.IsSeekable(), From 4a7042e8471f326ffe741586091ad41488fd1552 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:33:49 +0200 Subject: [PATCH 17/24] decoder/flac: handle unknown duration correctly If the duration is unknown, pass SignedSongTime::Negative(), as documented for decoder_initialized(). --- NEWS | 1 + src/decoder/plugins/FlacCommon.hxx | 4 +--- src/decoder/plugins/FlacDecoderPlugin.cxx | 7 ++++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index bd547f42f..dd6a1c3cb 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.19.17 (not yet released) * 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 diff --git a/src/decoder/plugins/FlacCommon.hxx b/src/decoder/plugins/FlacCommon.hxx index f38c9aacc..b5de6974d 100644 --- a/src/decoder/plugins/FlacCommon.hxx +++ b/src/decoder/plugins/FlacCommon.hxx @@ -55,9 +55,7 @@ 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). + * The total number of frames in this song. 0 means unknown. */ FLAC__uint64 total_frames; diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index d89d116a7..ff4862e78 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -145,9 +145,10 @@ flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd, if (data->initialized) { /* done */ - const auto duration2 = - SignedSongTime::FromScale(data->total_frames, - data->audio_format.sample_rate); + const auto duration2 = data->total_frames > 0 + ? SignedSongTime::FromScale(data->total_frames, + data->audio_format.sample_rate) + : SignedSongTime::Negative(); decoder_initialized(data->decoder, data->audio_format, data->input_stream.IsSeekable(), From e42eed4d4ccf600a18e84a59faa0c4e08459166f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:40:31 +0200 Subject: [PATCH 18/24] decoder/flac: remove pointless check --- src/decoder/plugins/FlacCommon.cxx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/decoder/plugins/FlacCommon.cxx b/src/decoder/plugins/FlacCommon.cxx index 18678a90a..882cfbdb5 100644 --- a/src/decoder/plugins/FlacCommon.cxx +++ b/src/decoder/plugins/FlacCommon.cxx @@ -155,7 +155,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,11 +166,8 @@ 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 / - (1000 * frame->header.blocksize); - else - bit_rate = 0; + unsigned bit_rate = nbytes * 8 * frame->header.sample_rate / + (1000 * frame->header.blocksize); auto cmd = decoder_data(data->decoder, data->input_stream, buffer, buffer_size, From 79d4f8674c13668e59448579e533bad957bda1d9 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:38:40 +0200 Subject: [PATCH 19/24] decoder/flac: remove "duration" parameter from flac_decoder_initialize() It's always 0. --- src/decoder/plugins/FlacCommon.cxx | 4 +--- src/decoder/plugins/FlacDecoderPlugin.cxx | 7 ++----- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/decoder/plugins/FlacCommon.cxx b/src/decoder/plugins/FlacCommon.cxx index 882cfbdb5..a3b1e9184 100644 --- a/src/decoder/plugins/FlacCommon.cxx +++ b/src/decoder/plugins/FlacCommon.cxx @@ -77,9 +77,7 @@ flac_got_stream_info(struct flac_data *data, } data->frame_size = data->audio_format.GetFrameSize(); - - if (data->total_frames == 0) - data->total_frames = stream_info->total_samples; + data->total_frames = stream_info->total_samples; data->initialized = true; } diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index ff4862e78..25049dfb7 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -132,11 +132,8 @@ 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)) { LogWarning(flac_domain, "problem reading metadata"); return false; @@ -295,7 +292,7 @@ flac_decode_internal(Decoder &decoder, return; } - if (!flac_decoder_initialize(&data, flac_dec, 0)) { + if (!flac_decoder_initialize(&data, flac_dec)) { FLAC__stream_decoder_finish(flac_dec); FLAC__stream_decoder_delete(flac_dec); return; From 475ac76a5f76d2f286b2289be90165fe438c4373 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:43:01 +0200 Subject: [PATCH 20/24] decoder/flac: late "total_frames" initialization --- src/decoder/plugins/FlacCommon.cxx | 3 ++- src/decoder/plugins/FlacCommon.hxx | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/FlacCommon.cxx b/src/decoder/plugins/FlacCommon.cxx index a3b1e9184..8aa84201d 100644 --- a/src/decoder/plugins/FlacCommon.cxx +++ b/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), position(0), + position(0), decoder(_decoder), input_stream(_input_stream) { } @@ -142,6 +142,7 @@ flac_got_first_frame(struct flac_data *data, const FLAC__FrameHeader *header) data->input_stream.IsSeekable(), duration); + data->total_frames = 0; /* unkown duration */ data->initialized = true; return true; diff --git a/src/decoder/plugins/FlacCommon.hxx b/src/decoder/plugins/FlacCommon.hxx index b5de6974d..2d424cbe5 100644 --- a/src/decoder/plugins/FlacCommon.hxx +++ b/src/decoder/plugins/FlacCommon.hxx @@ -56,6 +56,8 @@ struct flac_data : public FlacInput { /** * The total number of frames in this song. 0 means unknown. + * + * This attribute is defined if "initialized" is true. */ FLAC__uint64 total_frames; From 68064f1aa60d30febc69557b982057bb404e4e5e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 22:44:23 +0200 Subject: [PATCH 21/24] decoder/flac: move duplicate code to flac_data::Initialize() --- src/decoder/plugins/FlacCommon.cxx | 78 ++++++++++++----------- src/decoder/plugins/FlacCommon.hxx | 13 ++-- src/decoder/plugins/FlacDecoderPlugin.cxx | 9 --- 3 files changed, 47 insertions(+), 53 deletions(-) diff --git a/src/decoder/plugins/FlacCommon.cxx b/src/decoder/plugins/FlacCommon.cxx index 8aa84201d..eaf4d8b2f 100644 --- a/src/decoder/plugins/FlacCommon.cxx +++ b/src/decoder/plugins/FlacCommon.cxx @@ -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(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,20 +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(); - 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, @@ -123,29 +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(data->total_frames, - data->audio_format.sample_rate); - - decoder_initialized(data->decoder, data->audio_format, - data->input_stream.IsSeekable(), - duration); - - data->total_frames = 0; /* unkown duration */ - data->initialized = true; - - return true; + return data->Initialize(header->sample_rate, + header->bits_per_sample, + header->channels, + /* unknown duration */ + 0); } FLAC__StreamDecoderWriteStatus diff --git a/src/decoder/plugins/FlacCommon.hxx b/src/decoder/plugins/FlacCommon.hxx index 2d424cbe5..593a5addc 100644 --- a/src/decoder/plugins/FlacCommon.hxx +++ b/src/decoder/plugins/FlacCommon.hxx @@ -54,13 +54,6 @@ struct flac_data : public FlacInput { */ AudioFormat audio_format; - /** - * The total number of frames in this song. 0 means unknown. - * - * This attribute is defined if "initialized" is true. - */ - FLAC__uint64 total_frames; - /** * End of last frame's position within the stream. This is * used for bit rate calculations. @@ -73,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, diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index 25049dfb7..35bb041f0 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -141,15 +141,6 @@ flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd) if (data->initialized) { /* done */ - - const auto duration2 = data->total_frames > 0 - ? SignedSongTime::FromScale(data->total_frames, - data->audio_format.sample_rate) - : SignedSongTime::Negative(); - - decoder_initialized(data->decoder, data->audio_format, - data->input_stream.IsSeekable(), - duration2); return true; } From ed3bc4ab632305a54c41377b10e70991b2aeb003 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 23:03:49 +0200 Subject: [PATCH 22/24] decoder/flac: move code to FlacInitAndDecode() --- src/decoder/plugins/FlacDecoderPlugin.cxx | 36 ++++++++++++----------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index 35bb041f0..0990899d1 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -261,6 +261,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, @@ -274,24 +292,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; - } + FlacInitAndDecode(data, flac_dec, is_ogg); - if (!flac_decoder_initialize(&data, flac_dec)) { - FLAC__stream_decoder_finish(flac_dec); - FLAC__stream_decoder_delete(flac_dec); - return; - } - - flac_decoder_loop(&data, flac_dec); - - FLAC__stream_decoder_finish(flac_dec); FLAC__stream_decoder_delete(flac_dec); } From ab95027fc6e9d873501a468694399f3b4818ce72 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 8 Jul 2016 23:12:49 +0200 Subject: [PATCH 23/24] decoder/flac: suppress warning at end of stream This is required if a stream ands without another chained FLAC file. --- src/decoder/plugins/FlacDecoderPlugin.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index 0990899d1..c78b02310 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -135,7 +135,8 @@ static bool flac_decoder_initialize(struct flac_data *data, FLAC__StreamDecoder *sd) { if (!FLAC__stream_decoder_process_until_end_of_metadata(sd)) { - LogWarning(flac_domain, "problem reading metadata"); + if (FLAC__stream_decoder_get_state(sd) != FLAC__STREAM_DECODER_END_OF_STREAM) + LogWarning(flac_domain, "problem reading metadata"); return false; } From f28c746b6bcfadb6df8abbe5d1ae9f418568ef2b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 9 Jul 2016 00:40:57 +0200 Subject: [PATCH 24/24] release v0.19.17 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index dd6a1c3cb..df7493f80 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.19.17 (not yet released) +ver 0.19.17 (2016/07/09) * decoder - flac: fix assertion failure while seeking - flac: fix stream duration indicator