make NEXT on last song work as well, fix potential issues with PREV

feelfreelinux · Oct 6, 2023 · eae302b · eae302b
1 parent 35dd15a
commit eae302b
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 58 deletions.
diff --git 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<uint8_t>& data);
   void notify();
 };

diff --git a/cspot/src/SpircHandler.cpp b/cspot/src/SpircHandler.cpp
@@ -157,12 +157,14 @@ void SpircHandler::handleFrame(std::vector<uint8_t>& 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<TrackPlayer> SpircHandler::getTrackPlayer() {

diff --git 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
@@ -504,11 +504,18 @@ void TrackQueue::processTrack(std::shared_ptr<QueuedTrack> 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<QueuedTrack>(
         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() {