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
This commit is contained in:
Max Kellermann 2020-06-10 17:37:30 +02:00
parent 1008d5f67c
commit 374cc51f77
7 changed files with 36 additions and 6 deletions

2
NEWS
View File

@ -7,6 +7,8 @@ ver 0.21.24 (not yet released)
- modplug: fix Windows build failure - modplug: fix Windows build failure
- wildmidi: attempt to detect WildMidi using pkg-config - wildmidi: attempt to detect WildMidi using pkg-config
- wildmidi: fix Windows build failure - wildmidi: fix Windows build failure
* player
- don't restart current song if seeking beyond end
* Android * Android
- enable the decoder plugins ModPlug and WildMidi - enable the decoder plugins ModPlug and WildMidi
- fix build failure with Android NDK r21 - fix build failure with Android NDK r21

View File

@ -33,6 +33,8 @@
#include "util/ConstBuffer.hxx" #include "util/ConstBuffer.hxx"
#include "util/StringBuffer.hxx" #include "util/StringBuffer.hxx"
#include <stdexcept>
#include <assert.h> #include <assert.h>
#include <string.h> #include <string.h>
#include <math.h> #include <math.h>
@ -344,6 +346,10 @@ DecoderBridge::SeekError()
/* d'oh, we can't seek to the sub-song start position, /* d'oh, we can't seek to the sub-song start position,
what now? - no idea, ignoring the problem for now. */ what now? - no idea, ignoring the problem for now. */
initial_seek_running = false; initial_seek_running = false;
if (initial_seek_essential)
error = std::make_exception_ptr(std::runtime_error("Decoder failed to seek"));
return; return;
} }

View File

@ -62,6 +62,11 @@ public:
*/ */
bool initial_seek_pending; bool initial_seek_pending;
/**
* Are initial seek failures fatal?
*/
const bool initial_seek_essential;
/** /**
* Is the initial seek currently running? During this time, * Is the initial seek currently running? During this time,
* the decoder command is SEEK. This flag is set by * the decoder command is SEEK. This flag is set by
@ -107,9 +112,11 @@ public:
std::exception_ptr error; std::exception_ptr error;
DecoderBridge(DecoderControl &_dc, bool _initial_seek_pending, DecoderBridge(DecoderControl &_dc, bool _initial_seek_pending,
bool _initial_seek_essential,
std::unique_ptr<Tag> _tag) std::unique_ptr<Tag> _tag)
:dc(_dc), :dc(_dc),
initial_seek_pending(_initial_seek_pending), initial_seek_pending(_initial_seek_pending),
initial_seek_essential(_initial_seek_essential),
song_tag(std::move(_tag)) {} song_tag(std::move(_tag)) {}
~DecoderBridge(); ~DecoderBridge();

View File

@ -90,6 +90,7 @@ DecoderControl::IsCurrentSong(const DetachedSong &_song) const noexcept
void void
DecoderControl::Start(std::unique_ptr<DetachedSong> _song, DecoderControl::Start(std::unique_ptr<DetachedSong> _song,
SongTime _start_time, SongTime _end_time, SongTime _start_time, SongTime _end_time,
bool _initial_seek_essential,
MusicBuffer &_buffer, MusicBuffer &_buffer,
std::shared_ptr<MusicPipe> _pipe) noexcept std::shared_ptr<MusicPipe> _pipe) noexcept
{ {
@ -99,6 +100,7 @@ DecoderControl::Start(std::unique_ptr<DetachedSong> _song,
song = std::move(_song); song = std::move(_song);
start_time = _start_time; start_time = _start_time;
end_time = _end_time; end_time = _end_time;
initial_seek_essential = _initial_seek_essential;
buffer = &_buffer; buffer = &_buffer;
pipe = std::move(_pipe); pipe = std::move(_pipe);

View File

@ -117,6 +117,12 @@ public:
bool seek_error; bool seek_error;
bool seekable; bool seekable;
/**
* @see #DecoderBridge::initial_seek_essential
*/
bool initial_seek_essential;
SongTime seek_time; SongTime seek_time;
private: private:
@ -398,11 +404,14 @@ public:
* owned and freed by the decoder * owned and freed by the decoder
* @param start_time see #DecoderControl * @param start_time see #DecoderControl
* @param end_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 * @param pipe the pipe which receives the decoded chunks (owned by
* the caller) * the caller)
*/ */
void Start(std::unique_ptr<DetachedSong> song, void Start(std::unique_ptr<DetachedSong> song,
SongTime start_time, SongTime end_time, SongTime start_time, SongTime end_time,
bool initial_seek_essential,
MusicBuffer &buffer, MusicBuffer &buffer,
std::shared_ptr<MusicPipe> pipe) noexcept; std::shared_ptr<MusicPipe> pipe) noexcept;

