From 13c30b0d5b1feb81bd11ca65c3c60fad0fcd5457 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Sun, 23 Apr 2023 13:07:53 +0300 Subject: [PATCH 1/7] Add Playback History Qt-only plugin Copy and modify the playlist-manager-qt plugin code. A similar feature was requested in #553. However, this initial version of the playback history plugin tracks and displays only album history, not individual song/track history. Playback start/stop time is not tracked or shown either. --- configure.ac | 3 +- meson.build | 1 + src/meson.build | 1 + src/playback-history/Makefile | 13 + src/playback-history/meson.build | 7 + src/playback-history/playback-history.cc | 519 +++++++++++++++++++++++ src/qtui/main_window.cc | 14 +- src/qtui/main_window.h | 11 +- src/qtui/menus.cc | 7 + src/skins-qt/actions-mainwin.h | 1 + src/skins-qt/actions.cc | 10 + src/skins-qt/menus.cc | 1 + 12 files changed, 581 insertions(+), 7 deletions(-) create mode 100644 src/playback-history/Makefile create mode 100644 src/playback-history/meson.build create mode 100644 src/playback-history/playback-history.cc diff --git a/configure.ac b/configure.ac index 83096842b5..06f9db46ff 100644 --- a/configure.ac +++ b/configure.ac @@ -89,7 +89,7 @@ if test "x$USE_GTK" = "xyes" ; then fi if test "x$USE_QT" = "xyes" ; then - GENERAL_PLUGINS="$GENERAL_PLUGINS albumart-qt lyrics-qt playlist-manager-qt search-tool-qt song-info-qt statusicon-qt" + GENERAL_PLUGINS="$GENERAL_PLUGINS albumart-qt lyrics-qt playback-history playlist-manager-qt search-tool-qt song-info-qt statusicon-qt" GENERAL_PLUGINS="$GENERAL_PLUGINS qtui skins-qt" VISUALIZATION_PLUGINS="$VISUALIZATION_PLUGINS blur_scope-qt qt-spectrum vumeter-qt" fi @@ -887,6 +887,7 @@ if test "x$USE_QT" = "xyes" ; then echo " Album Art: yes" echo " Blur Scope: yes" echo " OpenGL Spectrum Analyzer: $have_qtglspectrum" + echo " Playback History: yes" echo " Playlist Manager: yes" echo " Search Tool: yes" echo " Song Info: yes" diff --git a/meson.build b/meson.build index 0450b0b17f..5589352d1b 100644 --- a/meson.build +++ b/meson.build @@ -309,6 +309,7 @@ if meson.version().version_compare('>= 0.53') 'Album Art': true, 'Blur Scope': true, 'OpenGL Spectrum Analyzer': get_variable('have_qtglspectrum', false), + 'Playback History': true, 'Playlist Manager': true, 'Search Tool': true, 'Song Info': true, diff --git a/src/meson.build b/src/meson.build index 50b76626bf..4ef72ca66e 100644 --- a/src/meson.build +++ b/src/meson.build @@ -97,6 +97,7 @@ if conf.has('USE_QT') subdir('albumart-qt') subdir('blur_scope-qt') subdir('lyrics-qt') + subdir('playback-history') subdir('playlist-manager-qt') subdir('qt-spectrum') subdir('qtui') diff --git a/src/playback-history/Makefile b/src/playback-history/Makefile new file mode 100644 index 0000000000..9574129bf3 --- /dev/null +++ b/src/playback-history/Makefile @@ -0,0 +1,13 @@ +PLUGIN = playback-history${PLUGIN_SUFFIX} + +SRCS = playback-history.cc + +include ../../buildsys.mk +include ../../extra.mk + +plugindir := ${plugindir}/${GENERAL_PLUGIN_DIR} + +LD = ${CXX} +CPPFLAGS += -I../.. ${QT_CFLAGS} +CFLAGS += ${PLUGIN_CFLAGS} +LIBS += ${QT_LIBS} -laudqt diff --git a/src/playback-history/meson.build b/src/playback-history/meson.build new file mode 100644 index 0000000000..7a587e33a4 --- /dev/null +++ b/src/playback-history/meson.build @@ -0,0 +1,7 @@ +shared_module('playback-history', + 'playback-history.cc', + dependencies: [audacious_dep, qt_dep, audqt_dep], + name_prefix: '', + install: true, + install_dir: general_plugin_dir +) diff --git a/src/playback-history/playback-history.cc b/src/playback-history/playback-history.cc new file mode 100644 index 0000000000..3326513205 --- /dev/null +++ b/src/playback-history/playback-history.cc @@ -0,0 +1,519 @@ +/* + * playback-history.cc + * Copyright (C) 2023 Igor Kushnir + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, + * or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +/** + * Returns a representation of @p str suitable for printing via audlog::log(). + */ +static const char * printable(const String & str) +{ + // Printing nullptr invokes undefined behavior. + return str ? str : ""; +} + +static QString toQString(const String & str) +{ + // The explicit cast to const char * is necessary to compile with Qt 6, + // because otherwise two implicit conversions would be necessary: from + // String to const char * and from const char * to QByteArrayView. + return QString::fromUtf8(static_cast(str)); +} + +class PlaybackHistory : public GeneralPlugin +{ +private: + static constexpr const char * aboutText = + N_("Playback History Plugin\n\n" + "Copyright 2023 Igor Kushnir \n\n" + "This plugin tracks and provides access to playback history.\n\n" + + "History entries are stored only in memory and are lost\n" + "on Audacious exit. When the plugin is disabled,\n" + "playback history is not tracked at all.\n" + "History entries are only added, never removed.\n" + "The user can restart Audacious or close Playback History\n" + "view to disable the plugin and clear the entries.\n\n" + + "Currently the playback history is actually an album history,\n" + "designed and tailored for the users of Shuffle by Album mode.\n" + "Extending the plugin to optionally track and display\n" + "individual song history could be a useful future development.\n" + "Feel free to contribute this feature."); + +public: + static constexpr PluginInfo info = {N_("Playback History"), PACKAGE, + aboutText, + nullptr, // prefs + PluginQtOnly}; + + constexpr PlaybackHistory() : GeneralPlugin(info, false) {} + + void * get_qt_widget() override; + int take_message(const char * code, const void *, int) override; +}; + +EXPORT PlaybackHistory aud_plugin_instance; + +class HistoryEntry +{ +public: + /** + * Creates an invalid entry that can only be assigned to or destroyed. + */ + HistoryEntry() = default; + + /** + * Stores the currently playing entry in @c this. + * + * Call this function when a song playback starts. + * @return @c true if the entry was retrieved and assigned successfully. + * @note @c this remains or becomes invalid if @c false is returned. + */ + bool assignPlayingEntry(); + + /** + * Starts playing this entry. + * + * @return @c true in case of success. + */ + bool play() const; + + /** + * Prints @c this using @c AUDDBG. + * + * @param prefix a possibly empty string to be printed before @c this. + * A nonempty @p prefix should end with whitespace. + */ + void debugPrint(const char * prefix) const; + + const String & album() const { return m_album; } + String playlistTitle() const { return m_playlist.get_title(); } + /** Returns the playlist Entry Number of this entry. */ + int entryNumber() const; + +private: + /** + * Retrieves the album at @a m_playlistPosition in @a m_playlist and assigns + * it to @p album. + * + * @return @c true if the album was retrieved successfully. + */ + bool retrieveAlbum(String & album) const; + + String m_album; + Playlist m_playlist; /**< the playlist, in which this entry was played */ + /** The position in @a m_playlist of the first played song from @a m_album. + * When the user modifies the playlist, positions shift, but this should + * happen rarely enough and therefore doesn't have to be handled perfectly. + * A linear search for @a m_album in @a m_playlist is inefficient, + * and thus is never performed by this plugin. */ + int m_playlistPosition = -1; +}; + +class HistoryModel : public QAbstractListModel +{ +public: + /** + * Updates a cached font. + * + * Pass the view's font to this function when the view is created and + * whenever its font changes. + */ + void setFont(const QFont & font); + + /** + * Plays the song of the item at @p index. + * + * Call this function when the user activates the item at @p index in the + * view. + */ + void activate(const QModelIndex & index); + + int rowCount(const QModelIndex & parent) const override; + int columnCount(const QModelIndex & parent) const override; + QVariant data(const QModelIndex & index, int role) const override; + +private: + enum + { + /** The column name is general, because a song title might optionally be + * displayed in place of an album title in the future. */ + ColumnText, + NColumns + }; + + // In this class the word "position" refers to an index into m_entries. + + bool isOutOfBounds(const QModelIndex & index) const; + int modelRowFromPosition(int position) const; + int positionFromIndex(const QModelIndex & index) const; + + /** + * Updates the font of the item that corresponds to @p position. + * + * Call this function twice right after modifying @a m_playingPosition: pass + * the previous and the current value of @a m_playingPosition to it. + */ + void updateFontForPosition(int position); + + /** + * Adds the newly played song into the playback history. + */ + void playbackStarted(); + + const HookReceiver activate_hook{ + "playback ready", this, &HistoryModel::playbackStarted}; + + Index m_entries; + /** the position of the entry that is + * currently playing or was played last */ + int m_playingPosition = -1; + /** a cached font used to highlight the item that is currently playing */ + QFont m_currentlyPlaingFont; +}; + +class HistoryView : public audqt::TreeView +{ +public: + HistoryView(); + +protected: + void changeEvent(QEvent * event) override; + +private: + HistoryModel m_model; +}; + +bool HistoryEntry::assignPlayingEntry() +{ + m_playlist = Playlist::playing_playlist(); + if (!m_playlist.exists()) + { + AUDWARN("Playback just started but no playlist is playing.\n"); + return false; + } + + m_playlistPosition = m_playlist.get_position(); + if (m_playlistPosition == -1) + { + AUDWARN("Playback just started but the playing playlist %s " + "has no playing entry.\n", + printable(playlistTitle())); + return false; + } + assert(m_playlistPosition >= 0); + assert(m_playlistPosition < m_playlist.n_entries()); + + return retrieveAlbum(m_album); +} + +bool HistoryEntry::play() const +{ + if (!m_playlist.exists()) + { + AUDWARN("The activated entry's playlist has been deleted.\n"); + return false; + } + + assert(m_playlistPosition >= 0); + if (m_playlistPosition >= m_playlist.n_entries()) + { + AUDWARN("The activated entry's position is now out of bounds.\n"); + return false; + } + + String currentAlbumAtPlaylistPosition; + if (!retrieveAlbum(currentAlbumAtPlaylistPosition)) + return false; + + // This check does not guarantee that the first played song from m_album + // still resides at m_playlistPosition in m_playlist. In case the user + // inserts or removes a few songs above m_playlistPosition, a different song + // from the same album or a song from an unrelated album that happens to + // have the same name goes undetected. But such coincidences should be much + // more rare and less of a problem than the album inequality condition + // checked here. Therefore, information that uniquely identifies the + // referenced song is not stored in a history entry just for this case. + if (currentAlbumAtPlaylistPosition != m_album) + { + AUDWARN("The album at the activated entry's playlist position has" + " changed.\n"); + return false; + } + + m_playlist.set_position(m_playlistPosition); + m_playlist.start_playback(); + m_playlist.activate(); + return true; +} + +void HistoryEntry::debugPrint(const char * prefix) const +{ + AUDDBG("%salbum=\"%s\", playlist=\"%s\", entry number=%d\n", prefix, + printable(m_album), printable(playlistTitle()), entryNumber()); +} + +int HistoryEntry::entryNumber() const +{ + // Add 1 because a playlist position is 0-based + // but an Entry Number in the UI is 1-based. + return m_playlistPosition + 1; +} + +bool HistoryEntry::retrieveAlbum(String & album) const +{ + String errorMessage; + const auto tuple = m_playlist.entry_tuple(m_playlistPosition, + Playlist::Wait, &errorMessage); + if (errorMessage || tuple.state() != Tuple::Valid) + { + AUDWARN("Failed to retrieve metadata of entry #%d in playlist %s: %s\n", + entryNumber(), printable(playlistTitle()), + errorMessage ? printable(errorMessage) + : "Song info could not be read"); + return false; + } + + album = tuple.get_str(Tuple::Album); + return true; +} + +void HistoryModel::setFont(const QFont & font) +{ + m_currentlyPlaingFont = font; + m_currentlyPlaingFont.setBold(true); + + if (m_playingPosition >= 0) + updateFontForPosition(m_playingPosition); +} + +void HistoryModel::activate(const QModelIndex & index) +{ + if (isOutOfBounds(index)) + return; + const int pos = positionFromIndex(index); + + // The "playback ready" hook is activated asynchronously, so + // playbackStarted() for the playback initiated here will be invoked after + // this function updates m_playingPosition and returns. + if (!m_entries[pos].play()) + return; + + // Update m_playingPosition here to prevent the imminent playbackStarted() + // invocation from appending a copy of the activated entry to m_entries. + if (pos != m_playingPosition) + { + const int prevPlayingPosition = m_playingPosition; + m_playingPosition = pos; + if (prevPlayingPosition >= 0) + updateFontForPosition(prevPlayingPosition); + updateFontForPosition(m_playingPosition); + } +} + +int HistoryModel::rowCount(const QModelIndex & parent) const +{ + return parent.isValid() ? 0 : m_entries.len(); +} + +int HistoryModel::columnCount(const QModelIndex & parent) const +{ + return parent.isValid() ? 0 : NColumns; +} + +QVariant HistoryModel::data(const QModelIndex & index, int role) const +{ + if (isOutOfBounds(index)) + return QVariant(); + const int pos = positionFromIndex(index); + + switch (role) + { + case Qt::DisplayRole: + return toQString(m_entries[pos].album()); + case Qt::ToolTipRole: + { + const auto & entry = m_entries[pos]; + // The playlist title and entry number are rarely interesting and + // therefore shown only in the tooltip. + return QString::fromUtf8(_("Album: %1
Playlist: %2" + "
Entry Number: %3")) + .arg(toQString(entry.album()), toQString(entry.playlistTitle()), + QString::number(entry.entryNumber())); + } + case Qt::FontRole: + if (pos == m_playingPosition) + return m_currentlyPlaingFont; + break; + } + + return QVariant(); +} + +bool HistoryModel::isOutOfBounds(const QModelIndex & index) const +{ + if (!index.isValid()) + { + AUDWARN("Invalid index.\n"); + return true; + } + if (index.row() >= m_entries.len()) + { + AUDWARN("Index row is out of bounds: %d >= %d\n", index.row(), + m_entries.len()); + return true; + } + return false; +} + +int HistoryModel::modelRowFromPosition(int position) const +{ + assert(position >= 0); + assert(position < m_entries.len()); + // Reverse the order of entries here in order to: + // 1) display most recently played entries at the top of the view and thus + // avoid scrolling to the bottom of the view each time a new entry is added; + // 2) efficiently append new elements to m_entries. + // This code must be kept in sync with playbackStarted(). + return m_entries.len() - 1 - position; +} + +int HistoryModel::positionFromIndex(const QModelIndex & index) const +{ + assert(!isOutOfBounds(index)); + // modelRowFromPosition() is an involution (self-inverse function), + // and thus this inverse function delegates to it. + return modelRowFromPosition(index.row()); +} + +void HistoryModel::updateFontForPosition(int position) +{ + const int row = modelRowFromPosition(position); + const auto topLeft = createIndex(row, 0); + // Optimize for the current single-column implementation, but also support + // NColumns > 1 to prevent a bug here if a column is added in the future. + if constexpr (NColumns == 1) + emit dataChanged(topLeft, topLeft, {Qt::FontRole}); + else + { + assert(NColumns > 1); + const auto bottomRight = createIndex(row, NColumns - 1); + emit dataChanged(topLeft, bottomRight, {Qt::FontRole}); + } +} + +void HistoryModel::playbackStarted() +{ + HistoryEntry entry; + if (!entry.assignPlayingEntry()) + return; + + entry.debugPrint("Started playing "); + AUDDBG("playing position=%d, entry count=%d\n", m_playingPosition, + m_entries.len()); + + if (m_playingPosition >= 0 && + m_entries[m_playingPosition].album() == entry.album()) + { + // In all probability, a song from the same album started playing, in + // which case there is nothing to do. Much less likely, a different + // album with the same name, separated by other albums from the + // previously played one, was just randomly selected. + // Ignore this possibility here. The users concerned about such an + // occurrence are advised to edit metadata in their music collections + // and ensure unique album names. The unique album names would also + // prevent Audacious from erroneously playing same-name albums + // in order if they happen to end up adjacent in a playlist. + return; + } + + const int prevPlayingPosition = m_playingPosition; + + // The last played entry appears at the top of the view. Therefore, the new + // entry is inserted at row 0. + // This code must be kept in sync with modelRowFromPosition(). + beginInsertRows(QModelIndex(), 0, 0); + // Update m_playingPosition during the row insertion to avoid + // updating the font for the new playing position separately below. + m_playingPosition = m_entries.len(); + m_entries.append(std::move(entry)); + endInsertRows(); + + if (prevPlayingPosition >= 0) + updateFontForPosition(prevPlayingPosition); +} + +HistoryView::HistoryView() +{ + AUDDBG("creating\n"); + + // Hide the header to save vertical space. A single-column caption is not + // very useful, because the user can learn the meaning of an item's text + // by examining its tooltip. + setHeaderHidden(true); + + setAllColumnsShowFocus(true); + setFrameShape(QFrame::NoFrame); + setIndentation(0); + + m_model.setFont(font()); + setModel(&m_model); + connect(this, &QTreeView::activated, &m_model, &HistoryModel::activate); +} + +void HistoryView::changeEvent(QEvent * event) +{ + if (event->type() == QEvent::FontChange) + m_model.setFont(font()); + + audqt::TreeView::changeEvent(event); +} + +static QPointer s_history_view; + +void * PlaybackHistory::get_qt_widget() +{ + assert(!s_history_view); + s_history_view = new HistoryView; + return s_history_view; +} + +int PlaybackHistory::take_message(const char * code, const void *, int) +{ + if (!strcmp(code, "grab focus") && s_history_view) + { + s_history_view->setFocus(Qt::OtherFocusReason); + return 0; + } + + return -1; +} diff --git a/src/qtui/main_window.cc b/src/qtui/main_window.cc index c4188cef77..e1274390aa 100644 --- a/src/qtui/main_window.cc +++ b/src/qtui/main_window.cc @@ -136,6 +136,7 @@ MainWindow::MainWindow() m_center_layout(audqt::make_vbox(m_center_widget, 0)), m_infobar(new InfoBar(this)), m_statusbar(new StatusBar(this)), m_search_tool(aud_plugin_lookup_basename("search-tool-qt")), + m_playback_history(aud_plugin_lookup_basename("playback-history")), m_playlist_manager(aud_plugin_lookup_basename("playlist-manager-qt")) { auto slider = new TimeSlider(this); @@ -423,10 +424,15 @@ void MainWindow::add_dock_item(audqt::DockItem * item) if (!restoreDockWidget(w)) { - addDockWidget(Qt::LeftDockWidgetArea, w); - // only the search tool is docked by default - if (strcmp(item->id(), "search-tool-qt")) - w->setFloating(true); + if (!strcmp(item->id(), "playback-history")) + addDockWidget(Qt::BottomDockWidgetArea, w); + else + { + addDockWidget(Qt::LeftDockWidgetArea, w); + // only the search tool and playback history are docked by default + if (strcmp(item->id(), "search-tool-qt")) + w->setFloating(true); + } } /* workaround for QTBUG-89144 */ diff --git a/src/qtui/main_window.h b/src/qtui/main_window.h index d10eda0953..5b92ee7aa4 100644 --- a/src/qtui/main_window.h +++ b/src/qtui/main_window.h @@ -51,7 +51,7 @@ class MainWindow : public QMainWindow, audqt::DockHost InfoBar * m_infobar; StatusBar * m_statusbar; - PluginHandle *m_search_tool, *m_playlist_manager; + PluginHandle *m_search_tool, *m_playback_history, *m_playlist_manager; QAction *m_menu_action, *m_search_action; QAction *m_play_pause_action, *m_stop_action, *m_stop_after_action; @@ -87,6 +87,11 @@ class MainWindow : public QMainWindow, audqt::DockHost if (m_search_tool) show_dock_plugin(m_search_tool); } + void show_playback_history() + { + if (m_playback_history) + show_dock_plugin(m_playback_history); + } void show_playlist_manager() { if (m_playlist_manager) @@ -115,7 +120,9 @@ class MainWindow : public QMainWindow, audqt::DockHost hook13{"qtui toggle infoarea", this, &MainWindow::update_visibility}, hook14{"qtui toggle statusbar", this, &MainWindow::update_visibility}, hook15{"qtui show search tool", this, &MainWindow::show_search_tool}, - hook16{"qtui show playlist manager", this, + hook16{"qtui show playback history", this, + &MainWindow::show_playback_history}, + hook17{"qtui show playlist manager", this, &MainWindow::show_playlist_manager}; }; diff --git a/src/qtui/menus.cc b/src/qtui/menus.cc index 986909ef24..9c37821e9f 100644 --- a/src/qtui/menus.cc +++ b/src/qtui/menus.cc @@ -81,6 +81,10 @@ static void configure_visualizations() } static void show_search_tool() { hook_call("qtui show search tool", nullptr); } +static void show_playback_history() +{ + hook_call("qtui show playback history", nullptr); +} static void show_playlist_manager() { hook_call("qtui show playlist manager", nullptr); @@ -161,6 +165,9 @@ QMenuBar * qtui_build_menubar(QWidget * parent) audqt::MenuCommand( {N_("Song _Info ..."), "dialog-information", "Ctrl+I"}, audqt::infowin_show_current), + audqt::MenuCommand( + {N_("Playback Histor_y ..."), "view-history", "Ctrl+H"}, + show_playback_history), audqt::MenuSep(), audqt::MenuCommand({N_("Set Repeat Point _A"), nullptr, "Ctrl+1"}, set_ab_repeat_a), diff --git a/src/skins-qt/actions-mainwin.h b/src/skins-qt/actions-mainwin.h index a62eb423ac..c620886bbe 100644 --- a/src/skins-qt/actions-mainwin.h +++ b/src/skins-qt/actions-mainwin.h @@ -25,6 +25,7 @@ void action_ab_set (); void action_play_file (); void action_play_folder (); void action_play_location (); +void action_playback_history(); void action_playlist_manager (); void action_search_tool (); diff --git a/src/skins-qt/actions.cc b/src/skins-qt/actions.cc index d04f88d609..f83d07d58e 100644 --- a/src/skins-qt/actions.cc +++ b/src/skins-qt/actions.cc @@ -74,6 +74,16 @@ void action_play_folder () void action_play_location () { audqt::urlopener_show (true); } +void action_playback_history() +{ + PluginHandle * manager = aud_plugin_lookup_basename("playback-history"); + if (manager) + { + aud_plugin_enable(manager, true); + focus_plugin_window(manager); + } +} + void action_playlist_manager () { PluginHandle * manager = aud_plugin_lookup_basename ("playlist-manager-qt"); diff --git a/src/skins-qt/menus.cc b/src/skins-qt/menus.cc index 152c1ac797..64c3b74645 100644 --- a/src/skins-qt/menus.cc +++ b/src/skins-qt/menus.cc @@ -90,6 +90,7 @@ static const audqt::MenuItem main_items[] = { static const audqt::MenuItem playback_items[] = { audqt::MenuCommand ({N_("Song Info ..."), "dialog-information", "I"}, audqt::infowin_show_current), + audqt::MenuCommand ({N_("Playback History ..."), "view-history", "H"}, action_playback_history), audqt::MenuSep (), audqt::MenuToggle ({N_("Repeat"), "media-playlist-repeat", "R"}, {nullptr, "repeat", "set repeat"}), audqt::MenuToggle ({N_("Shuffle"), "media-playlist-shuffle", "S"}, {nullptr, "shuffle", "set shuffle"}), From f984bbbfb707f7f3a8c60ced078d68ae6a84e49b Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Tue, 2 May 2023 18:39:41 +0300 Subject: [PATCH 2/7] playback-history: allow removing selected entries The implementation is straightforward, because audqt::TreeView already supports removing selected rows. --- src/playback-history/playback-history.cc | 72 +++++++++++++++++++++--- 1 file changed, 64 insertions(+), 8 deletions(-) diff --git a/src/playback-history/playback-history.cc b/src/playback-history/playback-history.cc index 3326513205..4be41c4056 100644 --- a/src/playback-history/playback-history.cc +++ b/src/playback-history/playback-history.cc @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include #include #include @@ -59,9 +60,10 @@ class PlaybackHistory : public GeneralPlugin "History entries are stored only in memory and are lost\n" "on Audacious exit. When the plugin is disabled,\n" "playback history is not tracked at all.\n" - "History entries are only added, never removed.\n" - "The user can restart Audacious or close Playback History\n" - "view to disable the plugin and clear the entries.\n\n" + "History entries are only added, never removed automatically.\n" + "The user can remove selected entries by pressing the Delete key.\n" + "Restart Audacious or disable the plugin by closing\n" + "Playback History view to clear the entries.\n\n" "Currently the playback history is actually an album history,\n" "designed and tailored for the users of Shuffle by Album mode.\n" @@ -162,6 +164,9 @@ class HistoryModel : public QAbstractListModel int columnCount(const QModelIndex & parent) const override; QVariant data(const QModelIndex & index, int role) const override; + bool removeRows(int row, int count, + const QModelIndex & parent = QModelIndex()) override; + private: enum { @@ -173,8 +178,10 @@ class HistoryModel : public QAbstractListModel // In this class the word "position" refers to an index into m_entries. + bool isModelRowOutOfBounds(int row) const; bool isOutOfBounds(const QModelIndex & index) const; int modelRowFromPosition(int position) const; + int positionFromModelRow(int row) const; int positionFromIndex(const QModelIndex & index) const; /** @@ -194,8 +201,8 @@ class HistoryModel : public QAbstractListModel "playback ready", this, &HistoryModel::playbackStarted}; Index m_entries; - /** the position of the entry that is - * currently playing or was played last */ + /** The position of the entry that is currently playing + * or was played last. -1 means "none". */ int m_playingPosition = -1; /** a cached font used to highlight the item that is currently playing */ QFont m_currentlyPlaingFont; @@ -379,6 +386,46 @@ QVariant HistoryModel::data(const QModelIndex & index, int role) const return QVariant(); } +bool HistoryModel::removeRows(int row, int count, const QModelIndex & parent) +{ + if (count <= 0 || parent.isValid()) + return false; + + const int lastRowToRemove = row + count - 1; + if (isModelRowOutOfBounds(row) || isModelRowOutOfBounds(lastRowToRemove)) + return false; + + const int pos = std::min(positionFromModelRow(row), + positionFromModelRow(lastRowToRemove)); + // pos is the lesser of the positions that correspond to the first and last + // removed model rows. Remove the range [pos, pos + count) from m_entries. + + beginRemoveRows(QModelIndex(), row, lastRowToRemove); + + if (m_playingPosition >= pos && m_playingPosition < pos + count) + m_playingPosition = -1; // unset the playing position before removing it + else if (m_playingPosition > pos) + { + // Shift the playing position to account for the removed range. + m_playingPosition -= count; + } + + m_entries.remove(pos, count); + + endRemoveRows(); + + return true; +} + +bool HistoryModel::isModelRowOutOfBounds(int row) const +{ + if (row >= 0 && row < m_entries.len()) + return false; + AUDWARN("Model row is out of bounds: %d is not in the range [0, %d)\n", row, + m_entries.len()); + return true; +} + bool HistoryModel::isOutOfBounds(const QModelIndex & index) const { if (!index.isValid()) @@ -407,12 +454,18 @@ int HistoryModel::modelRowFromPosition(int position) const return m_entries.len() - 1 - position; } -int HistoryModel::positionFromIndex(const QModelIndex & index) const +int HistoryModel::positionFromModelRow(int row) const { - assert(!isOutOfBounds(index)); + assert(!isModelRowOutOfBounds(row)); // modelRowFromPosition() is an involution (self-inverse function), // and thus this inverse function delegates to it. - return modelRowFromPosition(index.row()); + return modelRowFromPosition(row); +} + +int HistoryModel::positionFromIndex(const QModelIndex & index) const +{ + assert(!isOutOfBounds(index)); + return positionFromModelRow(index.row()); } void HistoryModel::updateFontForPosition(int position) @@ -485,6 +538,9 @@ HistoryView::HistoryView() setFrameShape(QFrame::NoFrame); setIndentation(0); + // Allow removing multiple items at once by pressing the Delete key. + setSelectionMode(ExtendedSelection); + m_model.setFont(font()); setModel(&m_model); connect(this, &QTreeView::activated, &m_model, &HistoryModel::activate); From ac25756a4faf2a885832221b0e3c6d2d26e01ae9 Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Wed, 3 May 2023 14:43:08 +0300 Subject: [PATCH 3/7] playback-history: select playlist entry on history entry selection The added feature allows to follow playback history in the playlist without activating history entries, i.e. without affecting playback. --- src/playback-history/playback-history.cc | 202 +++++++++++++++++++---- 1 file changed, 174 insertions(+), 28 deletions(-) diff --git a/src/playback-history/playback-history.cc b/src/playback-history/playback-history.cc index 4be41c4056..35df0db0bf 100644 --- a/src/playback-history/playback-history.cc +++ b/src/playback-history/playback-history.cc @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -102,6 +103,12 @@ class HistoryEntry */ bool assignPlayingEntry(); + /** + * Gives keyboard focus to the corresponding playlist entry and makes it the + * single selected entry in the playlist. + */ + void makeCurrent() const; + /** * Starts playing this entry. * @@ -131,6 +138,12 @@ class HistoryEntry */ bool retrieveAlbum(String & album) const; + /** + * Returns @c true if @a m_playlist exists and @a m_playlistPosition still + * points to the same playlist entry as at the time of last assignment. + */ + bool isAvailable() const; + String m_album; Playlist m_playlist; /**< the playlist, in which this entry was played */ /** The position in @a m_playlist of the first played song from @a m_album. @@ -152,6 +165,15 @@ class HistoryModel : public QAbstractListModel */ void setFont(const QFont & font); + /** + * Gives keyboard focus to the playlist entry that corresponds to the item + * at @p index and makes it the single selected entry in the playlist. + * + * Call this function when the user selects the item at @p index in the + * view. + */ + void makeCurrent(const QModelIndex & index) const; + /** * Plays the song of the item at @p index. * @@ -160,6 +182,11 @@ class HistoryModel : public QAbstractListModel */ void activate(const QModelIndex & index); + /** + * Returns @c true if rows are currently being removed from the model. + */ + bool areRowsBeingRemoved() const { return m_areRowsBeingRemoved; } + int rowCount(const QModelIndex & parent) const override; int columnCount(const QModelIndex & parent) const override; QVariant data(const QModelIndex & index, int role) const override; @@ -204,10 +231,17 @@ class HistoryModel : public QAbstractListModel /** The position of the entry that is currently playing * or was played last. -1 means "none". */ int m_playingPosition = -1; + bool m_areRowsBeingRemoved = false; /** a cached font used to highlight the item that is currently playing */ QFont m_currentlyPlaingFont; }; +#if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0) +/** The optimization returns early from duplicate makeCurrent() invocations and + * relies on a QMetaObject::invokeMethod() overload introduced in Qt 5.10. */ +#define OPTIMIZE_MAKE_CURRENT +#endif + class HistoryView : public audqt::TreeView { public: @@ -215,9 +249,16 @@ class HistoryView : public audqt::TreeView protected: void changeEvent(QEvent * event) override; + void currentChanged(const QModelIndex & current, + const QModelIndex & previous) override; private: + void makeCurrent(const QModelIndex & index); + HistoryModel m_model; +#ifdef OPTIMIZE_MAKE_CURRENT + QModelIndex m_newlyCurrentIndex; +#endif }; bool HistoryEntry::assignPlayingEntry() @@ -243,43 +284,33 @@ bool HistoryEntry::assignPlayingEntry() return retrieveAlbum(m_album); } -bool HistoryEntry::play() const +void HistoryEntry::makeCurrent() const { - if (!m_playlist.exists()) - { - AUDWARN("The activated entry's playlist has been deleted.\n"); - return false; - } + if (!isAvailable()) + return; - assert(m_playlistPosition >= 0); - if (m_playlistPosition >= m_playlist.n_entries()) - { - AUDWARN("The activated entry's position is now out of bounds.\n"); - return false; - } + m_playlist.select_all(false); + m_playlist.select_entry(m_playlistPosition, true); + m_playlist.set_focus(m_playlistPosition); - String currentAlbumAtPlaylistPosition; - if (!retrieveAlbum(currentAlbumAtPlaylistPosition)) - return false; + m_playlist.activate(); +} - // This check does not guarantee that the first played song from m_album - // still resides at m_playlistPosition in m_playlist. In case the user - // inserts or removes a few songs above m_playlistPosition, a different song - // from the same album or a song from an unrelated album that happens to - // have the same name goes undetected. But such coincidences should be much - // more rare and less of a problem than the album inequality condition - // checked here. Therefore, information that uniquely identifies the - // referenced song is not stored in a history entry just for this case. - if (currentAlbumAtPlaylistPosition != m_album) - { - AUDWARN("The album at the activated entry's playlist position has" - " changed.\n"); +bool HistoryEntry::play() const +{ + if (!isAvailable()) return false; - } m_playlist.set_position(m_playlistPosition); m_playlist.start_playback(); + + // Double-clicking a history entry makes it current just before activation. + // In this case m_playlist is already active here. However, m_playlist is + // not active here if the user performs the following steps: + // 1) select a history entry; 2) activate another playlist; + // 3) give focus to History view; 4) press the Enter key. m_playlist.activate(); + return true; } @@ -314,6 +345,43 @@ bool HistoryEntry::retrieveAlbum(String & album) const return true; } +bool HistoryEntry::isAvailable() const +{ + if (!m_playlist.exists()) + { + AUDWARN("The selected entry's playlist has been deleted.\n"); + return false; + } + + assert(m_playlistPosition >= 0); + if (m_playlistPosition >= m_playlist.n_entries()) + { + AUDWARN("The selected entry's position is now out of bounds.\n"); + return false; + } + + String currentAlbumAtPlaylistPosition; + if (!retrieveAlbum(currentAlbumAtPlaylistPosition)) + return false; + + // This check does not guarantee that the first played song from m_album + // still resides at m_playlistPosition in m_playlist. In case the user + // inserts or removes a few songs above m_playlistPosition, a different song + // from the same album or a song from an unrelated album that happens to + // have the same name goes undetected. But such coincidences should be much + // more rare and less of a problem than the album inequality condition + // checked here. Therefore, information that uniquely identifies the + // referenced song is not stored in a history entry just for this case. + if (currentAlbumAtPlaylistPosition != m_album) + { + AUDWARN("The album at the selected entry's playlist position has" + " changed.\n"); + return false; + } + + return true; +} + void HistoryModel::setFont(const QFont & font) { m_currentlyPlaingFont = font; @@ -323,6 +391,14 @@ void HistoryModel::setFont(const QFont & font) updateFontForPosition(m_playingPosition); } +void HistoryModel::makeCurrent(const QModelIndex & index) const +{ + if (isOutOfBounds(index)) + return; + const int pos = positionFromIndex(index); + m_entries[pos].makeCurrent(); +} + void HistoryModel::activate(const QModelIndex & index) { if (isOutOfBounds(index)) @@ -400,6 +476,7 @@ bool HistoryModel::removeRows(int row, int count, const QModelIndex & parent) // pos is the lesser of the positions that correspond to the first and last // removed model rows. Remove the range [pos, pos + count) from m_entries. + m_areRowsBeingRemoved = true; beginRemoveRows(QModelIndex(), row, lastRowToRemove); if (m_playingPosition >= pos && m_playingPosition < pos + count) @@ -413,6 +490,7 @@ bool HistoryModel::removeRows(int row, int count, const QModelIndex & parent) m_entries.remove(pos, count); endRemoveRows(); + m_areRowsBeingRemoved = false; return true; } @@ -544,6 +622,17 @@ HistoryView::HistoryView() m_model.setFont(font()); setModel(&m_model); connect(this, &QTreeView::activated, &m_model, &HistoryModel::activate); + + // Overriding currentChanged() is not sufficient, because a mouse click on + // a current item should make the corresponding playlist entry current but + // does not invoke currentChanged(). + // Connect makeCurrent() to QTreeView::pressed rather than + // QTreeView::clicked for two reasons: + // 1) Any mouse button click, not only left click, + // makes the clicked history view item current. + // 2) The item becomes current during mousePressEvent(), + // not mouseReleaseEvent(). + connect(this, &QTreeView::pressed, this, &HistoryView::makeCurrent); } void HistoryView::changeEvent(QEvent * event) @@ -554,6 +643,63 @@ void HistoryView::changeEvent(QEvent * event) audqt::TreeView::changeEvent(event); } +void HistoryView::currentChanged(const QModelIndex & current, + const QModelIndex & previous) +{ + audqt::TreeView::currentChanged(current, previous); + + AUDDBG("currentChanged: %d => %d\n", previous.row(), current.row()); + + // currentChanged() is called repeatedly while rows are being removed. + // Debug output when there are 4 history entries, the 4th entry is current + // and all 4 entries are removed: + // currentChanged: 3 => 2 + // currentChanged: 2 => 1 + // currentChanged: 1 => 0 + // currentChanged: 0 => -1 + // History entry removal is not an explicit selection of an item, and + // therefore should not affect playlist focus and selection. + if (m_model.areRowsBeingRemoved()) + return; + + // Connecting makeCurrent() to QTreeView::pressed makes clicked entries + // current. This code makes an entry selected via keyboard current. + // In case of keyboard navigation, the previous current index is invalid + // only when focus is transferred to the history view. A focus transfer is + // not an explicit selection of an item, and therefore should not affect + // playlist focus and selection. + if (previous.isValid() && current.isValid()) + makeCurrent(current); +} + +void HistoryView::makeCurrent(const QModelIndex & index) +{ + // QAbstractItemView::pressed is only emitted when the index is valid. + assert(index.isValid()); + +#ifdef OPTIMIZE_MAKE_CURRENT + AUDDBG("makeCurrent: %d => %d\n", m_newlyCurrentIndex.row(), index.row()); + + // Clicking on a noncurrent item calls currentChanged() and emits + // QTreeView::pressed. This function is invoked twice in a row then. + // Return early from the second call to avoid redundant work. + if (index == m_newlyCurrentIndex) + return; + m_newlyCurrentIndex = index; + + // Events are normally not processed between the corresponding + // QTreeView::pressed emission and currentChanged() call. So invalidating + // m_newlyCurrentIndex when control returns to the event loop works here. + [[maybe_unused]] const bool invoked = QMetaObject::invokeMethod( + this, [this] { m_newlyCurrentIndex = {}; }, Qt::QueuedConnection); + assert(invoked); +#else + AUDDBG("makeCurrent: %d\n", index.row()); +#endif + + m_model.makeCurrent(index); +} + static QPointer s_history_view; void * PlaybackHistory::get_qt_widget() From 81605bb94a07166439dbf42d3a9864da40c91c0b Mon Sep 17 00:00:00 2001 From: Igor Kushnir Date: Fri, 4 Oct 2024 18:30:00 +0300 Subject: [PATCH 4/7] playback-history: implement Song mode and mode switching preferences --- src/playback-history/playback-history.cc | 220 ++++++++++++++++++----- 1 file changed, 172 insertions(+), 48 deletions(-) diff --git a/src/playback-history/playback-history.cc b/src/playback-history/playback-history.cc index 35df0db0bf..7b9e57454c 100644 --- a/src/playback-history/playback-history.cc +++ b/src/playback-history/playback-history.cc @@ -1,6 +1,6 @@ /* * playback-history.cc - * Copyright (C) 2023 Igor Kushnir + * Copyright (C) 2023-2024 Igor Kushnir * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -30,7 +30,9 @@ #include #include #include +#include #include +#include #include /** @@ -55,7 +57,7 @@ class PlaybackHistory : public GeneralPlugin private: static constexpr const char * aboutText = N_("Playback History Plugin\n\n" - "Copyright 2023 Igor Kushnir \n\n" + "Copyright 2023-2024 Igor Kushnir \n\n" "This plugin tracks and provides access to playback history.\n\n" "History entries are stored only in memory and are lost\n" @@ -66,17 +68,21 @@ class PlaybackHistory : public GeneralPlugin "Restart Audacious or disable the plugin by closing\n" "Playback History view to clear the entries.\n\n" - "Currently the playback history is actually an album history,\n" - "designed and tailored for the users of Shuffle by Album mode.\n" - "Extending the plugin to optionally track and display\n" - "individual song history could be a useful future development.\n" - "Feel free to contribute this feature."); + "Two history item granularities (modes) are supported.\n" + "The user can select a mode in the plugin's settings.\n" + "The Song mode is the default. Each played song is stored\n" + "in history. Song titles are displayed in the list.\n" + "When the Album mode is selected and multiple songs from\n" + "a single album are played in a row, a single album entry\n" + "is stored in history. Album names are displayed in the list."); + + static const char * const defaults[]; + static const PreferencesWidget widgets[]; + static const PluginPreferences prefs; public: static constexpr PluginInfo info = {N_("Playback History"), PACKAGE, - aboutText, - nullptr, // prefs - PluginQtOnly}; + aboutText, &prefs, PluginQtOnly}; constexpr PlaybackHistory() : GeneralPlugin(info, false) {} @@ -86,9 +92,19 @@ class PlaybackHistory : public GeneralPlugin EXPORT PlaybackHistory aud_plugin_instance; +static constexpr const char * configSection = "playback-history"; +static constexpr const char * configEntryType = "entry_type"; + class HistoryEntry { public: + enum class Type + { + Song = Tuple::Field::Title, + Album = Tuple::Field::Album + }; + static constexpr Type defaultType = Type::Song; + /** * Creates an invalid entry that can only be assigned to or destroyed. */ @@ -124,19 +140,38 @@ class HistoryEntry */ void debugPrint(const char * prefix) const; - const String & album() const { return m_album; } + Type type() const { return m_type; } + /** + * Returns a translated human-readable designation of text() based on + * type(). + */ + const char * translatedTextDesignation() const; + /** + * Returns this entry's song title if type() is Song or its album name if + * type() is Album. + */ + const String & text() const { return m_text; } + + Playlist playlist() const { return m_playlist; } String playlistTitle() const { return m_playlist.get_title(); } /** Returns the playlist Entry Number of this entry. */ int entryNumber() const; private: /** - * Retrieves the album at @a m_playlistPosition in @a m_playlist and assigns - * it to @p album. + * Returns an untranslated human-readable designation of text() based on + * type(). + */ + const char * untranslatedTextDesignation() const; + + /** + * Retrieves the song title if type() is Song or the album name if type() is + * Album at @a m_playlistPosition in @a m_playlist and assigns it to + * @p text. * - * @return @c true if the album was retrieved successfully. + * @return @c true if the text was retrieved successfully. */ - bool retrieveAlbum(String & album) const; + bool retrieveText(String & text) const; /** * Returns @c true if @a m_playlist exists and @a m_playlistPosition still @@ -144,14 +179,18 @@ class HistoryEntry */ bool isAvailable() const; - String m_album; + String m_text; Playlist m_playlist; /**< the playlist, in which this entry was played */ - /** The position in @a m_playlist of the first played song from @a m_album. + /** The position in @a m_playlist of the song titled text() if type() is + * Song or of the first played song from the album named text() if type() is + * Album. * When the user modifies the playlist, positions shift, but this should * happen rarely enough and therefore doesn't have to be handled perfectly. - * A linear search for @a m_album in @a m_playlist is inefficient, - * and thus is never performed by this plugin. */ + * A linear search for a song title or an album name equal to text() in + * @a m_playlist is inefficient, and thus is never performed by this plugin. + */ int m_playlistPosition = -1; + Type m_type = defaultType; }; class HistoryModel : public QAbstractListModel @@ -197,9 +236,7 @@ class HistoryModel : public QAbstractListModel private: enum { - /** The column name is general, because a song title might optionally be - * displayed in place of an album title in the future. */ - ColumnText, + ColumnText, /**< a song title or an album name */ NColumns }; @@ -281,7 +318,20 @@ bool HistoryEntry::assignPlayingEntry() assert(m_playlistPosition >= 0); assert(m_playlistPosition < m_playlist.n_entries()); - return retrieveAlbum(m_album); + const auto entryType = aud_get_int(configSection, configEntryType); + if (entryType == static_cast(Type::Song) || + entryType == static_cast(Type::Album)) + { + m_type = static_cast(entryType); + } + else + { + AUDWARN("Invalid %s.%s config value: %d.\n", configSection, + configEntryType, entryType); + m_type = defaultType; + } + + return retrieveText(m_text); } void HistoryEntry::makeCurrent() const @@ -316,8 +366,21 @@ bool HistoryEntry::play() const void HistoryEntry::debugPrint(const char * prefix) const { - AUDDBG("%salbum=\"%s\", playlist=\"%s\", entry number=%d\n", prefix, - printable(m_album), printable(playlistTitle()), entryNumber()); + AUDDBG("%s%s=\"%s\", playlist=\"%s\", entry number=%d\n", prefix, + untranslatedTextDesignation(), printable(m_text), + printable(playlistTitle()), entryNumber()); +} + +const char * HistoryEntry::translatedTextDesignation() const +{ + switch (m_type) + { + case Type::Song: + return _("Title"); + case Type::Album: + return _("Album"); + } + Q_UNREACHABLE(); } int HistoryEntry::entryNumber() const @@ -327,7 +390,19 @@ int HistoryEntry::entryNumber() const return m_playlistPosition + 1; } -bool HistoryEntry::retrieveAlbum(String & album) const +const char * HistoryEntry::untranslatedTextDesignation() const +{ + switch (m_type) + { + case Type::Song: + return "title"; + case Type::Album: + return "album"; + } + Q_UNREACHABLE(); +} + +bool HistoryEntry::retrieveText(String & text) const { String errorMessage; const auto tuple = m_playlist.entry_tuple(m_playlistPosition, @@ -341,7 +416,7 @@ bool HistoryEntry::retrieveAlbum(String & album) const return false; } - album = tuple.get_str(Tuple::Album); + text = tuple.get_str(static_cast(m_type)); return true; } @@ -360,22 +435,25 @@ bool HistoryEntry::isAvailable() const return false; } - String currentAlbumAtPlaylistPosition; - if (!retrieveAlbum(currentAlbumAtPlaylistPosition)) + String currentTextAtPlaylistPosition; + if (!retrieveText(currentTextAtPlaylistPosition)) return false; - // This check does not guarantee that the first played song from m_album - // still resides at m_playlistPosition in m_playlist. In case the user - // inserts or removes a few songs above m_playlistPosition, a different song - // from the same album or a song from an unrelated album that happens to - // have the same name goes undetected. But such coincidences should be much - // more rare and less of a problem than the album inequality condition + // Text equality does not guarantee that the song, for which this history + // entry was created, still resides at m_playlistPosition in m_playlist. In + // case the user inserts or removes a few songs above m_playlistPosition: + // * if type() is Song, a different song with the same title is unnoticed; + // * if type() is Album, a different song from the same album or a song from + // an unrelated album that happens to have the same name goes undetected. + // But such coincidences should be much more rare + // and less of a problem than the text inequality condition // checked here. Therefore, information that uniquely identifies the // referenced song is not stored in a history entry just for this case. - if (currentAlbumAtPlaylistPosition != m_album) + if (currentTextAtPlaylistPosition != m_text) { - AUDWARN("The album at the selected entry's playlist position has" - " changed.\n"); + AUDWARN("The %s at the selected entry's playlist position has" + " changed.\n", + untranslatedTextDesignation()); return false; } @@ -413,6 +491,11 @@ void HistoryModel::activate(const QModelIndex & index) // Update m_playingPosition here to prevent the imminent playbackStarted() // invocation from appending a copy of the activated entry to m_entries. + // This does not prevent appending a different-type counterpart entry if the + // type of the activated entry does not match the currently configured + // History Item Granularity. Such a scenario is uncommon (happens only when + // the user switches between the History modes), and so is not specially + // handled or optimized for. if (pos != m_playingPosition) { const int prevPlayingPosition = m_playingPosition; @@ -442,15 +525,16 @@ QVariant HistoryModel::data(const QModelIndex & index, int role) const switch (role) { case Qt::DisplayRole: - return toQString(m_entries[pos].album()); + return toQString(m_entries[pos].text()); case Qt::ToolTipRole: { const auto & entry = m_entries[pos]; // The playlist title and entry number are rarely interesting and // therefore shown only in the tooltip. - return QString::fromUtf8(_("Album: %1
Playlist: %2" - "
Entry Number: %3")) - .arg(toQString(entry.album()), toQString(entry.playlistTitle()), + return QString::fromUtf8(_("%1: %2
Playlist: %3" + "
Entry Number: %4")) + .arg(QString::fromUtf8(entry.translatedTextDesignation()), + toQString(entry.text()), toQString(entry.playlistTitle()), QString::number(entry.entryNumber())); } case Qt::FontRole: @@ -572,11 +656,35 @@ void HistoryModel::playbackStarted() AUDDBG("playing position=%d, entry count=%d\n", m_playingPosition, m_entries.len()); - if (m_playingPosition >= 0 && - m_entries[m_playingPosition].album() == entry.album()) - { - // In all probability, a song from the same album started playing, in - // which case there is nothing to do. Much less likely, a different + const auto shouldAppendEntry = [this, &entry] { + if (m_playingPosition < 0) + return true; + const auto & prevPlayingEntry = m_entries[m_playingPosition]; + + if (prevPlayingEntry.type() != entry.type() || + prevPlayingEntry.playlist() != entry.playlist()) + { + return true; // the two entries are very different indeed + } + + // When the entry numbers differ, either the entries point to two + // different songs or the user has modified the playlist and playlist + // positions have shifted, which invalidated prevPlayingEntry. Either + // way, the entry should be appended if the type is Song. + if (entry.type() == HistoryEntry::Type::Song && + prevPlayingEntry.entryNumber() != entry.entryNumber()) + { + return true; + } + + // If the type is Song, equal entry numbers but differing song titles + // mean that the user has modified the playlist and playlist positions + // have shifted. Append the entry in this case, because it is not the + // same as the previous one. + // If the type is Album, let us assume the Shuffle by Album playback + // mode is enabled. Then equal album names, in all probability, + // mean that a song from the same album started playing, in which case + // the entry should not be appended. Much less likely, a different // album with the same name, separated by other albums from the // previously played one, was just randomly selected. // Ignore this possibility here. The users concerned about such an @@ -584,8 +692,10 @@ void HistoryModel::playbackStarted() // and ensure unique album names. The unique album names would also // prevent Audacious from erroneously playing same-name albums // in order if they happen to end up adjacent in a playlist. + return prevPlayingEntry.text() != entry.text(); + }(); + if (!shouldAppendEntry) return; - } const int prevPlayingPosition = m_playingPosition; @@ -719,3 +829,17 @@ int PlaybackHistory::take_message(const char * code, const void *, int) return -1; } + +const char * const PlaybackHistory::defaults[] = { + configEntryType, + aud::numeric_string(HistoryEntry::defaultType)>::str, + nullptr}; + +const PreferencesWidget PlaybackHistory::widgets[] = { + WidgetLabel(N_("History Item Granularity")), + WidgetRadio(N_("Song"), WidgetInt(configSection, configEntryType), + {static_cast(HistoryEntry::Type::Song)}), + WidgetRadio(N_("Album"), WidgetInt(configSection, configEntryType), + {static_cast(HistoryEntry::Type::Album)})}; + +const PluginPreferences PlaybackHistory::prefs = {{widgets}}; From 47ab4d366f5628b7d0268f94ace0dc499328c423 Mon Sep 17 00:00:00 2001 From: Thomas Lange Date: Fri, 10 Jan 2025 16:57:15 +0100 Subject: [PATCH 5/7] playback-history: Add "-qt" suffix for consistency --- configure.ac | 2 +- src/meson.build | 2 +- src/{playback-history => playback-history-qt}/Makefile | 2 +- src/{playback-history => playback-history-qt}/meson.build | 2 +- .../playback-history.cc | 0 src/qtui/main_window.cc | 4 ++-- src/skins-qt/actions-mainwin.h | 2 +- src/skins-qt/actions.cc | 8 ++++---- 8 files changed, 11 insertions(+), 11 deletions(-) rename src/{playback-history => playback-history-qt}/Makefile (83%) rename src/{playback-history => playback-history-qt}/meson.build (80%) rename src/{playback-history => playback-history-qt}/playback-history.cc (100%) diff --git a/configure.ac b/configure.ac index 06f9db46ff..931911bc16 100644 --- a/configure.ac +++ b/configure.ac @@ -89,7 +89,7 @@ if test "x$USE_GTK" = "xyes" ; then fi if test "x$USE_QT" = "xyes" ; then - GENERAL_PLUGINS="$GENERAL_PLUGINS albumart-qt lyrics-qt playback-history playlist-manager-qt search-tool-qt song-info-qt statusicon-qt" + GENERAL_PLUGINS="$GENERAL_PLUGINS albumart-qt lyrics-qt playback-history-qt playlist-manager-qt search-tool-qt song-info-qt statusicon-qt" GENERAL_PLUGINS="$GENERAL_PLUGINS qtui skins-qt" VISUALIZATION_PLUGINS="$VISUALIZATION_PLUGINS blur_scope-qt qt-spectrum vumeter-qt" fi diff --git a/src/meson.build b/src/meson.build index 4ef72ca66e..71657bf2dc 100644 --- a/src/meson.build +++ b/src/meson.build @@ -97,7 +97,7 @@ if conf.has('USE_QT') subdir('albumart-qt') subdir('blur_scope-qt') subdir('lyrics-qt') - subdir('playback-history') + subdir('playback-history-qt') subdir('playlist-manager-qt') subdir('qt-spectrum') subdir('qtui') diff --git a/src/playback-history/Makefile b/src/playback-history-qt/Makefile similarity index 83% rename from src/playback-history/Makefile rename to src/playback-history-qt/Makefile index 9574129bf3..7f037d92cf 100644 --- a/src/playback-history/Makefile +++ b/src/playback-history-qt/Makefile @@ -1,4 +1,4 @@ -PLUGIN = playback-history${PLUGIN_SUFFIX} +PLUGIN = playback-history-qt${PLUGIN_SUFFIX} SRCS = playback-history.cc diff --git a/src/playback-history/meson.build b/src/playback-history-qt/meson.build similarity index 80% rename from src/playback-history/meson.build rename to src/playback-history-qt/meson.build index 7a587e33a4..111c56453c 100644 --- a/src/playback-history/meson.build +++ b/src/playback-history-qt/meson.build @@ -1,4 +1,4 @@ -shared_module('playback-history', +shared_module('playback-history-qt', 'playback-history.cc', dependencies: [audacious_dep, qt_dep, audqt_dep], name_prefix: '', diff --git a/src/playback-history/playback-history.cc b/src/playback-history-qt/playback-history.cc similarity index 100% rename from src/playback-history/playback-history.cc rename to src/playback-history-qt/playback-history.cc diff --git a/src/qtui/main_window.cc b/src/qtui/main_window.cc index e1274390aa..972e0ee20f 100644 --- a/src/qtui/main_window.cc +++ b/src/qtui/main_window.cc @@ -136,7 +136,7 @@ MainWindow::MainWindow() m_center_layout(audqt::make_vbox(m_center_widget, 0)), m_infobar(new InfoBar(this)), m_statusbar(new StatusBar(this)), m_search_tool(aud_plugin_lookup_basename("search-tool-qt")), - m_playback_history(aud_plugin_lookup_basename("playback-history")), + m_playback_history(aud_plugin_lookup_basename("playback-history-qt")), m_playlist_manager(aud_plugin_lookup_basename("playlist-manager-qt")) { auto slider = new TimeSlider(this); @@ -424,7 +424,7 @@ void MainWindow::add_dock_item(audqt::DockItem * item) if (!restoreDockWidget(w)) { - if (!strcmp(item->id(), "playback-history")) + if (!strcmp(item->id(), "playback-history-qt")) addDockWidget(Qt::BottomDockWidgetArea, w); else { diff --git a/src/skins-qt/actions-mainwin.h b/src/skins-qt/actions-mainwin.h index c620886bbe..02876cba73 100644 --- a/src/skins-qt/actions-mainwin.h +++ b/src/skins-qt/actions-mainwin.h @@ -25,7 +25,7 @@ void action_ab_set (); void action_play_file (); void action_play_folder (); void action_play_location (); -void action_playback_history(); +void action_playback_history (); void action_playlist_manager (); void action_search_tool (); diff --git a/src/skins-qt/actions.cc b/src/skins-qt/actions.cc index f83d07d58e..5f81904448 100644 --- a/src/skins-qt/actions.cc +++ b/src/skins-qt/actions.cc @@ -76,11 +76,11 @@ void action_play_location () void action_playback_history() { - PluginHandle * manager = aud_plugin_lookup_basename("playback-history"); - if (manager) + PluginHandle * history = aud_plugin_lookup_basename ("playback-history-qt"); + if (history) { - aud_plugin_enable(manager, true); - focus_plugin_window(manager); + aud_plugin_enable (history, true); + focus_plugin_window (history); } } From ca99c0dfb00fb917d6375ed9cd838373930f6a14 Mon Sep 17 00:00:00 2001 From: Thomas Lange Date: Fri, 10 Jan 2025 17:05:21 +0100 Subject: [PATCH 6/7] skins-qt: Extract function --- src/skins-qt/actions.cc | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/skins-qt/actions.cc b/src/skins-qt/actions.cc index 5f81904448..d069ac2b05 100644 --- a/src/skins-qt/actions.cc +++ b/src/skins-qt/actions.cc @@ -74,35 +74,22 @@ void action_play_folder () void action_play_location () { audqt::urlopener_show (true); } -void action_playback_history() +static void enable_and_focus_plugin (const char * name) { - PluginHandle * history = aud_plugin_lookup_basename ("playback-history-qt"); - if (history) + PluginHandle * plugin = aud_plugin_lookup_basename (name); + if (plugin) { - aud_plugin_enable (history, true); - focus_plugin_window (history); + aud_plugin_enable (plugin, true); + focus_plugin_window (plugin); } } +void action_playback_history () + { enable_and_focus_plugin ("playback-history-qt"); } void action_playlist_manager () -{ - PluginHandle * manager = aud_plugin_lookup_basename ("playlist-manager-qt"); - if (manager) - { - aud_plugin_enable (manager, true); - focus_plugin_window (manager); - } -} - + { enable_and_focus_plugin ("playlist-manager-qt"); } void action_search_tool () -{ - PluginHandle * search = aud_plugin_lookup_basename ("search-tool-qt"); - if (search) - { - aud_plugin_enable (search, true); - focus_plugin_window (search); - } -} + { enable_and_focus_plugin ("search-tool-qt"); } void action_playlist_rename () { audqt::playlist_show_rename (ACTIVE); } From d0ed96f343e40483677dd9e088cfbcc503b59454 Mon Sep 17 00:00:00 2001 From: Thomas Lange Date: Fri, 10 Jan 2025 17:42:47 +0100 Subject: [PATCH 7/7] playback-history: Don't set a menu icon "view-history" is no specified icon name. Since I found no good alternative just don't set any. See also: https://specifications.freedesktop.org/icon-naming-spec/latest/ --- src/qtui/menus.cc | 2 +- src/skins-qt/menus.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qtui/menus.cc b/src/qtui/menus.cc index 9c37821e9f..fe7e62a035 100644 --- a/src/qtui/menus.cc +++ b/src/qtui/menus.cc @@ -166,7 +166,7 @@ QMenuBar * qtui_build_menubar(QWidget * parent) {N_("Song _Info ..."), "dialog-information", "Ctrl+I"}, audqt::infowin_show_current), audqt::MenuCommand( - {N_("Playback Histor_y ..."), "view-history", "Ctrl+H"}, + {N_("Playback Histor_y ..."), nullptr, "Ctrl+H"}, show_playback_history), audqt::MenuSep(), audqt::MenuCommand({N_("Set Repeat Point _A"), nullptr, "Ctrl+1"}, diff --git a/src/skins-qt/menus.cc b/src/skins-qt/menus.cc index 64c3b74645..077aa58849 100644 --- a/src/skins-qt/menus.cc +++ b/src/skins-qt/menus.cc @@ -90,7 +90,7 @@ static const audqt::MenuItem main_items[] = { static const audqt::MenuItem playback_items[] = { audqt::MenuCommand ({N_("Song Info ..."), "dialog-information", "I"}, audqt::infowin_show_current), - audqt::MenuCommand ({N_("Playback History ..."), "view-history", "H"}, action_playback_history), + audqt::MenuCommand ({N_("Playback History ..."), nullptr, "H"}, action_playback_history), audqt::MenuSep (), audqt::MenuToggle ({N_("Repeat"), "media-playlist-repeat", "R"}, {nullptr, "repeat", "set repeat"}), audqt::MenuToggle ({N_("Shuffle"), "media-playlist-shuffle", "S"}, {nullptr, "shuffle", "set shuffle"}),