diff --git a/src/playlist/Playlist.cpp b/src/playlist/Playlist.cpp index 0eef28e22a..e49bc8457f 100644 --- a/src/playlist/Playlist.cpp +++ b/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 +{ + std::vector items; + std::copy(begin(m_presetHistory), end(m_presetHistory), std::back_inserter(items)); + + return items; +} + + auto Playlist::Filter() -> class Filter& { return m_filter; diff --git a/src/playlist/Playlist.hpp b/src/playlist/Playlist.hpp index 393e88a8a2..eb974f1062 100644 --- a/src/playlist/Playlist.hpp +++ b/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::max(); @@ -89,7 +89,7 @@ class Playlist virtual bool Empty() const; /** - * @brief Clears the current playlist. + * @brief Clears the current playlist and playback history. */ virtual void Clear(); @@ -97,13 +97,15 @@ class Playlist * @brief Returns the playlist items. * @return A vector with items in the current playlist. */ - virtual const std::vector& Items() const; + virtual auto Items() const -> const std::vector&; /** * @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; + /** * @brief Returns the current playlist filter list. * @return The filter list for the current playlist. diff --git a/src/playlist/PlaylistCWrapper.cpp b/src/playlist/PlaylistCWrapper.cpp index 3c2e6be6e9..5a368c31a3 100644 --- a/src/playlist/PlaylistCWrapper.cpp +++ b/src/playlist/PlaylistCWrapper.cpp @@ -62,6 +62,11 @@ void PlaylistCWrapper::OnPresetSwitchFailed(const char* presetFilename, const ch } auto* playlist = reinterpret_cast(userData); + + playlist->m_lastPresetSwitchFailed = true; + playlist->m_lastFailedPresetFileName = presetFilename; + playlist->m_lastFailedPresetError = message; + auto lastDirection = playlist->GetLastNavigationDirection(); if (lastDirection != NavigationDirection::Last) @@ -69,38 +74,6 @@ void PlaylistCWrapper::OnPresetSwitchFailed(const char* presetFilename, const ch // 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; } diff --git a/src/playlist/PlaylistCWrapper.hpp b/src/playlist/PlaylistCWrapper.hpp index 20d315f073..125e22b830 100644 --- a/src/playlist/PlaylistCWrapper.hpp +++ b/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. diff --git a/src/playlist/api/projectM-4/playlist_playback.h b/src/playlist/api/projectM-4/playlist_playback.h index 2d63eef7cd..f3b0861428 100644 --- a/src/playlist/api/projectM-4/playlist_playback.h +++ b/src/playlist/api/projectM-4/playlist_playback.h @@ -28,8 +28,8 @@ #include "projectM-4/playlist_types.h" -#include #include +#include #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 */ diff --git a/tests/playlist/PlaylistTest.cpp b/tests/playlist/PlaylistTest.cpp index 7ae4ff335d..874004adff 100644 --- a/tests/playlist/PlaylistTest.cpp +++ b/tests/playlist/PlaylistTest.cpp @@ -153,6 +153,38 @@ TEST(projectMPlaylistPlaylist, AddItemNoDuplicates) } +TEST(projectMPlaylistPlaylist, AddItemWithHistory) +{ + Playlist playlist; + EXPECT_TRUE(playlist.AddItem("/some/file", 0, false)); + EXPECT_TRUE(playlist.AddItem("/some/other/file", Playlist::InsertAtEnd, false)); + EXPECT_TRUE(playlist.AddItem("/and/another/file", Playlist::InsertAtEnd, false)); + + ASSERT_EQ(playlist.Size(), 3); + + playlist.SetPresetIndex(1); + playlist.SetPresetIndex(2); + playlist.SetPresetIndex(0); + + auto historyItemsBefore = playlist.HistoryItems(); + ASSERT_EQ(historyItemsBefore.size(), 3); + EXPECT_EQ(historyItemsBefore.at(0), 0); // Playback started with index 0 + EXPECT_EQ(historyItemsBefore.at(1), 1); + EXPECT_EQ(historyItemsBefore.at(2), 2); + + EXPECT_TRUE(playlist.AddItem("/yet/another/file", 1, false)); + + ASSERT_EQ(playlist.Size(), 4); + + auto historyItemsAfter = playlist.HistoryItems(); + ASSERT_EQ(historyItemsAfter.size(), 3); + EXPECT_EQ(historyItemsAfter.at(0), 0); + EXPECT_EQ(historyItemsAfter.at(1), 2); + EXPECT_EQ(historyItemsAfter.at(2), 3); +} + + + TEST(projectMPlaylistPlaylist, AddPathRecursively) { Playlist playlist; @@ -218,7 +250,7 @@ TEST(projectMPlaylistPlaylist, AddPathNonRecursively) } -TEST(projectMPlaylistPlaylist, AddPathnonRecursivelyNoDuplicates) +TEST(projectMPlaylistPlaylist, AddPathNonRecursivelyNoDuplicates) { Playlist playlist; @@ -289,6 +321,36 @@ TEST(projectMPlaylistPlaylist, RemoveItemFromMiddle) } +TEST(projectMPlaylistPlaylist, RemoveItemWithHistory) +{ + Playlist playlist; + EXPECT_TRUE(playlist.AddItem("/some/file", Playlist::InsertAtEnd, false)); + EXPECT_TRUE(playlist.AddItem("/some/other/file", Playlist::InsertAtEnd, false)); + EXPECT_TRUE(playlist.AddItem("/yet/another/file", Playlist::InsertAtEnd, false)); + + ASSERT_EQ(playlist.Size(), 3); + + playlist.SetPresetIndex(1); + playlist.SetPresetIndex(2); + playlist.SetPresetIndex(0); + + auto historyItemsBefore = playlist.HistoryItems(); + ASSERT_EQ(historyItemsBefore.size(), 3); + EXPECT_EQ(historyItemsBefore.at(0), 0); // Playback started with index 0 + EXPECT_EQ(historyItemsBefore.at(1), 1); + EXPECT_EQ(historyItemsBefore.at(2), 2); + + EXPECT_TRUE(playlist.RemoveItem(1)); + + ASSERT_EQ(playlist.Size(), 2); + + auto historyItemsAfter = playlist.HistoryItems(); + ASSERT_EQ(historyItemsAfter.size(), 2); + EXPECT_EQ(historyItemsAfter.at(0), 0); + EXPECT_EQ(historyItemsAfter.at(1), 1); +} + + TEST(projectMPlaylistPlaylist, RemoveItemIndexOutOfBounds) { Playlist playlist;