From 1033dbca2bc695e42068483f23b3d09dd188ec10 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 29 Jul 2019 11:31:30 +0200 Subject: [PATCH 01/37] playlist/Song: add missing includes --- src/playlist/PlaylistSong.cxx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/playlist/PlaylistSong.cxx b/src/playlist/PlaylistSong.cxx index 543d8d96f..4672172cc 100644 --- a/src/playlist/PlaylistSong.cxx +++ b/src/playlist/PlaylistSong.cxx @@ -25,6 +25,9 @@ #include "util/UriUtil.hxx" #include "song/DetachedSong.hxx" +#include +#include + #include static void From 86d0534638b6854af663b77d7e082ea8c1af76bf Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 13:56:00 +0200 Subject: [PATCH 02/37] lib/xiph/OggVisitor: more API documentation --- src/lib/xiph/OggVisitor.hxx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/lib/xiph/OggVisitor.hxx b/src/lib/xiph/OggVisitor.hxx index a7e6eb9b8..e20548dd2 100644 --- a/src/lib/xiph/OggVisitor.hxx +++ b/src/lib/xiph/OggVisitor.hxx @@ -69,8 +69,21 @@ private: void HandlePackets(); protected: + /** + * Called when the "beginning of stream" packet has been seen. + * + * @param packet the "beginning of stream" packet + */ virtual void OnOggBeginning(const ogg_packet &packet) = 0; + + /** + * Called for each follow-up packet. + */ virtual void OnOggPacket(const ogg_packet &packet) = 0; + + /** + * Called after the "end of stream" packet has been processed. + */ virtual void OnOggEnd() = 0; }; From 6de088140b2fb3afe2905ad3ea2e2d0648eb603f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 12 Jul 2019 18:33:27 +0200 Subject: [PATCH 03/37] lib/xiph/OggVisitor: invoke OnOggPacket() with the "E_O_S" packet The "end of stream" packet is not special; it contains normal data, and thus we should pass it to OnOggPacket(). This fixes one part of https://github.com/MusicPlayerDaemon/MPD/issues/601 --- NEWS | 1 + src/lib/xiph/OggVisitor.cxx | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 69121ac7f..d67bbb26a 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.21.12 (not yet released) * decoder - opus: ignore case in replay gain tag names + - opus, vorbis: decode the "end of stream" packet * Windows - support backslash in relative URIs loaded from playlists diff --git a/src/lib/xiph/OggVisitor.cxx b/src/lib/xiph/OggVisitor.cxx index b975d8d17..04a02469f 100644 --- a/src/lib/xiph/OggVisitor.cxx +++ b/src/lib/xiph/OggVisitor.cxx @@ -69,12 +69,12 @@ OggVisitor::HandlePacket(const ogg_packet &packet) /* fail if BOS is missing */ throw std::runtime_error("BOS packet expected"); + OnOggPacket(packet); + if (packet.e_o_s) { EndStream(); return; } - - OnOggPacket(packet); } inline void From b81138bda1c6cb1b3d1fd8ecbf4174ac939b5f44 Mon Sep 17 00:00:00 2001 From: Diomendius <42310725+Diomendius@users.noreply.github.com> Date: Fri, 2 Aug 2019 17:47:45 +1200 Subject: [PATCH 04/37] Fix JACK plugin outputting only to left channel The JACK output plugin would not correctly upmix mono input files when exactly 2 output ports were configured. This fixes that. --- NEWS | 2 ++ src/output/plugins/JackOutputPlugin.cxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d67bbb26a..d6bed1990 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ ver 0.21.12 (not yet released) * decoder - opus: ignore case in replay gain tag names - opus, vorbis: decode the "end of stream" packet +* output + - jack: fix mono-to-stereo conversion * Windows - support backslash in relative URIs loaded from playlists diff --git a/src/output/plugins/JackOutputPlugin.cxx b/src/output/plugins/JackOutputPlugin.cxx index f9535386e..6b82b8bf7 100644 --- a/src/output/plugins/JackOutputPlugin.cxx +++ b/src/output/plugins/JackOutputPlugin.cxx @@ -540,7 +540,7 @@ JackOutput::Start() std::fill(dports + num_dports, dports + audio_format.channels, dports[0]); } else if (num_dports > audio_format.channels) { - if (audio_format.channels == 1 && num_dports > 2) { + if (audio_format.channels == 1 && num_dports >= 2) { /* mono input file: connect the one source channel to the both destination channels */ duplicate_port = dports[1]; From e00464435bb80697f0fba06417a40275c9944dce Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 15:52:35 +0200 Subject: [PATCH 05/37] util/Compiler.h: move compiler version checks to meson.build --- meson.build | 6 ++++++ src/util/Compiler.h | 12 ------------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/meson.build b/meson.build index dd4bef90d..6d35c5990 100644 --- a/meson.build +++ b/meson.build @@ -15,6 +15,12 @@ version_cxx = vcs_tag(input: 'src/GitVersion.cxx', output: 'GitVersion.cxx') compiler = meson.get_compiler('cpp') c_compiler = meson.get_compiler('c') +if compiler.get_id() == 'gcc' and compiler.version().version_compare('<6') + warning('Your GCC version is too old. You need at least version 6.') +elif compiler.get_id() == 'clang' and compiler.version().version_compare('<3') + warning('Your clang version is too old. You need at least version 3.') +endif + conf = configuration_data() conf.set_quoted('PACKAGE', meson.project_name()) conf.set_quoted('PACKAGE_NAME', meson.project_name()) diff --git a/src/util/Compiler.h b/src/util/Compiler.h index 60f7fe678..6a64ad098 100644 --- a/src/util/Compiler.h +++ b/src/util/Compiler.h @@ -57,18 +57,6 @@ (GCC_VERSION > 0 && CLANG_VERSION == 0 && \ GCC_VERSION < GCC_MAKE_VERSION(major, minor, 0)) -#ifdef __clang__ -# if __clang_major__ < 3 -# error Sorry, your clang version is too old. You need at least version 3.1. -# endif -#elif defined(__GNUC__) -# if GCC_OLDER_THAN(6,0) -# error Sorry, your gcc version is too old. You need at least version 6.0. -# endif -#else -# warning Untested compiler. Use at your own risk! -#endif - /** * Are we building with the specified version of clang or newer? */ From 31da8eac9b9324ab09645fa73dc84dc7cc6013d7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:48:27 +0200 Subject: [PATCH 06/37] util/StaticFifoBuffer: add `noexcept` --- src/util/StaticFifoBuffer.hxx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/util/StaticFifoBuffer.hxx b/src/util/StaticFifoBuffer.hxx index 076a573bb..bf74a22c8 100644 --- a/src/util/StaticFifoBuffer.hxx +++ b/src/util/StaticFifoBuffer.hxx @@ -56,11 +56,11 @@ protected: T data[size]; public: - constexpr size_type GetCapacity() const { + constexpr size_type GetCapacity() const noexcept { return size; } - void Shift() { + void Shift() noexcept { if (head == 0) return; @@ -74,15 +74,15 @@ public: head = 0; } - void Clear() { + void Clear() noexcept { head = tail = 0; } - bool empty() const { + bool empty() const noexcept { return head == tail; } - bool IsFull() const { + bool IsFull() const noexcept { return head == 0 && tail == size; } @@ -90,7 +90,7 @@ public: * Prepares writing. Returns a buffer range which may be written. * When you are finished, call Append(). */ - Range Write() { + Range Write() noexcept { if (empty()) Clear(); else if (tail == size) @@ -103,7 +103,7 @@ public: * Expands the tail of the buffer, after data has been written to * the buffer returned by Write(). */ - void Append(size_type n) { + void Append(size_type n) noexcept { assert(tail <= size); assert(n <= size); assert(tail + n <= size); @@ -115,14 +115,14 @@ public: * Return a buffer range which may be read. The buffer pointer is * writable, to allow modifications while parsing. */ - Range Read() { + Range Read() noexcept { return Range(data + head, tail - head); } /** * Marks a chunk as consumed. */ - void Consume(size_type n) { + void Consume(size_type n) noexcept { assert(tail <= size); assert(head <= tail); assert(n <= tail); From adc25e648fe5237694d237fd781d4705bf01bbd4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:49:08 +0200 Subject: [PATCH 07/37] util/StaticFifoBuffer: add `constexpr` --- src/util/StaticFifoBuffer.hxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/StaticFifoBuffer.hxx b/src/util/StaticFifoBuffer.hxx index bf74a22c8..3598d7246 100644 --- a/src/util/StaticFifoBuffer.hxx +++ b/src/util/StaticFifoBuffer.hxx @@ -78,11 +78,11 @@ public: head = tail = 0; } - bool empty() const noexcept { + constexpr bool empty() const noexcept { return head == tail; } - bool IsFull() const noexcept { + constexpr bool IsFull() const noexcept { return head == 0 && tail == size; } @@ -115,7 +115,7 @@ public: * Return a buffer range which may be read. The buffer pointer is * writable, to allow modifications while parsing. */ - Range Read() noexcept { + constexpr Range Read() noexcept { return Range(data + head, tail - head); } From 52bee8f81fe749c37030ee3095471d8c2f0d8ab4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:49:50 +0200 Subject: [PATCH 08/37] util/StaticFifoBuffer: add GetAvailable() --- src/util/StaticFifoBuffer.hxx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/StaticFifoBuffer.hxx b/src/util/StaticFifoBuffer.hxx index 3598d7246..615f92e3f 100644 --- a/src/util/StaticFifoBuffer.hxx +++ b/src/util/StaticFifoBuffer.hxx @@ -111,6 +111,10 @@ public: tail += n; } + constexpr size_type GetAvailable() const noexcept { + return tail - head; + } + /** * Return a buffer range which may be read. The buffer pointer is * writable, to allow modifications while parsing. From 089615a01e9f5f2384d797974d79cde2bbf5ae8a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:36:32 +0200 Subject: [PATCH 09/37] decoder/mad: include cleanup --- src/decoder/plugins/MadDecoderPlugin.cxx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 5f97c4f91..f8230ce69 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -21,10 +21,8 @@ #include "MadDecoderPlugin.hxx" #include "../DecoderAPI.hxx" #include "input/InputStream.hxx" -#include "config/Block.hxx" #include "tag/Id3Scan.hxx" #include "tag/Id3ReplayGain.hxx" -#include "tag/Rva2.hxx" #include "tag/Handler.hxx" #include "tag/ReplayGain.hxx" #include "tag/MixRamp.hxx" @@ -40,8 +38,6 @@ #include #endif -#include - #include #include #include From 61a3c69a06f0ceb970326c85ab687f5e33ab7219 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 16 Jun 2019 10:02:51 +0200 Subject: [PATCH 10/37] decoder/mad: make enums strictly-typed --- src/decoder/plugins/MadDecoderPlugin.cxx | 96 ++++++++++++------------ 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index f8230ce69..05fef9b3c 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -45,17 +45,17 @@ static constexpr unsigned long FRAMES_CUSHION = 2000; -enum mp3_action { - DECODE_SKIP = -3, - DECODE_BREAK = -2, - DECODE_CONT = -1, - DECODE_OK = 0 +enum class MadDecoderAction { + SKIP, + BREAK, + CONT, + OK }; -enum muteframe { - MUTEFRAME_NONE, - MUTEFRAME_SKIP, - MUTEFRAME_SEEK +enum class MadDecoderMuteFrame { + NONE, + SKIP, + SEEK }; /* the number of samples of silence the decoder inserts at start */ @@ -125,7 +125,7 @@ struct MadDecoder { SignedSongTime total_time; SongTime elapsed_time; SongTime seek_time; - enum muteframe mute_frame = MUTEFRAME_NONE; + MadDecoderMuteFrame mute_frame = MadDecoderMuteFrame::NONE; long *frame_offsets = nullptr; mad_timer_t *times = nullptr; unsigned long highest_frame = 0; @@ -149,8 +149,8 @@ struct MadDecoder { bool Seek(long offset); bool FillBuffer(); void ParseId3(size_t tagsize, Tag *tag); - enum mp3_action DecodeNextFrameHeader(Tag *tag); - enum mp3_action DecodeNextFrame(); + MadDecoderAction DecodeNextFrameHeader(Tag *tag); + MadDecoderAction DecodeNextFrame(); gcc_pure offset_type ThisFrameOffset() const noexcept; @@ -360,26 +360,26 @@ id3_tag_query(const void *p0, size_t length) } #endif /* !ENABLE_ID3TAG */ -static enum mp3_action +static MadDecoderAction RecoverFrameError(struct mad_stream &stream) { if (MAD_RECOVERABLE(stream.error)) - return DECODE_SKIP; + return MadDecoderAction::SKIP; else if (stream.error == MAD_ERROR_BUFLEN) - return DECODE_CONT; + return MadDecoderAction::CONT; FormatWarning(mad_domain, "unrecoverable frame level error: %s", mad_stream_errorstr(&stream)); - return DECODE_BREAK; + return MadDecoderAction::BREAK; } -enum mp3_action +MadDecoderAction MadDecoder::DecodeNextFrameHeader(Tag *tag) { if ((stream.buffer == nullptr || stream.error == MAD_ERROR_BUFLEN) && !FillBuffer()) - return DECODE_BREAK; + return MadDecoderAction::BREAK; if (mad_header_decode(&frame.header, &stream)) { if (stream.error == MAD_ERROR_LOSTSYNC && stream.this_frame) { @@ -389,7 +389,7 @@ MadDecoder::DecodeNextFrameHeader(Tag *tag) if (tagsize > 0) { ParseId3((size_t)tagsize, tag); - return DECODE_CONT; + return MadDecoderAction::CONT; } } @@ -400,24 +400,24 @@ MadDecoder::DecodeNextFrameHeader(Tag *tag) if (layer == (mad_layer)0) { if (new_layer != MAD_LAYER_II && new_layer != MAD_LAYER_III) { /* Only layer 2 and 3 have been tested to work */ - return DECODE_SKIP; + return MadDecoderAction::SKIP; } layer = new_layer; } else if (new_layer != layer) { /* Don't decode frames with a different layer than the first */ - return DECODE_SKIP; + return MadDecoderAction::SKIP; } - return DECODE_OK; + return MadDecoderAction::OK; } -enum mp3_action +MadDecoderAction MadDecoder::DecodeNextFrame() { if ((stream.buffer == nullptr || stream.error == MAD_ERROR_BUFLEN) && !FillBuffer()) - return DECODE_BREAK; + return MadDecoderAction::BREAK; if (mad_frame_decode(&frame, &stream)) { if (stream.error == MAD_ERROR_LOSTSYNC) { @@ -426,14 +426,14 @@ MadDecoder::DecodeNextFrame() stream.this_frame); if (tagsize > 0) { mad_stream_skip(&stream, tagsize); - return DECODE_CONT; + return MadDecoderAction::CONT; } } return RecoverFrameError(stream); } - return DECODE_OK; + return MadDecoderAction::OK; } /* xing stuff stolen from alsaplayer, and heavily modified by jat */ @@ -695,20 +695,20 @@ MadDecoder::DecodeFirstFrame(Tag *tag) struct xing xing; while (true) { - enum mp3_action ret; + MadDecoderAction ret; do { ret = DecodeNextFrameHeader(tag); - } while (ret == DECODE_CONT); - if (ret == DECODE_BREAK) + } while (ret == MadDecoderAction::CONT); + if (ret == MadDecoderAction::BREAK) return false; - if (ret == DECODE_SKIP) continue; + if (ret == MadDecoderAction::SKIP) continue; do { ret = DecodeNextFrame(); - } while (ret == DECODE_CONT); - if (ret == DECODE_BREAK) + } while (ret == MadDecoderAction::CONT); + if (ret == MadDecoderAction::BREAK) return false; - if (ret == DECODE_OK) break; + if (ret == MadDecoderAction::OK) break; } struct mad_bitptr ptr = stream.anc_ptr; @@ -720,7 +720,7 @@ MadDecoder::DecodeFirstFrame(Tag *tag) * if an xing tag exists, use that! */ if (parse_xing(&xing, &ptr, &bitlen)) { - mute_frame = MUTEFRAME_SKIP; + mute_frame = MadDecoderMuteFrame::SKIP; if ((xing.flags & XING_FRAMES) && xing.frames) { mad_timer_t duration = frame.header.duration; @@ -902,14 +902,14 @@ MadDecoder::Read() switch (mute_frame) { DecoderCommand cmd; - case MUTEFRAME_SKIP: - mute_frame = MUTEFRAME_NONE; + case MadDecoderMuteFrame::SKIP: + mute_frame = MadDecoderMuteFrame::NONE; break; - case MUTEFRAME_SEEK: + case MadDecoderMuteFrame::SEEK: if (elapsed_time >= seek_time) - mute_frame = MUTEFRAME_NONE; + mute_frame = MadDecoderMuteFrame::NONE; break; - case MUTEFRAME_NONE: + case MadDecoderMuteFrame::NONE: cmd = SyncAndSend(); if (cmd == DecoderCommand::SEEK) { assert(input_stream.IsSeekable()); @@ -924,7 +924,7 @@ MadDecoder::Read() client->SeekError(); } else { seek_time = t; - mute_frame = MUTEFRAME_SEEK; + mute_frame = MadDecoderMuteFrame::SEEK; client->CommandFinished(); } } else if (cmd != DecoderCommand::NONE) @@ -932,7 +932,7 @@ MadDecoder::Read() } while (true) { - enum mp3_action ret; + MadDecoderAction ret; do { Tag tag; @@ -941,21 +941,21 @@ MadDecoder::Read() if (!tag.IsEmpty()) client->SubmitTag(input_stream, std::move(tag)); - } while (ret == DECODE_CONT); - if (ret == DECODE_BREAK) + } while (ret == MadDecoderAction::CONT); + if (ret == MadDecoderAction::BREAK) return false; - const bool skip = ret == DECODE_SKIP; + const bool skip = ret == MadDecoderAction::SKIP; - if (mute_frame == MUTEFRAME_NONE) { + if (mute_frame == MadDecoderMuteFrame::NONE) { do { ret = DecodeNextFrame(); - } while (ret == DECODE_CONT); - if (ret == DECODE_BREAK) + } while (ret == MadDecoderAction::CONT); + if (ret == MadDecoderAction::BREAK) return false; } - if (!skip && ret == DECODE_OK) + if (!skip && ret == MadDecoderAction::OK) return true; } } From f51e5551546760fa0925d3fc855d54eb7c9ce80a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 16 Jun 2019 10:01:10 +0200 Subject: [PATCH 11/37] decoder/mad: change "mp3_" suffix to "mad_" --- src/decoder/plugins/MadDecoderPlugin.cxx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 05fef9b3c..3acb4fcb4 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -105,7 +105,7 @@ mad_fixed_to_24_buffer(int32_t *dest, const struct mad_synth *synth, } static bool -mp3_plugin_init(const ConfigBlock &block) +mad_plugin_init(const ConfigBlock &block) { gapless_playback = block.GetBlockValue("gapless", DEFAULT_GAPLESS_MP3_PLAYBACK); @@ -643,7 +643,7 @@ parse_lame(struct lame *lame, struct mad_bitptr *ptr, int *bitlen) } static inline SongTime -mp3_frame_duration(const struct mad_frame *frame) +mad_frame_duration(const struct mad_frame *frame) { return ToSongTime(frame->header.duration); } @@ -673,7 +673,7 @@ MadDecoder::FileSizeToSongLength() if (input_stream.KnownSize()) { offset_type rest = RestIncludingThisFrame(); - const SongTime frame_duration = mp3_frame_duration(&frame); + const SongTime frame_duration = mad_frame_duration(&frame); const SongTime duration = SongTime::FromScale(rest, frame.header.bitrate / 8); @@ -961,7 +961,7 @@ MadDecoder::Read() } static void -mp3_decode(DecoderClient &client, InputStream &input_stream) +mad_decode(DecoderClient &client, InputStream &input_stream) { MadDecoder data(&client, input_stream); @@ -1007,18 +1007,18 @@ mad_decoder_scan_stream(InputStream &is, TagHandler &handler) noexcept return true; } -static const char *const mp3_suffixes[] = { "mp3", "mp2", nullptr }; -static const char *const mp3_mime_types[] = { "audio/mpeg", nullptr }; +static const char *const mad_suffixes[] = { "mp3", "mp2", nullptr }; +static const char *const mad_mime_types[] = { "audio/mpeg", nullptr }; const struct DecoderPlugin mad_decoder_plugin = { "mad", - mp3_plugin_init, + mad_plugin_init, nullptr, - mp3_decode, + mad_decode, nullptr, nullptr, mad_decoder_scan_stream, nullptr, - mp3_suffixes, - mp3_mime_types, + mad_suffixes, + mad_mime_types, }; From 6b8ca514bb3f327b3f23ffe4730657a93de53035 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:58:05 +0200 Subject: [PATCH 12/37] decoder/mad: fix broken log message Broken since commit f8bfea8bae44134e8002db869fd4ec137d1c0d6e --- src/decoder/plugins/MadDecoderPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 3acb4fcb4..0facfc0ec 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -969,7 +969,7 @@ mad_decode(DecoderClient &client, InputStream &input_stream) if (!data.DecodeFirstFrame(&tag)) { if (client.GetCommand() == DecoderCommand::NONE) LogError(mad_domain, - "input/Input does not appear to be a mp3 bit stream"); + "input does not appear to be a mp3 bit stream"); return; } From 1d74a029a228be228b6d852d9eff028d709e1384 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:20:55 +0200 Subject: [PATCH 13/37] decoder/mad: simplify variable initialization in FillBuffer() --- src/decoder/plugins/MadDecoderPlugin.cxx | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 0facfc0ec..7d25121cc 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -223,18 +223,14 @@ MadDecoder::Seek(long offset) inline bool MadDecoder::FillBuffer() { - size_t remaining, length; - unsigned char *dest; + size_t remaining = 0, length = READ_BUFFER_SIZE; + unsigned char *dest = input_buffer; if (stream.next_frame != nullptr) { remaining = stream.bufend - stream.next_frame; memmove(input_buffer, stream.next_frame, remaining); - dest = input_buffer + remaining; - length = READ_BUFFER_SIZE - remaining; - } else { - remaining = 0; - length = READ_BUFFER_SIZE; - dest = input_buffer; + dest += remaining; + length -= remaining; } /* we've exhausted the read buffer, so give up!, these potential From f9eff31205901653d82d84aacd39e0820f0b8f17 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:20:15 +0200 Subject: [PATCH 14/37] decoder/mad: use sizeof(input_buffer) --- src/decoder/plugins/MadDecoderPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 7d25121cc..88fc2998f 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -223,7 +223,7 @@ MadDecoder::Seek(long offset) inline bool MadDecoder::FillBuffer() { - size_t remaining = 0, length = READ_BUFFER_SIZE; + size_t remaining = 0, length = sizeof(input_buffer); unsigned char *dest = input_buffer; if (stream.next_frame != nullptr) { From 10da9ee7bab531add7edac81fe5beced37456b4b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:19:23 +0200 Subject: [PATCH 15/37] decoder/mad: refactor local variables in FillBuffer() --- src/decoder/plugins/MadDecoderPlugin.cxx | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 88fc2998f..68380489d 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -223,26 +223,31 @@ MadDecoder::Seek(long offset) inline bool MadDecoder::FillBuffer() { - size_t remaining = 0, length = sizeof(input_buffer); + /* amount of rest data still residing in the buffer */ + size_t rest_size = 0; + + size_t max_read_size = sizeof(input_buffer); unsigned char *dest = input_buffer; if (stream.next_frame != nullptr) { - remaining = stream.bufend - stream.next_frame; - memmove(input_buffer, stream.next_frame, remaining); - dest += remaining; - length -= remaining; + rest_size = stream.bufend - stream.next_frame; + memmove(input_buffer, stream.next_frame, rest_size); + dest += rest_size; + max_read_size -= rest_size; } /* we've exhausted the read buffer, so give up!, these potential * mp3 frames are way too big, and thus unlikely to be mp3 frames */ - if (length == 0) + if (max_read_size == 0) return false; - length = decoder_read(client, input_stream, dest, length); - if (length == 0) + size_t nbytes = decoder_read(client, input_stream, + dest, max_read_size); + if (nbytes == 0) { return false; + } - mad_stream_buffer(&stream, input_buffer, length + remaining); + mad_stream_buffer(&stream, input_buffer, rest_size + nbytes); stream.error = MAD_ERROR_NONE; return true; From 9d44a6d2ae7df74085210c4fc8c7cf1f617a65e5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 00:26:57 +0200 Subject: [PATCH 16/37] decoder/mad: use Clamp() --- src/decoder/plugins/MadDecoderPlugin.cxx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 68380489d..bc7ed3e39 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -27,6 +27,7 @@ #include "tag/ReplayGain.hxx" #include "tag/MixRamp.hxx" #include "CheckAudioFormat.hxx" +#include "util/Clamp.hxx" #include "util/StringCompare.hxx" #include "util/Domain.hxx" #include "Log.hxx" @@ -84,14 +85,9 @@ mad_fixed_to_24_sample(mad_fixed_t sample) /* round */ sample = sample + (1L << (MAD_F_FRACBITS - bits)); - /* clip */ - if (gcc_unlikely(sample > MAX)) - sample = MAX; - else if (gcc_unlikely(sample < MIN)) - sample = MIN; - /* quantize */ - return sample >> (MAD_F_FRACBITS + 1 - bits); + return Clamp(sample, MIN, MAX) + >> (MAD_F_FRACBITS + 1 - bits); } static void From f7ed7446ae4be9226de554d6d75a14a9fb71dd7c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 00:26:36 +0200 Subject: [PATCH 17/37] decoder/mad: use MAD_F_MIN and MAD_F_MAX --- src/decoder/plugins/MadDecoderPlugin.cxx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index bc7ed3e39..5a9490bcb 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -79,14 +79,12 @@ static inline int32_t mad_fixed_to_24_sample(mad_fixed_t sample) { static constexpr unsigned bits = 24; - static constexpr mad_fixed_t MIN = -MAD_F_ONE; - static constexpr mad_fixed_t MAX = MAD_F_ONE - 1; /* round */ sample = sample + (1L << (MAD_F_FRACBITS - bits)); /* quantize */ - return Clamp(sample, MIN, MAX) + return Clamp(sample, MAD_F_MIN, MAD_F_MAX) >> (MAD_F_FRACBITS + 1 - bits); } From 779a6855ff5f9ab5c1e2486f13fc0d42d978669c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 00:28:59 +0200 Subject: [PATCH 18/37] decoder/mad: add `noexcept` --- src/decoder/plugins/MadDecoderPlugin.cxx | 66 ++++++++++++------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 5a9490bcb..18ae27530 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -76,7 +76,7 @@ ToSongTime(mad_timer_t t) noexcept } static inline int32_t -mad_fixed_to_24_sample(mad_fixed_t sample) +mad_fixed_to_24_sample(mad_fixed_t sample) noexcept { static constexpr unsigned bits = 24; @@ -137,14 +137,14 @@ struct MadDecoder { InputStream &input_stream; enum mad_layer layer = mad_layer(0); - MadDecoder(DecoderClient *client, InputStream &input_stream); - ~MadDecoder(); + MadDecoder(DecoderClient *client, InputStream &input_stream) noexcept; + ~MadDecoder() noexcept; - bool Seek(long offset); - bool FillBuffer(); - void ParseId3(size_t tagsize, Tag *tag); - MadDecoderAction DecodeNextFrameHeader(Tag *tag); - MadDecoderAction DecodeNextFrame(); + bool Seek(long offset) noexcept; + bool FillBuffer() noexcept; + void ParseId3(size_t tagsize, Tag *tag) noexcept; + MadDecoderAction DecodeNextFrameHeader(Tag *tag) noexcept; + MadDecoderAction DecodeNextFrame() noexcept; gcc_pure offset_type ThisFrameOffset() const noexcept; @@ -155,11 +155,11 @@ struct MadDecoder { /** * Attempt to calulcate the length of the song from filesize */ - void FileSizeToSongLength(); + void FileSizeToSongLength() noexcept; - bool DecodeFirstFrame(Tag *tag); + bool DecodeFirstFrame(Tag *tag) noexcept; - void AllocateBuffers() { + void AllocateBuffers() noexcept { assert(max_frames > 0); assert(frame_offsets == nullptr); assert(times == nullptr); @@ -171,25 +171,25 @@ struct MadDecoder { gcc_pure long TimeToFrame(SongTime t) const noexcept; - void UpdateTimerNextFrame(); + void UpdateTimerNextFrame() noexcept; /** * Sends the synthesized current frame via * DecoderClient::SubmitData(). */ - DecoderCommand SendPCM(unsigned i, unsigned pcm_length); + DecoderCommand SendPCM(unsigned i, unsigned pcm_length) noexcept; /** * Synthesize the current frame and send it via * DecoderClient::SubmitData(). */ - DecoderCommand SyncAndSend(); + DecoderCommand SyncAndSend() noexcept; - bool Read(); + bool Read() noexcept; }; MadDecoder::MadDecoder(DecoderClient *_client, - InputStream &_input_stream) + InputStream &_input_stream) noexcept :client(_client), input_stream(_input_stream) { mad_stream_init(&stream); @@ -200,7 +200,7 @@ MadDecoder::MadDecoder(DecoderClient *_client, } inline bool -MadDecoder::Seek(long offset) +MadDecoder::Seek(long offset) noexcept { try { input_stream.LockSeek(offset); @@ -215,7 +215,7 @@ MadDecoder::Seek(long offset) } inline bool -MadDecoder::FillBuffer() +MadDecoder::FillBuffer() noexcept { /* amount of rest data still residing in the buffer */ size_t rest_size = 0; @@ -277,7 +277,7 @@ parse_id3_mixramp(struct id3_tag *tag) noexcept #endif inline void -MadDecoder::ParseId3(size_t tagsize, Tag *mpd_tag) +MadDecoder::ParseId3(size_t tagsize, Tag *mpd_tag) noexcept { #ifdef ENABLE_ID3TAG std::unique_ptr allocated; @@ -345,7 +345,7 @@ MadDecoder::ParseId3(size_t tagsize, Tag *mpd_tag) * of the ID3 frame. */ static signed long -id3_tag_query(const void *p0, size_t length) +id3_tag_query(const void *p0, size_t length) noexcept { const char *p = (const char *)p0; @@ -356,7 +356,7 @@ id3_tag_query(const void *p0, size_t length) #endif /* !ENABLE_ID3TAG */ static MadDecoderAction -RecoverFrameError(struct mad_stream &stream) +RecoverFrameError(struct mad_stream &stream) noexcept { if (MAD_RECOVERABLE(stream.error)) return MadDecoderAction::SKIP; @@ -370,7 +370,7 @@ RecoverFrameError(struct mad_stream &stream) } MadDecoderAction -MadDecoder::DecodeNextFrameHeader(Tag *tag) +MadDecoder::DecodeNextFrameHeader(Tag *tag) noexcept { if ((stream.buffer == nullptr || stream.error == MAD_ERROR_BUFLEN) && !FillBuffer()) @@ -408,7 +408,7 @@ MadDecoder::DecodeNextFrameHeader(Tag *tag) } MadDecoderAction -MadDecoder::DecodeNextFrame() +MadDecoder::DecodeNextFrame() noexcept { if ((stream.buffer == nullptr || stream.error == MAD_ERROR_BUFLEN) && !FillBuffer()) @@ -467,7 +467,7 @@ struct lame { }; static bool -parse_xing(struct xing *xing, struct mad_bitptr *ptr, int *oldbitlen) +parse_xing(struct xing *xing, struct mad_bitptr *ptr, int *oldbitlen) noexcept { int bitlen = *oldbitlen; @@ -547,7 +547,7 @@ parse_xing(struct xing *xing, struct mad_bitptr *ptr, int *oldbitlen) } static bool -parse_lame(struct lame *lame, struct mad_bitptr *ptr, int *bitlen) +parse_lame(struct lame *lame, struct mad_bitptr *ptr, int *bitlen) noexcept { /* Unlike the xing header, the lame tag has a fixed length. Fail if * not all 36 bytes (288 bits) are there. */ @@ -638,7 +638,7 @@ parse_lame(struct lame *lame, struct mad_bitptr *ptr, int *bitlen) } static inline SongTime -mad_frame_duration(const struct mad_frame *frame) +mad_frame_duration(const struct mad_frame *frame) noexcept { return ToSongTime(frame->header.duration); } @@ -663,7 +663,7 @@ MadDecoder::RestIncludingThisFrame() const noexcept } inline void -MadDecoder::FileSizeToSongLength() +MadDecoder::FileSizeToSongLength() noexcept { if (input_stream.KnownSize()) { offset_type rest = RestIncludingThisFrame(); @@ -685,7 +685,7 @@ MadDecoder::FileSizeToSongLength() } inline bool -MadDecoder::DecodeFirstFrame(Tag *tag) +MadDecoder::DecodeFirstFrame(Tag *tag) noexcept { struct xing xing; @@ -758,7 +758,7 @@ MadDecoder::DecodeFirstFrame(Tag *tag) return true; } -MadDecoder::~MadDecoder() +MadDecoder::~MadDecoder() noexcept { mad_synth_finish(&synth); mad_frame_finish(&frame); @@ -783,7 +783,7 @@ MadDecoder::TimeToFrame(SongTime t) const noexcept } void -MadDecoder::UpdateTimerNextFrame() +MadDecoder::UpdateTimerNextFrame() noexcept { if (current_frame >= highest_frame) { /* record this frame's properties in frame_offsets @@ -809,7 +809,7 @@ MadDecoder::UpdateTimerNextFrame() } DecoderCommand -MadDecoder::SendPCM(unsigned i, unsigned pcm_length) +MadDecoder::SendPCM(unsigned i, unsigned pcm_length) noexcept { unsigned max_samples = sizeof(output_buffer) / sizeof(output_buffer[0]) / @@ -838,7 +838,7 @@ MadDecoder::SendPCM(unsigned i, unsigned pcm_length) } inline DecoderCommand -MadDecoder::SyncAndSend() +MadDecoder::SyncAndSend() noexcept { mad_synth_frame(&synth, &frame); @@ -890,7 +890,7 @@ MadDecoder::SyncAndSend() } inline bool -MadDecoder::Read() +MadDecoder::Read() noexcept { UpdateTimerNextFrame(); From a0a74951b87402aefd15c5d93d51e5912372962f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 00:38:45 +0200 Subject: [PATCH 19/37] decoder/mad: eliminate attribute "bit_rate" This also fixes a bug which caused the bit rate to not update after seeking. --- NEWS | 1 + src/decoder/plugins/MadDecoderPlugin.cxx | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index d6bed1990..ed5b4d94c 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ ver 0.21.12 (not yet released) * decoder + - mad: update bit rate after seeking - opus: ignore case in replay gain tag names - opus, vorbis: decode the "end of stream" packet * output diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 18ae27530..509fb1381 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -132,7 +132,6 @@ struct MadDecoder { bool found_replay_gain = false; bool found_first_frame = false; bool decoded_first_frame = false; - unsigned long bit_rate; DecoderClient *const client; InputStream &input_stream; enum mad_layer layer = mad_layer(0); @@ -788,7 +787,6 @@ MadDecoder::UpdateTimerNextFrame() noexcept if (current_frame >= highest_frame) { /* record this frame's properties in frame_offsets (for seeking) and times */ - bit_rate = frame.header.bitrate; if (current_frame >= max_frames) /* cap current_frame */ @@ -829,7 +827,7 @@ MadDecoder::SendPCM(unsigned i, unsigned pcm_length) noexcept auto cmd = client->SubmitData(input_stream, output_buffer, sizeof(output_buffer[0]) * num_samples, - bit_rate / 1000); + frame.header.bitrate / 1000); if (cmd != DecoderCommand::NONE) return cmd; } From d39d2874b4642f55f9fd0c959d34eb15fe8ee972 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 07:49:41 +0200 Subject: [PATCH 20/37] decoder/mad: move code to methods RunDecoder(), RunScan() --- src/decoder/plugins/MadDecoderPlugin.cxx | 70 +++++++++++++++--------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 509fb1381..f30c3d7ae 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -139,6 +139,9 @@ struct MadDecoder { MadDecoder(DecoderClient *client, InputStream &input_stream) noexcept; ~MadDecoder() noexcept; + void RunDecoder() noexcept; + bool RunScan(TagHandler &handler) noexcept; + bool Seek(long offset) noexcept; bool FillBuffer() noexcept; void ParseId3(size_t tagsize, Tag *tag) noexcept; @@ -953,51 +956,64 @@ MadDecoder::Read() noexcept } } -static void -mad_decode(DecoderClient &client, InputStream &input_stream) +inline void +MadDecoder::RunDecoder() noexcept { - MadDecoder data(&client, input_stream); + assert(client != nullptr); Tag tag; - if (!data.DecodeFirstFrame(&tag)) { - if (client.GetCommand() == DecoderCommand::NONE) + if (!DecodeFirstFrame(&tag)) { + if (client->GetCommand() == DecoderCommand::NONE) LogError(mad_domain, "input does not appear to be a mp3 bit stream"); return; } - data.AllocateBuffers(); + AllocateBuffers(); - client.Ready(CheckAudioFormat(data.frame.header.samplerate, - SampleFormat::S24_P32, - MAD_NCHANNELS(&data.frame.header)), - input_stream.IsSeekable(), - data.total_time); + client->Ready(CheckAudioFormat(frame.header.samplerate, + SampleFormat::S24_P32, + MAD_NCHANNELS(&frame.header)), + input_stream.IsSeekable(), + total_time); if (!tag.IsEmpty()) - client.SubmitTag(input_stream, std::move(tag)); + client->SubmitTag(input_stream, std::move(tag)); - while (data.Read()) {} + while (Read()) {} +} + +static void +mad_decode(DecoderClient &client, InputStream &input_stream) +{ + MadDecoder data(&client, input_stream); + data.RunDecoder(); +} + +inline bool +MadDecoder::RunScan(TagHandler &handler) noexcept +{ + if (!DecodeFirstFrame(nullptr)) + return false; + + if (!total_time.IsNegative()) + handler.OnDuration(SongTime(total_time)); + + try { + handler.OnAudioFormat(CheckAudioFormat(frame.header.samplerate, + SampleFormat::S24_P32, + MAD_NCHANNELS(&frame.header))); + } catch (...) { + } + + return true; } static bool mad_decoder_scan_stream(InputStream &is, TagHandler &handler) noexcept { MadDecoder data(nullptr, is); - if (!data.DecodeFirstFrame(nullptr)) - return false; - - if (!data.total_time.IsNegative()) - handler.OnDuration(SongTime(data.total_time)); - - try { - handler.OnAudioFormat(CheckAudioFormat(data.frame.header.samplerate, - SampleFormat::S24_P32, - MAD_NCHANNELS(&data.frame.header))); - } catch (...) { - } - - return true; + return data.RunScan(handler); } static const char *const mad_suffixes[] = { "mp3", "mp2", nullptr }; From 00830a20e3231460bf8927844fd6fed0ddad5c0a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 07:48:50 +0200 Subject: [PATCH 21/37] decoder/mad: convert to class, make almost everything private --- src/decoder/plugins/MadDecoderPlugin.cxx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index f30c3d7ae..8bddf14be 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -106,7 +106,7 @@ mad_plugin_init(const ConfigBlock &block) return true; } -struct MadDecoder { +class MadDecoder { static constexpr size_t READ_BUFFER_SIZE = 40960; static constexpr size_t MP3_DATA_OUTPUT_BUFFER_SIZE = 2048; @@ -136,12 +136,14 @@ struct MadDecoder { InputStream &input_stream; enum mad_layer layer = mad_layer(0); +public: MadDecoder(DecoderClient *client, InputStream &input_stream) noexcept; ~MadDecoder() noexcept; void RunDecoder() noexcept; bool RunScan(TagHandler &handler) noexcept; +private: bool Seek(long offset) noexcept; bool FillBuffer() noexcept; void ParseId3(size_t tagsize, Tag *tag) noexcept; From c87d6825ec0707fcfbc7a5b3fae971d7db35cf4d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 08:04:50 +0200 Subject: [PATCH 22/37] decoder/mad: add API documentation --- src/decoder/plugins/MadDecoderPlugin.cxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 8bddf14be..3fb38e0b9 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -175,6 +175,11 @@ private: gcc_pure long TimeToFrame(SongTime t) const noexcept; + /** + * Record the current frame's offset in the "frame_offsets" + * buffer and go forward to the next frame, updating the + * attributes "current_frame" and "timer". + */ void UpdateTimerNextFrame() noexcept; /** From 4f56fdc3970047943571147d4fa320db6e900840 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 08:09:45 +0200 Subject: [PATCH 23/37] decoder/mad: make "current_frame" zero-based Increment "current_frame" after processing the frame. --- src/decoder/plugins/MadDecoderPlugin.cxx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 3fb38e0b9..7b3675e37 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -863,7 +863,7 @@ MadDecoder::SyncAndSend() noexcept drop_start_frames--; return DecoderCommand::NONE; } else if ((drop_end_frames > 0) && - (current_frame == (max_frames + 1 - drop_end_frames))) { + current_frame == max_frames - drop_end_frames) { /* stop decoding, effectively dropping all remaining frames */ return DecoderCommand::STOP; @@ -877,7 +877,7 @@ MadDecoder::SyncAndSend() noexcept unsigned pcm_length = synth.pcm.length; if (drop_end_samples && - (current_frame == max_frames - drop_end_frames)) { + current_frame == max_frames - drop_end_frames - 1) { if (drop_end_samples >= pcm_length) pcm_length = 0; else @@ -889,7 +889,7 @@ MadDecoder::SyncAndSend() noexcept return cmd; if (drop_end_samples && - (current_frame == max_frames - drop_end_frames)) + current_frame == max_frames - drop_end_frames - 1) /* stop decoding, effectively dropping * all remaining samples */ return DecoderCommand::STOP; @@ -900,20 +900,21 @@ MadDecoder::SyncAndSend() noexcept inline bool MadDecoder::Read() noexcept { - UpdateTimerNextFrame(); - switch (mute_frame) { DecoderCommand cmd; case MadDecoderMuteFrame::SKIP: mute_frame = MadDecoderMuteFrame::NONE; + UpdateTimerNextFrame(); break; case MadDecoderMuteFrame::SEEK: if (elapsed_time >= seek_time) mute_frame = MadDecoderMuteFrame::NONE; + UpdateTimerNextFrame(); break; case MadDecoderMuteFrame::NONE: cmd = SyncAndSend(); + UpdateTimerNextFrame(); if (cmd == DecoderCommand::SEEK) { assert(input_stream.IsSeekable()); From 9b99a9897a96cbc7e7c3697d5c3c4e70ffcf0047 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 08:09:45 +0200 Subject: [PATCH 24/37] decoder/mad: don't count the Xing/LAME metadata frame The Xing/LAME frame indicates how many frames there are, but that excludes the initial Xing/LAME frame. Therefore, it should not be counted. This fixes an off-by-one bug which caused the last frame to be skipped, fixing one part of https://github.com/MusicPlayerDaemon/MPD/issues/601 --- src/decoder/plugins/MadDecoderPlugin.cxx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 7b3675e37..33a9be644 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -905,7 +905,6 @@ MadDecoder::Read() noexcept case MadDecoderMuteFrame::SKIP: mute_frame = MadDecoderMuteFrame::NONE; - UpdateTimerNextFrame(); break; case MadDecoderMuteFrame::SEEK: if (elapsed_time >= seek_time) From 3e3d8c7f9d095daac987a9688b1917e556ae8649 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 2 Aug 2019 22:18:45 +0200 Subject: [PATCH 25/37] decoder/mad: pad the input buffer with zero bytes and end of file libmad requires padding the input buffer with "MAD_BUFFER_GUARD" zero bytes at the end of the file, or else it is unable to decode the last frame. This fixes yet another bug which prevented this plugin from decoding the last frame, see https://github.com/MusicPlayerDaemon/MPD/issues/601 --- src/decoder/plugins/MadDecoderPlugin.cxx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 33a9be644..626fec855 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -132,6 +132,14 @@ class MadDecoder { bool found_replay_gain = false; bool found_first_frame = false; bool decoded_first_frame = false; + + /** + * If this flag is true, then end-of-file was seen and a + * padding of 8 zero bytes were appended to #input_buffer, to + * allow libmad to decode the last frame. + */ + bool was_eof = false; + DecoderClient *const client; InputStream &input_stream; enum mad_layer layer = mad_layer(0); @@ -247,7 +255,12 @@ MadDecoder::FillBuffer() noexcept size_t nbytes = decoder_read(client, input_stream, dest, max_read_size); if (nbytes == 0) { - return false; + if (was_eof || max_read_size < MAD_BUFFER_GUARD) + return false; + + was_eof = true; + nbytes = MAD_BUFFER_GUARD; + memset(dest, 0, nbytes); } mad_stream_buffer(&stream, input_buffer, rest_size + nbytes); @@ -922,6 +935,7 @@ MadDecoder::Read() noexcept if (j < highest_frame) { if (Seek(frame_offsets[j])) { current_frame = j; + was_eof = false; client->CommandFinished(); } else client->SeekError(); From 952c79323591e28ee1bfff76cff6f4aefeaefeab Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 08:22:05 +0200 Subject: [PATCH 26/37] decoder/mad: subtract libmad decoder delay from LAME encoder padding Apparently, libmad not only inserts 529 samples of silence at the beginning of the file, but also removes them at the end. This solves the last piece of https://github.com/MusicPlayerDaemon/MPD/issues/601 Closes https://github.com/MusicPlayerDaemon/MPD/issues/601 --- NEWS | 1 + src/decoder/plugins/MadDecoderPlugin.cxx | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/NEWS b/NEWS index ed5b4d94c..1f40838f0 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,7 @@ ver 0.21.12 (not yet released) * decoder - mad: update bit rate after seeking + - mad: fix several bugs preventing the plugin from decoding the last frame - opus: ignore case in replay gain tag names - opus, vorbis: decode the "end of stream" packet * output diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 626fec855..c27103185 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -749,9 +749,17 @@ MadDecoder::DecodeFirstFrame(Tag *tag) noexcept struct lame lame; if (parse_lame(&lame, &ptr, &bitlen)) { if (gapless_playback && input_stream.IsSeekable()) { + /* libmad inserts 529 samples of + silence at the beginning and + removes those 529 samples at the + end */ drop_start_samples = lame.encoder_delay + DECODERDELAY; drop_end_samples = lame.encoder_padding; + if (drop_end_samples > DECODERDELAY) + drop_end_samples -= DECODERDELAY; + else + drop_end_samples = 0; } /* Album gain isn't currently used. See comment in From 5e5fadb5f2ec0d9c5c5baba9fe3ec673a7c78617 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 08:49:26 +0200 Subject: [PATCH 27/37] decoder/mad: remove unnecessary initializers These will not be used until they are initialized in SyncAndSend(). --- src/decoder/plugins/MadDecoderPlugin.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index c27103185..aa8772859 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -125,8 +125,8 @@ class MadDecoder { unsigned long highest_frame = 0; unsigned long max_frames = 0; unsigned long current_frame = 0; - unsigned int drop_start_frames = 0; - unsigned int drop_end_frames = 0; + unsigned int drop_start_frames; + unsigned int drop_end_frames; unsigned int drop_start_samples = 0; unsigned int drop_end_samples = 0; bool found_replay_gain = false; From 187204f03c319d68e7d883d644f96c353ff801e7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 08:51:15 +0200 Subject: [PATCH 28/37] decoder/mad: move code to HandleCurrentFrame() --- src/decoder/plugins/MadDecoderPlugin.cxx | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index aa8772859..7cc57b245 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -202,6 +202,11 @@ private: */ DecoderCommand SyncAndSend() noexcept; + /** + * @return false to stop decoding + */ + bool HandleCurrentFrame() noexcept; + bool Read() noexcept; }; @@ -919,7 +924,7 @@ MadDecoder::SyncAndSend() noexcept } inline bool -MadDecoder::Read() noexcept +MadDecoder::HandleCurrentFrame() noexcept { switch (mute_frame) { DecoderCommand cmd; @@ -956,6 +961,15 @@ MadDecoder::Read() noexcept return false; } + return true; +} + +inline bool +MadDecoder::Read() noexcept +{ + if (!HandleCurrentFrame()) + return false; + while (true) { MadDecoderAction ret; do { From 8a432c9b7f9808ccd168d8e33be3f828ec316cec Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:19:19 +0200 Subject: [PATCH 29/37] decoder/mad: move code to LoadNextFrame() --- src/decoder/plugins/MadDecoderPlugin.cxx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 7cc57b245..a73716cb1 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -207,6 +207,8 @@ private: */ bool HandleCurrentFrame() noexcept; + bool LoadNextFrame() noexcept; + bool Read() noexcept; }; @@ -965,11 +967,8 @@ MadDecoder::HandleCurrentFrame() noexcept } inline bool -MadDecoder::Read() noexcept +MadDecoder::LoadNextFrame() noexcept { - if (!HandleCurrentFrame()) - return false; - while (true) { MadDecoderAction ret; do { @@ -999,6 +998,13 @@ MadDecoder::Read() noexcept } } +inline bool +MadDecoder::Read() noexcept +{ + return HandleCurrentFrame() && + LoadNextFrame(); +} + inline void MadDecoder::RunDecoder() noexcept { From 9d0fe725ebb3a8567f1c12d7c7036e987404ca44 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:32:42 +0200 Subject: [PATCH 30/37] decoder/mad: rename a few misnamed methods --- src/decoder/plugins/MadDecoderPlugin.cxx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index a73716cb1..3e6fc7b11 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -194,13 +194,13 @@ private: * Sends the synthesized current frame via * DecoderClient::SubmitData(). */ - DecoderCommand SendPCM(unsigned i, unsigned pcm_length) noexcept; + DecoderCommand SubmitPCM(unsigned i, unsigned pcm_length) noexcept; /** * Synthesize the current frame and send it via * DecoderClient::SubmitData(). */ - DecoderCommand SyncAndSend() noexcept; + DecoderCommand SynthAndSubmit() noexcept; /** * @return false to stop decoding @@ -845,7 +845,7 @@ MadDecoder::UpdateTimerNextFrame() noexcept } DecoderCommand -MadDecoder::SendPCM(unsigned i, unsigned pcm_length) noexcept +MadDecoder::SubmitPCM(unsigned i, unsigned pcm_length) noexcept { unsigned max_samples = sizeof(output_buffer) / sizeof(output_buffer[0]) / @@ -874,7 +874,7 @@ MadDecoder::SendPCM(unsigned i, unsigned pcm_length) noexcept } inline DecoderCommand -MadDecoder::SyncAndSend() noexcept +MadDecoder::SynthAndSubmit() noexcept { mad_synth_frame(&synth, &frame); @@ -912,7 +912,7 @@ MadDecoder::SyncAndSend() noexcept pcm_length -= drop_end_samples; } - auto cmd = SendPCM(i, pcm_length); + auto cmd = SubmitPCM(i, pcm_length); if (cmd != DecoderCommand::NONE) return cmd; @@ -940,7 +940,7 @@ MadDecoder::HandleCurrentFrame() noexcept UpdateTimerNextFrame(); break; case MadDecoderMuteFrame::NONE: - cmd = SyncAndSend(); + cmd = SynthAndSubmit(); UpdateTimerNextFrame(); if (cmd == DecoderCommand::SEEK) { assert(input_stream.IsSeekable()); From d00afc912cae96b00e3730920572ee4f1d22a846 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:36:05 +0200 Subject: [PATCH 31/37] decoder/mad: eliminate the loop in SubmitPCM() libmad has a hard-coded maximum PCM buffer size; if we make our output_buffer just as large, we can avoid the loop, because any possible size will fit. --- src/decoder/plugins/MadDecoderPlugin.cxx | 33 +++++++----------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 3e6fc7b11..083f7f978 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -108,14 +108,13 @@ mad_plugin_init(const ConfigBlock &block) class MadDecoder { static constexpr size_t READ_BUFFER_SIZE = 40960; - static constexpr size_t MP3_DATA_OUTPUT_BUFFER_SIZE = 2048; struct mad_stream stream; struct mad_frame frame; struct mad_synth synth; mad_timer_t timer; unsigned char input_buffer[READ_BUFFER_SIZE]; - int32_t output_buffer[MP3_DATA_OUTPUT_BUFFER_SIZE]; + int32_t output_buffer[sizeof(mad_pcm::samples) / sizeof(mad_fixed_t)]; SignedSongTime total_time; SongTime elapsed_time; SongTime seek_time; @@ -847,30 +846,16 @@ MadDecoder::UpdateTimerNextFrame() noexcept DecoderCommand MadDecoder::SubmitPCM(unsigned i, unsigned pcm_length) noexcept { - unsigned max_samples = sizeof(output_buffer) / - sizeof(output_buffer[0]) / - MAD_NCHANNELS(&frame.header); + unsigned int num_samples = pcm_length - i; - while (i < pcm_length) { - unsigned int num_samples = pcm_length - i; - if (num_samples > max_samples) - num_samples = max_samples; + mad_fixed_to_24_buffer(output_buffer, &synth, + i, i + num_samples, + MAD_NCHANNELS(&frame.header)); + num_samples *= MAD_NCHANNELS(&frame.header); - i += num_samples; - - mad_fixed_to_24_buffer(output_buffer, &synth, - i - num_samples, i, - MAD_NCHANNELS(&frame.header)); - num_samples *= MAD_NCHANNELS(&frame.header); - - auto cmd = client->SubmitData(input_stream, output_buffer, - sizeof(output_buffer[0]) * num_samples, - frame.header.bitrate / 1000); - if (cmd != DecoderCommand::NONE) - return cmd; - } - - return DecoderCommand::NONE; + return client->SubmitData(input_stream, output_buffer, + sizeof(output_buffer[0]) * num_samples, + frame.header.bitrate / 1000); } inline DecoderCommand From 51abed9732f9b84723a96d64104f9707a40d2e45 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:40:06 +0200 Subject: [PATCH 32/37] decoder/mad: pass mad_pcm to mad_fixed_to_24_buffer() --- src/decoder/plugins/MadDecoderPlugin.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 083f7f978..5d3cb5fec 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -89,13 +89,13 @@ mad_fixed_to_24_sample(mad_fixed_t sample) noexcept } static void -mad_fixed_to_24_buffer(int32_t *dest, const struct mad_synth *synth, +mad_fixed_to_24_buffer(int32_t *dest, const struct mad_pcm &src, unsigned int start, unsigned int end, unsigned int num_channels) { for (unsigned i = start; i < end; ++i) for (unsigned c = 0; c < num_channels; ++c) - *dest++ = mad_fixed_to_24_sample(synth->pcm.samples[c][i]); + *dest++ = mad_fixed_to_24_sample(src.samples[c][i]); } static bool @@ -848,7 +848,7 @@ MadDecoder::SubmitPCM(unsigned i, unsigned pcm_length) noexcept { unsigned int num_samples = pcm_length - i; - mad_fixed_to_24_buffer(output_buffer, &synth, + mad_fixed_to_24_buffer(output_buffer, synth.pcm, i, i + num_samples, MAD_NCHANNELS(&frame.header)); num_samples *= MAD_NCHANNELS(&frame.header); From fc18fd571c83b28e04631b43b71a3999296a5a89 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:42:05 +0200 Subject: [PATCH 33/37] decoder/mad: return from SynthAndSubmit() early --- src/decoder/plugins/MadDecoderPlugin.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 5d3cb5fec..24158bb49 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -892,9 +892,9 @@ MadDecoder::SynthAndSubmit() noexcept if (drop_end_samples && current_frame == max_frames - drop_end_frames - 1) { if (drop_end_samples >= pcm_length) - pcm_length = 0; - else - pcm_length -= drop_end_samples; + return DecoderCommand::STOP; + + pcm_length -= drop_end_samples; } auto cmd = SubmitPCM(i, pcm_length); From 2a07354cadf54b6c59df056f8c963b2642ea2f58 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:44:02 +0200 Subject: [PATCH 34/37] decoder/mad: change integers to size_t --- src/decoder/plugins/MadDecoderPlugin.cxx | 28 ++++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 24158bb49..25fd740d1 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -90,10 +90,10 @@ mad_fixed_to_24_sample(mad_fixed_t sample) noexcept static void mad_fixed_to_24_buffer(int32_t *dest, const struct mad_pcm &src, - unsigned int start, unsigned int end, + size_t start, size_t end, unsigned int num_channels) { - for (unsigned i = start; i < end; ++i) + for (size_t i = start; i < end; ++i) for (unsigned c = 0; c < num_channels; ++c) *dest++ = mad_fixed_to_24_sample(src.samples[c][i]); } @@ -121,9 +121,9 @@ class MadDecoder { MadDecoderMuteFrame mute_frame = MadDecoderMuteFrame::NONE; long *frame_offsets = nullptr; mad_timer_t *times = nullptr; - unsigned long highest_frame = 0; - unsigned long max_frames = 0; - unsigned long current_frame = 0; + size_t highest_frame = 0; + size_t max_frames = 0; + size_t current_frame = 0; unsigned int drop_start_frames; unsigned int drop_end_frames; unsigned int drop_start_samples = 0; @@ -180,7 +180,7 @@ private: } gcc_pure - long TimeToFrame(SongTime t) const noexcept; + size_t TimeToFrame(SongTime t) const noexcept; /** * Record the current frame's offset in the "frame_offsets" @@ -193,7 +193,7 @@ private: * Sends the synthesized current frame via * DecoderClient::SubmitData(). */ - DecoderCommand SubmitPCM(unsigned i, unsigned pcm_length) noexcept; + DecoderCommand SubmitPCM(size_t start, size_t n) noexcept; /** * Synthesize the current frame and send it via @@ -804,10 +804,10 @@ MadDecoder::~MadDecoder() noexcept delete[] times; } -long +size_t MadDecoder::TimeToFrame(SongTime t) const noexcept { - unsigned long i; + size_t i; for (i = 0; i < highest_frame; ++i) { auto frame_time = ToSongTime(times[i]); @@ -844,9 +844,9 @@ MadDecoder::UpdateTimerNextFrame() noexcept } DecoderCommand -MadDecoder::SubmitPCM(unsigned i, unsigned pcm_length) noexcept +MadDecoder::SubmitPCM(size_t i, size_t pcm_length) noexcept { - unsigned int num_samples = pcm_length - i; + size_t num_samples = pcm_length - i; mad_fixed_to_24_buffer(output_buffer, synth.pcm, i, i + num_samples, @@ -882,13 +882,13 @@ MadDecoder::SynthAndSubmit() noexcept return DecoderCommand::STOP; } - unsigned i = 0; + size_t i = 0; if (!decoded_first_frame) { i = drop_start_samples; decoded_first_frame = true; } - unsigned pcm_length = synth.pcm.length; + size_t pcm_length = synth.pcm.length; if (drop_end_samples && current_frame == max_frames - drop_end_frames - 1) { if (drop_end_samples >= pcm_length) @@ -931,7 +931,7 @@ MadDecoder::HandleCurrentFrame() noexcept assert(input_stream.IsSeekable()); const auto t = client->GetSeekTime(); - unsigned long j = TimeToFrame(t); + size_t j = TimeToFrame(t); if (j < highest_frame) { if (Seek(frame_offsets[j])) { current_frame = j; From 9661062ae2f5b2bb94f3e4f32ef95baabd8ad63a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 11:59:41 +0200 Subject: [PATCH 35/37] decoder/mad: pass const reference to RecoverFrameError() --- src/decoder/plugins/MadDecoderPlugin.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 25fd740d1..1e357fd81 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -384,7 +384,7 @@ id3_tag_query(const void *p0, size_t length) noexcept #endif /* !ENABLE_ID3TAG */ static MadDecoderAction -RecoverFrameError(struct mad_stream &stream) noexcept +RecoverFrameError(const struct mad_stream &stream) noexcept { if (MAD_RECOVERABLE(stream.error)) return MadDecoderAction::SKIP; From f2d8fd769de98df13568989b7d6d164474380d19 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 12:30:10 +0200 Subject: [PATCH 36/37] player/Thread: don't restart unseekable song after failed seek attempt The check IsSeekableCurrentSong() was added by commit 44b200240f1f4b8394dd2e58fec72da3d3ec448f in version 0.20.19, but it caused a regression: by doing the branch only if the current song is seekable, the player would restart the current song if it was not seekable, and later the initial seek would fail; but we already know it's not seekable, and so we should fail early. --- NEWS | 2 ++ src/decoder/Control.hxx | 5 +++++ src/player/Thread.cxx | 13 +++++++++++++ 3 files changed, 20 insertions(+) diff --git a/NEWS b/NEWS index 1f40838f0..a4cade59e 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ ver 0.21.12 (not yet released) - opus, vorbis: decode the "end of stream" packet * output - jack: fix mono-to-stereo conversion +* player + - don't restart unseekable song after failed seek attempt * Windows - support backslash in relative URIs loaded from playlists diff --git a/src/decoder/Control.hxx b/src/decoder/Control.hxx index 1e272255f..385516a62 100644 --- a/src/decoder/Control.hxx +++ b/src/decoder/Control.hxx @@ -320,6 +320,11 @@ public: gcc_pure bool IsCurrentSong(const DetachedSong &_song) const noexcept; + gcc_pure + bool IsUnseekableCurrentSong(const DetachedSong &_song) const noexcept { + return !seekable && IsCurrentSong(_song); + } + gcc_pure bool IsSeekableCurrentSong(const DetachedSong &_song) const noexcept { return seekable && IsCurrentSong(_song); diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index 028842b61..05e691bd1 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -599,6 +599,19 @@ Player::SeekDecoder() noexcept { assert(pc.next_song != nullptr); + if (pc.seek_time > SongTime::zero() && // TODO: allow this only if the song duration is known + dc.IsUnseekableCurrentSong(*pc.next_song)) { + /* seeking into the current song; but we already know + it's not seekable, so let's fail early */ + /* note the seek_time>0 check: if seeking to the + beginning, we can simply restart the decoder */ + pc.next_song.reset(); + pc.SetError(PlayerError::DECODER, + std::make_exception_ptr(std::runtime_error("Not seekable"))); + pc.CommandFinished(); + return true; + } + CancelPendingSeek(); { From ae19bda1f297727e47b867ca8394692682553958 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 3 Aug 2019 12:48:20 +0200 Subject: [PATCH 37/37] release v0.21.12 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index a4cade59e..daddfec74 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.21.12 (not yet released) +ver 0.21.12 (2019/08/03) * decoder - mad: update bit rate after seeking - mad: fix several bugs preventing the plugin from decoding the last frame