Playlist Library: Improve failed preset handling using a loop

src/playlist/Playlist.cpp

@@ -65,13 +65,22 @@ bool Playlist::AddItem(const std::string& filename, uint32_t index, bool allowDu
         }
     }
 
-    m_presetHistory.clear();
     if (index >= m_items.size())
     {
         m_items.emplace_back(filename);
     }
     else
     {
+        // Increment indices of items equal or grater than the newly added index.
+        for (auto& historyItem : m_presetHistory)
+        {
+            if (historyItem >= index)
+            {
+                historyItem++;
+            }
+        }
+
+
         m_items.emplace(m_items.cbegin() + index, filename);
     }
 
@@ -83,7 +92,6 @@ auto Playlist::AddPath(const std::string& path, uint32_t index, bool recursive,
 {
     uint32_t presetsAdded{0};
 
-    m_presetHistory.clear();
     if (recursive)
     {
         try
@@ -129,6 +137,15 @@ auto Playlist::AddPath(const std::string& path, uint32_t index, bool recursive,
         }
     }
 
+    // Increment indices of items in playback history equal or grater than the newly added index.
+    for (auto& historyItem : m_presetHistory)
+    {
+        if (historyItem >= index)
+        {
+            historyItem += presetsAdded;
+        }
+    }
+
     return presetsAdded;
 }
 
@@ -140,7 +157,23 @@ auto Playlist::RemoveItem(uint32_t index) -> bool
         return false;
     }
 
-    m_presetHistory.clear();
+    // Remove item from history and decrement indices of items after the removed index.
+    for (auto it = begin(m_presetHistory); it != end(m_presetHistory);)
+    {
+        if (*it == index)
+        {
+            it = m_presetHistory.erase(it);
+            continue;
+        }
+
+        if (*it > index)
+        {
+            (*it)--;
+        }
+
+        ++it;
+    }
+
     m_items.erase(m_items.cbegin() + index);
 
     return true;
@@ -333,6 +366,15 @@ void Playlist::RemoveLastHistoryEntry()
 }
 
 
