From 374cc51f770f73feef4c24b35c6c522ccc865f02 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Jun 2020 17:37:30 +0200 Subject: [PATCH] decoder/Bridge: add flag to make initial seek errors fatal When the client wants to seek, but the decoder has already finished decoding the current song, the player restarts the decoder with an initial seek at the new position. When this initial seek fails, MPD pretends nothing has happened and plays this song from the start. With this new flag, a restarted decoder marks the initial seek as "essential" and fails the decoder if that seek fails. Closes https://github.com/MusicPlayerDaemon/MPD/issues/895 --- NEWS | 2 ++ src/decoder/Bridge.cxx | 6 ++++++ src/decoder/Bridge.hxx | 7 +++++++ src/decoder/Control.cxx | 2 ++ src/decoder/Control.hxx | 9 +++++++++ src/decoder/Thread.cxx | 1 + src/player/Thread.cxx | 15 +++++++++------ 7 files changed, 36 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 34057bbc2..c6924f5aa 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ ver 0.21.24 (not yet released) - modplug: fix Windows build failure - wildmidi: attempt to detect WildMidi using pkg-config - wildmidi: fix Windows build failure +* player + - don't restart current song if seeking beyond end * Android - enable the decoder plugins ModPlug and WildMidi - fix build failure with Android NDK r21 diff --git a/src/decoder/Bridge.cxx b/src/decoder/Bridge.cxx index 59d80cc48..e11a0a45a 100644 --- a/src/decoder/Bridge.cxx +++ b/src/decoder/Bridge.cxx @@ -33,6 +33,8 @@ #include "util/ConstBuffer.hxx" #include "util/StringBuffer.hxx" +#include + #include #include #include @@ -344,6 +346,10 @@ DecoderBridge::SeekError() /* d'oh, we can't seek to the sub-song start position, what now? - no idea, ignoring the problem for now. */ initial_seek_running = false; + + if (initial_seek_essential) + error = std::make_exception_ptr(std::runtime_error("Decoder failed to seek")); + return; } diff --git a/src/decoder/Bridge.hxx b/src/decoder/Bridge.hxx index f94909f31..eb005ea79 100644 --- a/src/decoder/Bridge.hxx +++ b/src/decoder/Bridge.hxx @@ -62,6 +62,11 @@ public: */ bool initial_seek_pending; + /** + * Are initial seek failures fatal? + */ + const bool initial_seek_essential; + /** * Is the initial seek currently running? During this time, * the decoder command is SEEK. This flag is set by @@ -107,9 +112,11 @@ public: std::exception_ptr error; DecoderBridge(DecoderControl &_dc, bool _initial_seek_pending, + bool _initial_seek_essential, std::unique_ptr _tag) :dc(_dc), initial_seek_pending(_initial_seek_pending), + initial_seek_essential(_initial_seek_essential), song_tag(std::move(_tag)) {} ~DecoderBridge(); diff --git a/src/decoder/Control.cxx b/src/decoder/Control.cxx index cf8a74385..4962e194b 100644 --- a/src/decoder/Control.cxx +++ b/src/decoder/Control.cxx @@ -90,6 +90,7 @@ DecoderControl::IsCurrentSong(const DetachedSong &_song) const noexcept void DecoderControl::Start(std::unique_ptr _song, SongTime _start_time, SongTime _end_time, + bool _initial_seek_essential, MusicBuffer &_buffer, std::shared_ptr _pipe) noexcept { @@ -99,6 +100,7 @@ DecoderControl::Start(std::unique_ptr _song, song = std::move(_song); start_time = _start_time; end_time = _end_time; + initial_seek_essential = _initial_seek_essential; buffer = &_buffer; pipe = std::move(_pipe); diff --git a/src/decoder/Control.hxx b/src/decoder/Control.hxx index 385516a62..b630a7283 100644 --- a/src/decoder/Control.hxx +++ b/src/decoder/Control.hxx @@ -117,6 +117,12 @@ public: bool seek_error; bool seekable; + + /** + * @see #DecoderBridge::initial_seek_essential + */ + bool initial_seek_essential; + SongTime seek_time; private: @@ -398,11 +404,14 @@ public: * owned and freed by the decoder * @param start_time see #DecoderControl * @param end_time see #DecoderControl + * @param initial_seek_essential see + * #DecoderBridge::initial_seek_essential * @param pipe the pipe which receives the decoded chunks (owned by * the caller) */ void Start(std::unique_ptr song, SongTime start_time, SongTime end_time, + bool initial_seek_essential, MusicBuffer &buffer, std::shared_ptr pipe) noexcept; diff --git a/src/decoder/Thread.cxx b/src/decoder/Thread.cxx index 45129a206..b2670de65 100644 --- a/src/decoder/Thread.cxx +++ b/src/decoder/Thread.cxx @@ -461,6 +461,7 @@ decoder_run_song(DecoderControl &dc, dc.start_time = dc.seek_time; DecoderBridge bridge(dc, dc.start_time.IsPositive(), + dc.initial_seek_essential, /* pass the song tag only if it's authoritative, i.e. if it's a local file - tags on "stream" songs are just diff --git a/src/player/Thread.cxx b/src/player/Thread.cxx index a10429472..286855136 100644 --- a/src/player/Thread.cxx +++ b/src/player/Thread.cxx @@ -223,7 +223,8 @@ private: * * Caller must lock the mutex. */ - void StartDecoder(std::shared_ptr pipe) noexcept; + void StartDecoder(std::shared_ptr pipe, + bool initial_seek_essential) noexcept; /** * The decoder has acknowledged the "START" command (see @@ -364,7 +365,8 @@ public: }; void -Player::StartDecoder(std::shared_ptr _pipe) noexcept +Player::StartDecoder(std::shared_ptr _pipe, + bool initial_seek_essential) noexcept { assert(queued || pc.command == PlayerCommand::SEEK); assert(pc.next_song != nullptr); @@ -376,6 +378,7 @@ Player::StartDecoder(std::shared_ptr _pipe) noexcept dc.Start(std::make_unique(*pc.next_song), start_time, pc.next_song->GetEndTime(), + initial_seek_essential, buffer, std::move(_pipe)); } @@ -633,7 +636,7 @@ Player::SeekDecoder() noexcept pipe->Clear(); /* re-start the decoder */ - StartDecoder(pipe); + StartDecoder(pipe, true); ActivateDecoder(); pc.seeking = true; @@ -711,7 +714,7 @@ Player::ProcessCommand() noexcept pc.CommandFinished(); if (dc.IsIdle()) - StartDecoder(std::make_shared()); + StartDecoder(std::make_shared(), false); break; @@ -982,7 +985,7 @@ Player::Run() noexcept const std::lock_guard lock(pc.mutex); - StartDecoder(pipe); + StartDecoder(pipe, true); ActivateDecoder(); pc.state = PlayerState::PLAY; @@ -1022,7 +1025,7 @@ Player::Run() noexcept assert(dc.pipe == nullptr || dc.pipe == pipe); - StartDecoder(std::make_shared()); + StartDecoder(std::make_shared(), false); } if (/* no cross-fading if MPD is going to pause at the