From 6e52ab285a7d8e94bb3ecda98f3dfa2e35ade0bf Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 8 Sep 2016 10:29:49 +0200 Subject: [PATCH] player/Control: use class Error as C++ exception, throw it --- src/Partition.hxx | 31 +++++++-------- src/command/PlayerCommands.cxx | 48 ++++++++--------------- src/player/Control.cxx | 27 +++++-------- src/player/Control.hxx | 15 ++++--- src/queue/Playlist.cxx | 16 ++++---- src/queue/Playlist.hxx | 56 +++++++++++++++++++-------- src/queue/PlaylistControl.cxx | 71 ++++++++++++++++------------------ src/queue/PlaylistEdit.cxx | 7 +++- src/queue/PlaylistState.cxx | 20 +++++++--- 9 files changed, 150 insertions(+), 141 deletions(-) diff --git a/src/Partition.hxx b/src/Partition.hxx index 32dfa834a..d5a619e08 100644 --- a/src/Partition.hxx +++ b/src/Partition.hxx @@ -129,35 +129,32 @@ struct Partition final : QueueListener, PlayerListener, MixerListener { playlist.Stop(pc); } - bool PlayPosition(int position, Error &error) { - return playlist.PlayPosition(pc, position, error); + void PlayPosition(int position) { + return playlist.PlayPosition(pc, position); } - bool PlayId(int id, Error &error) { - return playlist.PlayId(pc, id, error); + void PlayId(int id) { + return playlist.PlayId(pc, id); } - bool PlayNext(Error &error) { - return playlist.PlayNext(pc, error); + void PlayNext() { + return playlist.PlayNext(pc); } - bool PlayPrevious(Error &error) { - return playlist.PlayPrevious(pc, error); + void PlayPrevious() { + return playlist.PlayPrevious(pc); } - bool SeekSongPosition(unsigned song_position, - SongTime seek_time, Error &error) { - return playlist.SeekSongPosition(pc, song_position, seek_time, - error); + void SeekSongPosition(unsigned song_position, SongTime seek_time) { + playlist.SeekSongPosition(pc, song_position, seek_time); } - bool SeekSongId(unsigned song_id, SongTime seek_time, Error &error) { - return playlist.SeekSongId(pc, song_id, seek_time, error); + void SeekSongId(unsigned song_id, SongTime seek_time) { + playlist.SeekSongId(pc, song_id, seek_time); } - bool SeekCurrent(SignedSongTime seek_time, bool relative, - Error &error) { - return playlist.SeekCurrent(pc, seek_time, relative, error); + void SeekCurrent(SignedSongTime seek_time, bool relative) { + playlist.SeekCurrent(pc, seek_time, relative); } void SetRepeat(bool new_value) { diff --git a/src/command/PlayerCommands.cxx b/src/command/PlayerCommands.cxx index eec1ff77b..a5705988a 100644 --- a/src/command/PlayerCommands.cxx +++ b/src/command/PlayerCommands.cxx @@ -62,10 +62,8 @@ handle_play(Client &client, Request args, gcc_unused Response &r) { int song = args.ParseOptional(0, -1); - Error error; - return client.partition.PlayPosition(song, error) - ? CommandResult::OK - : print_error(r, error); + client.partition.PlayPosition(song); + return CommandResult::OK; } CommandResult @@ -73,10 +71,8 @@ handle_playid(Client &client, Request args, gcc_unused Response &r) { int id = args.ParseOptional(0, -1); - Error error; - return client.partition.PlayId(id, error) - ? CommandResult::OK - : print_error(r, error); + client.partition.PlayId(id); + return CommandResult::OK; } CommandResult @@ -223,20 +219,16 @@ handle_next(Client &client, gcc_unused Request args, gcc_unused Response &r) playlist.queue.single = single; }; - Error error; - return client.partition.PlayNext(error) - ? CommandResult::OK - : print_error(r, error); + client.partition.PlayNext(); + return CommandResult::OK; } CommandResult handle_previous(Client &client, gcc_unused Request args, gcc_unused Response &r) { - Error error; - return client.partition.PlayPrevious(error) - ? CommandResult::OK - : print_error(r, error); + client.partition.PlayPrevious(); + return CommandResult::OK; } CommandResult @@ -281,40 +273,34 @@ handle_clearerror(Client &client, gcc_unused Request args, } CommandResult -handle_seek(Client &client, Request args, Response &r) +handle_seek(Client &client, Request args, gcc_unused Response &r) { unsigned song = args.ParseUnsigned(0); SongTime seek_time = args.ParseSongTime(1); - Error error; - return client.partition.SeekSongPosition(song, seek_time, error) - ? CommandResult::OK - : print_error(r, error); + client.partition.SeekSongPosition(song, seek_time); + return CommandResult::OK; } CommandResult -handle_seekid(Client &client, Request args, Response &r) +handle_seekid(Client &client, Request args, gcc_unused Response &r) { unsigned id = args.ParseUnsigned(0); SongTime seek_time = args.ParseSongTime(1); - Error error; - return client.partition.SeekSongId(id, seek_time, error) - ? CommandResult::OK - : print_error(r, error); + client.partition.SeekSongId(id, seek_time); + return CommandResult::OK; } CommandResult -handle_seekcur(Client &client, Request args, Response &r) +handle_seekcur(Client &client, Request args, gcc_unused Response &r) { const char *p = args.front(); bool relative = *p == '+' || *p == '-'; SignedSongTime seek_time = ParseCommandArgSignedSongTime(p); - Error error; - return client.partition.SeekCurrent(seek_time, relative, error) - ? CommandResult::OK - : print_error(r, error); + client.partition.SeekCurrent(seek_time, relative); + return CommandResult::OK; } CommandResult diff --git a/src/player/Control.cxx b/src/player/Control.cxx index 26e4e0210..e495c0d00 100644 --- a/src/player/Control.cxx +++ b/src/player/Control.cxx @@ -49,20 +49,18 @@ PlayerControl::~PlayerControl() delete tagged_song; } -bool -PlayerControl::Play(DetachedSong *song, Error &error_r) +void +PlayerControl::Play(DetachedSong *song) { assert(song != nullptr); const ScopeLock protect(mutex); - bool success = SeekLocked(song, SongTime::zero(), error_r); + SeekLocked(song, SongTime::zero()); - if (success && state == PlayerState::PAUSE) + if (state == PlayerState::PAUSE) /* if the player was paused previously, we need to unpause it */ PauseLocked(); - - return success; } void @@ -203,8 +201,8 @@ PlayerControl::LockEnqueueSong(DetachedSong *song) EnqueueSongLocked(song); } -bool -PlayerControl::SeekLocked(DetachedSong *song, SongTime t, Error &error_r) +void +PlayerControl::SeekLocked(DetachedSong *song, SongTime t) { assert(song != nullptr); @@ -226,28 +224,23 @@ PlayerControl::SeekLocked(DetachedSong *song, SongTime t, Error &error_r) if (error_type != PlayerError::NONE) { assert(error.IsDefined()); - error_r.Set(error); - return false; + throw error; } assert(!error.IsDefined()); - return true; } -bool -PlayerControl::LockSeek(DetachedSong *song, SongTime t, Error &error_r) +void +PlayerControl::LockSeek(DetachedSong *song, SongTime t) { assert(song != nullptr); { const ScopeLock protect(mutex); - if (!SeekLocked(song, t, error_r)) - return false; + SeekLocked(song, t); } idle_add(IDLE_PLAYER); - - return true; } void diff --git a/src/player/Control.hxx b/src/player/Control.hxx index f9a224d44..1fda61ede 100644 --- a/src/player/Control.hxx +++ b/src/player/Control.hxx @@ -308,10 +308,12 @@ private: public: /** + * Throws std::runtime_error or #Error on error. + * * @param song the song to be queued; the given instance will * be owned and freed by the player */ - bool Play(DetachedSong *song, Error &error); + void Play(DetachedSong *song); /** * see PlayerCommand::CANCEL @@ -425,7 +427,10 @@ private: SynchronousCommand(PlayerCommand::QUEUE); } - bool SeekLocked(DetachedSong *song, SongTime t, Error &error_r); + /** + * Throws std::runtime_error or #Error on error. + */ + void SeekLocked(DetachedSong *song, SongTime t); public: /** @@ -437,12 +442,12 @@ public: /** * Makes the player thread seek the specified song to a position. * + * Throws std::runtime_error or #Error on error. + * * @param song the song to be queued; the given instance will be owned * and freed by the player - * @return true on success, false on failure (e.g. if MPD isn't - * playing currently) */ - bool LockSeek(DetachedSong *song, SongTime t, Error &error_r); + void LockSeek(DetachedSong *song, SongTime t); void SetCrossFade(float cross_fade_seconds); diff --git a/src/queue/Playlist.cxx b/src/queue/Playlist.cxx index f7984b34a..cc0a9efde 100644 --- a/src/queue/Playlist.cxx +++ b/src/queue/Playlist.cxx @@ -151,8 +151,8 @@ playlist::UpdateQueuedSong(PlayerControl &pc, const DetachedSong *prev) } } -bool -playlist::PlayOrder(PlayerControl &pc, unsigned order, Error &error) +void +playlist::PlayOrder(PlayerControl &pc, unsigned order) { playing = true; queued = -1; @@ -163,12 +163,9 @@ playlist::PlayOrder(PlayerControl &pc, unsigned order, Error &error) current = order; - if (!pc.Play(new DetachedSong(song), error)) - return false; + pc.Play(new DetachedSong(song)); SongStarted(); - - return true; } void @@ -227,8 +224,11 @@ playlist::ResumePlayback(PlayerControl &pc) Stop(pc); else /* continue playback at the next song */ - /* TODO: log error? */ - PlayNext(pc, IgnoreError()); + try { + PlayNext(pc); + } catch (...) { + /* TODO: log error? */ + } } void diff --git a/src/queue/Playlist.hxx b/src/queue/Playlist.hxx index c588e092f..2e9f81cd1 100644 --- a/src/queue/Playlist.hxx +++ b/src/queue/Playlist.hxx @@ -275,41 +275,63 @@ public: void Stop(PlayerControl &pc); - bool PlayPosition(PlayerControl &pc, int position, Error &error); + /** + * Throws std::runtime_error or #Error on error. + */ + void PlayPosition(PlayerControl &pc, int position); - bool PlayOrder(PlayerControl &pc, unsigned order, Error &error); + /** + * Throws std::runtime_error or #Error on error. + */ + void PlayOrder(PlayerControl &pc, unsigned order); - bool PlayId(PlayerControl &pc, int id, Error &error); + /** + * Throws std::runtime_error or #Error on error. + */ + void PlayId(PlayerControl &pc, int id); - bool PlayNext(PlayerControl &pc, Error &error); + /** + * Throws std::runtime_error or #Error on error. + */ + void PlayNext(PlayerControl &pc); - bool PlayPrevious(PlayerControl &pc, Error &error); + /** + * Throws std::runtime_error or #Error on error. + */ + void PlayPrevious(PlayerControl &pc); - bool SeekSongOrder(PlayerControl &pc, + /** + * Throws std::runtime_error or #Error on error. + */ + void SeekSongOrder(PlayerControl &pc, unsigned song_order, - SongTime seek_time, - Error &error); + SongTime seek_time); - bool SeekSongPosition(PlayerControl &pc, + /** + * Throws std::runtime_error or #Error on error. + */ + void SeekSongPosition(PlayerControl &pc, unsigned sonag_position, - SongTime seek_time, - Error &error); + SongTime seek_time); - bool SeekSongId(PlayerControl &pc, - unsigned song_id, SongTime seek_time, - Error &error); + /** + * Throws std::runtime_error or #Error on error. + */ + void SeekSongId(PlayerControl &pc, + unsigned song_id, SongTime seek_time); /** * Seek within the current song. Fails if MPD is not currently * playing. * + * Throws std::runtime_error or #Error on error. + * * @param seek_time the time * @param relative if true, then the specified time is relative to the * current position */ - bool SeekCurrent(PlayerControl &pc, - SignedSongTime seek_time, bool relative, - Error &error); + void SeekCurrent(PlayerControl &pc, + SignedSongTime seek_time, bool relative); bool GetRepeat() const { return queue.repeat; diff --git a/src/queue/PlaylistControl.cxx b/src/queue/PlaylistControl.cxx index 84e0f82cf..43c701623 100644 --- a/src/queue/PlaylistControl.cxx +++ b/src/queue/PlaylistControl.cxx @@ -56,8 +56,8 @@ playlist::Stop(PlayerControl &pc) } } -bool -playlist::PlayPosition(PlayerControl &pc, int song, Error &error) +void +playlist::PlayPosition(PlayerControl &pc, int song) { pc.LockClearError(); @@ -66,13 +66,13 @@ playlist::PlayPosition(PlayerControl &pc, int song, Error &error) /* play any song ("current" song, or the first song */ if (queue.IsEmpty()) - return true; + return; if (playing) { /* already playing: unpause playback, just in case it was paused, and return */ pc.LockSetPause(false); - return true; + return; } /* select a song: "current" song, or the first one */ @@ -102,24 +102,26 @@ playlist::PlayPosition(PlayerControl &pc, int song, Error &error) stop_on_error = false; error_count = 0; - return PlayOrder(pc, i, error); + PlayOrder(pc, i); } -bool -playlist::PlayId(PlayerControl &pc, int id, Error &error) +void +playlist::PlayId(PlayerControl &pc, int id) { - if (id == -1) - return PlayPosition(pc, id, error); + if (id == -1) { + PlayPosition(pc, id); + return; + } int song = queue.IdToPosition(id); if (song < 0) throw PlaylistError::NoSuchSong(); - return PlayPosition(pc, song, error); + PlayPosition(pc, song); } -bool -playlist::PlayNext(PlayerControl &pc, Error &error) +void +playlist::PlayNext(PlayerControl &pc) { if (!playing) throw PlaylistError::NotPlaying(); @@ -156,19 +158,16 @@ playlist::PlayNext(PlayerControl &pc, Error &error) discard them anyway */ } - if (!PlayOrder(pc, next_order, error)) - return false; + PlayOrder(pc, next_order); } /* Consume mode removes each played songs. */ if (queue.consume) DeleteOrder(pc, old_current); - - return true; } -bool -playlist::PlayPrevious(PlayerControl &pc, Error &error) +void +playlist::PlayPrevious(PlayerControl &pc) { if (!playing) throw PlaylistError::NotPlaying(); @@ -188,12 +187,11 @@ playlist::PlayPrevious(PlayerControl &pc, Error &error) order = current; } - return PlayOrder(pc, order, error); + PlayOrder(pc, order); } -bool -playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time, - Error &error) +void +playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time) { assert(queue.IsValidOrder(i)); @@ -215,20 +213,19 @@ playlist::SeekSongOrder(PlayerControl &pc, unsigned i, SongTime seek_time, queued = -1; - if (!pc.LockSeek(new DetachedSong(queue.GetOrder(i)), seek_time, error)) { + try { + pc.LockSeek(new DetachedSong(queue.GetOrder(i)), seek_time); + } catch (...) { UpdateQueuedSong(pc, queued_song); - return false; + throw; } UpdateQueuedSong(pc, nullptr); - - return true; } -bool +void playlist::SeekSongPosition(PlayerControl &pc, unsigned song, - SongTime seek_time, - Error &error) + SongTime seek_time) { if (!queue.IsValidPosition(song)) throw PlaylistError::BadRange(); @@ -237,24 +234,22 @@ playlist::SeekSongPosition(PlayerControl &pc, unsigned song, ? queue.PositionToOrder(song) : song; - return SeekSongOrder(pc, i, seek_time, error); + SeekSongOrder(pc, i, seek_time); } -bool -playlist::SeekSongId(PlayerControl &pc, unsigned id, SongTime seek_time, - Error &error) +void +playlist::SeekSongId(PlayerControl &pc, unsigned id, SongTime seek_time) { int song = queue.IdToPosition(id); if (song < 0) throw PlaylistError::NoSuchSong(); - return SeekSongPosition(pc, song, seek_time, error); + SeekSongPosition(pc, song, seek_time); } -bool +void playlist::SeekCurrent(PlayerControl &pc, - SignedSongTime seek_time, bool relative, - Error &error) + SignedSongTime seek_time, bool relative) { if (!playing) throw PlaylistError::NotPlaying(); @@ -274,5 +269,5 @@ playlist::SeekCurrent(PlayerControl &pc, if (seek_time.IsNegative()) seek_time = SignedSongTime::zero(); - return SeekSongOrder(pc, current, SongTime(seek_time), error); + SeekSongOrder(pc, current, SongTime(seek_time)); } diff --git a/src/queue/PlaylistEdit.cxx b/src/queue/PlaylistEdit.cxx index 80ab90f17..ccfbe85a6 100644 --- a/src/queue/PlaylistEdit.cxx +++ b/src/queue/PlaylistEdit.cxx @@ -238,8 +238,11 @@ playlist::DeleteInternal(PlayerControl &pc, if (current >= 0 && !paused) /* play the song after the deleted one */ - /* TODO: log error? */ - PlayOrder(pc, current, IgnoreError()); + try { + PlayOrder(pc, current); + } catch (...) { + /* TODO: log error? */ + } else { /* stop the player */ diff --git a/src/queue/PlaylistState.cxx b/src/queue/PlaylistState.cxx index 2e84a4f97..f5cce93a3 100644 --- a/src/queue/PlaylistState.cxx +++ b/src/queue/PlaylistState.cxx @@ -195,12 +195,20 @@ playlist_state_restore(const char *line, TextFile &file, if (state == PlayerState::STOP /* && config_option */) playlist.current = current; - else if (seek_time.count() == 0) - /* TODO: log error? */ - playlist.PlayPosition(pc, current, IgnoreError()); - else - playlist.SeekSongPosition(pc, current, seek_time, - IgnoreError()); + else if (seek_time.count() == 0) { + try { + playlist.PlayPosition(pc, current); + } catch (...) { + /* TODO: log error? */ + } + } else { + try { + playlist.SeekSongPosition(pc, current, + seek_time); + } catch (...) { + /* TODO: log error? */ + } + } if (state == PlayerState::PAUSE) pc.LockPause();