+auto Playlist::HistoryItems() const -> std::vector<uint32_t>
+{
+    std::vector<uint32_t> items;
+    std::copy(begin(m_presetHistory), end(m_presetHistory), std::back_inserter(items));
+
+    return items;
+}
+
+
 auto Playlist::Filter() -> class Filter&
 {
     return m_filter;

src/playlist/Playlist.hpp

@@ -37,7 +37,7 @@ class Playlist
 {
 public:
     /**
-     * Short-hand constant which can be used in AddItem() to add new presets at the end of the playlist.
+     * Shorthand constant which can be used in AddItem() to add new presets at the end of the playlist.
      */
     static constexpr auto InsertAtEnd = std::numeric_limits<uint32_t>::max();
 
@@ -89,21 +89,23 @@ class Playlist
     virtual bool Empty() const;
 
     /**
-     * @brief Clears the current playlist.
+     * @brief Clears the current playlist and playback history.
      */
     virtual void Clear();
 
     /**
      * @brief Returns the playlist items.
      * @return A vector with items in the current playlist.
      */
-    virtual const std::vector<Item>& Items() const;
+    virtual auto Items() const -> const std::vector<Item>&;
 
     /**
      * @brief Adds a preset file to the playlist.
      *
      * Use Playlist::InsertAtEnd as index to always insert an item at the end of the playlist.
      *
+     * The playback history will be kept, and indices are updated accordingly.
+     *
      * @param filename The file path and name to add.
      * @param index The index to insert the preset at. If larger than the playlist size, it's added
      *              to the end of the playlist.
@@ -117,7 +119,9 @@ class Playlist
      * @brief Adds presets (recursively) from the given path.
      *
      * The function will scan the given path (and possible subdirs) for files with a .milk extension
-     * and and the to the playlist, starting at the given index.
+     * and add them to the playlist, starting at the given index.
+     *
+     * The playback history will be kept, and indices are updated accordingly.
      *
      * The order of the added files is unspecified. Use the Sort() method to sort the playlist or
      * the newly added range.
@@ -135,7 +139,8 @@ class Playlist
                          bool allowDuplicates) -> uint32_t;
 
     /**
-     * @brief Removed a playlist item at the given playlist index.
+     * @brief Removes a playlist item at the given playlist index.
+     * The playback history will be kept, and indices are updated accordingly.
      * @param index The index to remove.
      * @return True if an item was removed, false if the index was out of bounds and no item was
      *         removed..
@@ -159,6 +164,8 @@ class Playlist
      *
      * Sorting is case-sensitive.
      *
+     * The playback history is cleared when calling this function.
+     *
      * @param startIndex The index to start sorting at. If the index is larger than the last
      *                   item index, the playlist will remain unchanged.
      * @param count The number of items to sort. If the value exceeds the playlist length, only
@@ -226,6 +233,12 @@ class Playlist
      */
     virtual void RemoveLastHistoryEntry();
 
+    /**
+     * @brief Returns a vector with the playlist indices of the current playback history.
+     * @return A vector of indices with the last played presets.
+     */
+    virtual auto HistoryItems() const -> std::vector<uint32_t>;
+
     /**
      * @brief Returns the current playlist filter list.
      * @return The filter list for the current playlist.

src/playlist/PlaylistCWrapper.cpp

@@ -62,45 +62,18 @@ void PlaylistCWrapper::OnPresetSwitchFailed(const char* presetFilename, const ch
     }
 
     auto* playlist = reinterpret_cast<PlaylistCWrapper*>(userData);
+
+    playlist->m_lastPresetSwitchFailed = true;
+    playlist->m_lastFailedPresetFileName = presetFilename;
+    playlist->m_lastFailedPresetError = message;
+
     auto lastDirection = playlist->GetLastNavigationDirection();
 
     if (lastDirection != NavigationDirection::Last)
     {
         // Don't let the user go back to a broken preset.
         playlist->RemoveLastHistoryEntry();
     }
-
-    // Preset switch may fail due to broken presets, retry a few times before giving up.
-    if (playlist->m_presetSwitchFailedCount >= playlist->m_presetSwitchRetryCount)
-    {
-        if (playlist->m_presetSwitchFailedEventCallback != nullptr)
-        {
-            playlist->m_presetSwitchFailedEventCallback(presetFilename, message,
-                                                        playlist->m_presetSwitchFailedEventUserData);
-        }
-
-        return;
-    }
-
-    playlist->m_presetSwitchFailedCount++;
-
-    uint32_t playlistIndex{};
-    switch (lastDirection)
-    {
-        case NavigationDirection::Previous:
-            playlistIndex = playlist->PreviousPresetIndex();
-            break;
-
-        case NavigationDirection::Next:
-            playlistIndex = playlist->NextPresetIndex();
-            break;
-
-        case NavigationDirection::Last:
-            playlistIndex = playlist->LastPresetIndex();
-            break;
-    }
-
-    playlist->PlayPresetIndex(playlistIndex, playlist->m_hardCutRequested, false);
 }
 
 
@@ -132,22 +105,64 @@ void PlaylistCWrapper::SetPresetSwitchFailedCallback(projectm_playlist_preset_sw
 
 void PlaylistCWrapper::PlayPresetIndex(uint32_t index, bool hardCut, bool resetFailureCount)
 {
-    if (resetFailureCount)
-    {
-        m_presetSwitchFailedCount = 0;
-    }
-
     m_hardCutRequested = hardCut;
 
-    const auto& playlistItems = Items();
+    auto& playlistItems = Items();
 
-    if (playlistItems.size() <= index)
+    uint32_t failedCount = 0;
+    while (true)
     {
-        return;
-    }
+        if (playlistItems.size() <= index)
+        {
+            return;
+        }
+
+        projectm_load_preset_file(m_projectMInstance,
+                                  playlistItems.at(index).Filename().c_str(), !hardCut);
 
-    projectm_load_preset_file(m_projectMInstance,
-                              playlistItems.at(index).Filename().c_str(), !hardCut);
+        if (!m_lastPresetSwitchFailed)
+        {
+            break;
+        }
+
+        failedCount++;
+
+        if (failedCount >= m_presetSwitchRetryCount)
+        {
+            if (m_presetSwitchFailedEventCallback != nullptr)
+            {
+                m_presetSwitchFailedEventCallback(m_lastFailedPresetFileName.c_str(),
+                                                  m_lastFailedPresetError.c_str(),
+                                                  m_presetSwitchFailedEventUserData);
+            }
+
+            return;
+        }
+
+        m_lastPresetSwitchFailed = false;
+
+        // Remove failed preset from playlist
+        RemoveItem(index);
+
+        if (playlistItems.empty())
+        {
+            return;
+        }
+
+        // Set next index to proper value depending on navigation
+        switch (GetLastNavigationDirection())
+        {
+            case NavigationDirection::Last:
+                index = LastPresetIndex();
+                break;
+            case NavigationDirection::Next:
+                index = NextPresetIndex();
+                break;
+            case NavigationDirection::Previous:
+                index = PreviousPresetIndex();
+                break;
+        }
+    }
 
     if (m_presetSwitchedEventCallback != nullptr)
     {
@@ -156,13 +171,13 @@ void PlaylistCWrapper::PlayPresetIndex(uint32_t index, bool hardCut, bool resetF
 }
 
 
-void PlaylistCWrapper::SetLastNavigationDirection(PlaylistCWrapper::NavigationDirection direction)
+void PlaylistCWrapper::SetLastNavigationDirection(NavigationDirection direction)
 {
     m_lastNavigationDirection = direction;
 }
 
 
-auto PlaylistCWrapper::GetLastNavigationDirection() const -> PlaylistCWrapper::NavigationDirection
+auto PlaylistCWrapper::GetLastNavigationDirection() const -> NavigationDirection
 {
     return m_lastNavigationDirection;
 }
@@ -263,7 +278,7 @@ auto projectm_playlist_items(projectm_playlist_handle instance, uint32_t start,
 
     if (start >= items.size())
     {
-        auto* array = new char* [1] {};
+        auto* array = new char*[1]{};
         return array;
     }
 

src/playlist/PlaylistCWrapper.hpp

@@ -98,8 +98,10 @@ class PlaylistCWrapper : public Playlist
 private:
     projectm_handle m_projectMInstance{nullptr}; //!< The projectM instance handle this instance is connected to.
 
-    uint32_t m_presetSwitchRetryCount{5};  //!< Number of switch retries before sending the failure event to the application.
-    uint32_t m_presetSwitchFailedCount{0}; //!< Number of retries since the last preset switch.
+    uint32_t m_presetSwitchRetryCount{500}; //!< Number of switch retries before sending the failure event to the application.
+    bool m_lastPresetSwitchFailed{false};   //!< Indicates that the last preset switch has failed.
+    std::string m_lastFailedPresetFileName; //!< File name of the last failed preset.
+    std::string m_lastFailedPresetError;    //!< Error message of the last failure.
 
     bool m_hardCutRequested{false}; //!< Stores the type of the last requested switch attempt.
 

src/playlist/api/projectM-4/playlist_playback.h

@@ -28,8 +28,8 @@
 
 #include "projectM-4/playlist_types.h"
 
-#include <stdint.h>
 #include <stdbool.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -53,9 +53,10 @@ PROJECTM_PLAYLIST_EXPORT bool projectm_playlist_get_shuffle(projectm_playlist_ha
 
 /**
  * @brief Sets the number of retries after failed preset switches.
- * @note Don't set this value too high, as each retry is done recursively.
+ * @note Retry behavior changed in v4.2, using a loop. Default retry count is now 500. Failed items
+ *       are also being removed from the playlist, so they're not tried again.
  * @param instance The playlist manager instance.
- * @param retry_count The number of retries after failed preset switches. Default is 5. Set to 0
+ * @param retry_count The number of retries after failed preset switches. Default is 500. Set to 0
  *                    to simply forward the failure event from projectM.
  * @since 4.0.0
  */