From eae302b355dc5884b73e5d97eb0e9833fc0744ab Mon Sep 17 00:00:00 2001 From: philippe44 Date: Thu, 5 Oct 2023 22:02:44 -0700 Subject: [PATCH] make NEXT on last song work as well, fix potential issues with PREV --- cspot/include/SpircHandler.h | 6 +-- cspot/src/SpircHandler.cpp | 38 +++++++++--------- cspot/src/TrackPlayer.cpp | 2 +- cspot/src/TrackQueue.cpp | 75 +++++++++++++++++++----------------- 4 files changed, 63 insertions(+), 58 deletions(-) diff --git a/cspot/include/SpircHandler.h b/cspot/include/SpircHandler.h index 7a454536..33727608 100644 --- a/cspot/include/SpircHandler.h +++ b/cspot/include/SpircHandler.h @@ -48,9 +48,9 @@ class SpircHandler { void setPause(bool pause); - void previousSong(); + bool previousSong(); - void nextSong(); + bool nextSong(); void notifyAudioReachedPlayback(); void updatePositionMs(uint32_t position); @@ -74,7 +74,7 @@ class SpircHandler { void sendEvent(EventType type); void sendEvent(EventType type, EventData data); - void skipSong(TrackQueue::SkipDirection dir); + bool skipSong(TrackQueue::SkipDirection dir); void handleFrame(std::vector& data); void notify(); }; diff --git a/cspot/src/SpircHandler.cpp b/cspot/src/SpircHandler.cpp index 52032aed..771c537b 100644 --- a/cspot/src/SpircHandler.cpp +++ b/cspot/src/SpircHandler.cpp @@ -157,12 +157,14 @@ void SpircHandler::handleFrame(std::vector& data) { setPause(false); break; case MessageType_kMessageTypeNext: - nextSong(); - sendEvent(EventType::NEXT); + if (nextSong()) { + sendEvent(EventType::NEXT); + } break; case MessageType_kMessageTypePrev: - previousSong(); - sendEvent(EventType::PREV); + if (previousSong()) { + sendEvent(EventType::PREV); + } break; case MessageType_kMessageTypeLoad: { this->trackPlayer->start(); @@ -227,34 +229,32 @@ void SpircHandler::notify() { this->sendCmd(MessageType_kMessageTypeNotify); } -void SpircHandler::skipSong(TrackQueue::SkipDirection dir) { +bool SpircHandler::skipSong(TrackQueue::SkipDirection dir) { if (trackQueue->skipTrack(dir)) { - playbackState->setPlaybackState(PlaybackState::State::Playing); - notify(); - + // flush first to clean sink + sendEvent(EventType::FLUSH); + // Reset track state trackPlayer->resetState(); - sendEvent(EventType::PLAY_PAUSE, false); + return true; } else { + // can't skip, just pause where we are + sendEvent(EventType::PLAY_PAUSE, true); + playbackState->setPlaybackState(PlaybackState::State::Paused); - playbackState->updatePositionMs(0); notify(); - sendEvent(EventType::PLAY_PAUSE, true); + return false; } - - notify(); - - sendEvent(EventType::FLUSH); } -void SpircHandler::nextSong() { - skipSong(TrackQueue::SkipDirection::NEXT); +bool SpircHandler::nextSong() { + return skipSong(TrackQueue::SkipDirection::NEXT); } -void SpircHandler::previousSong() { - skipSong(TrackQueue::SkipDirection::PREV); +bool SpircHandler::previousSong() { + return skipSong(TrackQueue::SkipDirection::PREV); } std::shared_ptr SpircHandler::getTrackPlayer() { diff --git a/cspot/src/TrackPlayer.cpp b/cspot/src/TrackPlayer.cpp index d310caa5..ee51e14d 100644 --- a/cspot/src/TrackPlayer.cpp +++ b/cspot/src/TrackPlayer.cpp @@ -119,7 +119,7 @@ void TrackPlayer::runTask() { while (isRunning) { // Ensure we even have any tracks to play if (!this->trackQueue->hasTracks() || - (endOfQueueReached && trackQueue->isFinished())) { + (!pendingReset && endOfQueueReached && trackQueue->isFinished())) { this->trackQueue->playableSemaphore->twait(300); continue; } diff --git a/cspot/src/TrackQueue.cpp b/cspot/src/TrackQueue.cpp index e1d3584b..4369f3cf 100644 --- a/cspot/src/TrackQueue.cpp +++ b/cspot/src/TrackQueue.cpp @@ -504,11 +504,18 @@ void TrackQueue::processTrack(std::shared_ptr track) { bool TrackQueue::queueNextTrack(int offset, uint32_t positionMs) { const int requestedRefIndex = offset + currentTracksIndex; + if (requestedRefIndex < 0 || requestedRefIndex >= currentTracks.size()) { return false; } - if (offset < 0) { + // in case we re-queue current track, make sure position is updated (0) + if (offset == 0 && preloadedTracks.size() && + preloadedTracks[0]->ref == currentTracks[currentTracksIndex]) { + preloadedTracks.pop_front(); + } + + if (offset <= 0) { preloadedTracks.push_front(std::make_shared( currentTracks[requestedRefIndex], ctx, positionMs)); } else { @@ -520,53 +527,51 @@ bool TrackQueue::queueNextTrack(int offset, uint32_t positionMs) { } bool TrackQueue::skipTrack(SkipDirection dir, bool expectNotify) { - bool canSkipNext = currentTracks.size() > currentTracksIndex + 1; - bool canSkipPrev = currentTracksIndex > 0; - uint64_t position = !playbackState->innerFrame.state.has_position_ms ? 0 : - playbackState->innerFrame.state.position_ms + - ctx->timeProvider->getSyncedTimestamp() - - playbackState->innerFrame.state.position_measured_at; + bool skipped = true; + std::scoped_lock lock(tracksMutex); - if (dir == SkipDirection::PREV && (currentTracksIndex == 0 || position > 3000)) { - queueNextTrack(0); + if (dir == SkipDirection::PREV) { + uint64_t position = !playbackState->innerFrame.state.has_position_ms ? 0 : + playbackState->innerFrame.state.position_ms + + ctx->timeProvider->getSyncedTimestamp() - + playbackState->innerFrame.state.position_measured_at; - // Reset position to zero (in that case it's always expected) - notifyPending = true; + if (currentTracksIndex > 0 && position < 3000) { + queueNextTrack(-1); - return true; - } else if ((dir == SkipDirection::NEXT && canSkipNext) || - (dir == SkipDirection::PREV && canSkipPrev)) { - std::scoped_lock lock(tracksMutex); - if (dir == SkipDirection::NEXT) { - preloadedTracks.pop_front(); + if (preloadedTracks.size() > MAX_TRACKS_PRELOAD) { + preloadedTracks.pop_back(); + } - if (!queueNextTrack(preloadedTracks.size() + 1)) { - CSPOT_LOG(info, "Failed to queue next track"); + currentTracksIndex--; + } else { + queueNextTrack(0); } - - currentTracksIndex++; } else { - queueNextTrack(-1); + if (currentTracks.size() > currentTracksIndex + 1) { + preloadedTracks.pop_front(); - if (preloadedTracks.size() > MAX_TRACKS_PRELOAD) { - preloadedTracks.pop_back(); - } + if (!queueNextTrack(preloadedTracks.size() + 1)) { + CSPOT_LOG(info, "Failed to queue next track"); + } - currentTracksIndex--; + currentTracksIndex++; + } else { + skipped = false; + } } - // Update frame data - playbackState->innerFrame.state.playing_track_index = currentTracksIndex; + if (skipped) { + // Update frame data + playbackState->innerFrame.state.playing_track_index = currentTracksIndex; - if (expectNotify) { - // Reset position to zero - notifyPending = true; + if (expectNotify) { + // Reset position to zero + notifyPending = true; + } } - return true; - } - - return false; + return skipped; } bool TrackQueue::hasTracks() {