View File

@ -461,6 +461,7 @@ decoder_run_song(DecoderControl &dc,
dc.start_time = dc.seek_time; dc.start_time = dc.seek_time;
DecoderBridge bridge(dc, dc.start_time.IsPositive(), DecoderBridge bridge(dc, dc.start_time.IsPositive(),
dc.initial_seek_essential,
/* pass the song tag only if it's /* pass the song tag only if it's
authoritative, i.e. if it's a local authoritative, i.e. if it's a local
file - tags on "stream" songs are just file - tags on "stream" songs are just

View File

@ -223,7 +223,8 @@ private:
* *
* Caller must lock the mutex. * Caller must lock the mutex.
*/ */
void StartDecoder(std::shared_ptr<MusicPipe> pipe) noexcept; void StartDecoder(std::shared_ptr<MusicPipe> pipe,
bool initial_seek_essential) noexcept;
/** /**
* The decoder has acknowledged the "START" command (see * The decoder has acknowledged the "START" command (see
@ -364,7 +365,8 @@ public:
}; };
void void
Player::StartDecoder(std::shared_ptr<MusicPipe> _pipe) noexcept Player::StartDecoder(std::shared_ptr<MusicPipe> _pipe,
bool initial_seek_essential) noexcept
{ {
assert(queued || pc.command == PlayerCommand::SEEK); assert(queued || pc.command == PlayerCommand::SEEK);
assert(pc.next_song != nullptr); assert(pc.next_song != nullptr);
@ -376,6 +378,7 @@ Player::StartDecoder(std::shared_ptr<MusicPipe> _pipe) noexcept
dc.Start(std::make_unique<DetachedSong>(*pc.next_song), dc.Start(std::make_unique<DetachedSong>(*pc.next_song),
start_time, pc.next_song->GetEndTime(), start_time, pc.next_song->GetEndTime(),
initial_seek_essential,
buffer, std::move(_pipe)); buffer, std::move(_pipe));
} }
@ -633,7 +636,7 @@ Player::SeekDecoder() noexcept
pipe->Clear(); pipe->Clear();
/* re-start the decoder */ /* re-start the decoder */
StartDecoder(pipe); StartDecoder(pipe, true);
ActivateDecoder(); ActivateDecoder();
pc.seeking = true; pc.seeking = true;
@ -711,7 +714,7 @@ Player::ProcessCommand() noexcept
pc.CommandFinished(); pc.CommandFinished();
if (dc.IsIdle()) if (dc.IsIdle())
StartDecoder(std::make_shared<MusicPipe>()); StartDecoder(std::make_shared<MusicPipe>(), false);
break; break;
@ -982,7 +985,7 @@ Player::Run() noexcept
const std::lock_guard<Mutex> lock(pc.mutex); const std::lock_guard<Mutex> lock(pc.mutex);
StartDecoder(pipe); StartDecoder(pipe, true);
ActivateDecoder(); ActivateDecoder();
pc.state = PlayerState::PLAY; pc.state = PlayerState::PLAY;
@ -1022,7 +1025,7 @@ Player::Run() noexcept
assert(dc.pipe == nullptr || dc.pipe == pipe); assert(dc.pipe == nullptr || dc.pipe == pipe);
StartDecoder(std::make_shared<MusicPipe>()); StartDecoder(std::make_shared<MusicPipe>(), false);
} }
if (/* no cross-fading if MPD is going to pause at the if (/* no cross-fading if MPD is going to pause at the