[vlc-devel] [PATCH] qt: player_controller: check item before restore playback position

[Alexandre Janniaux]

The restoration request is emitted from the UI, and the player event
signalling that the input item changed is emitted from a player thread
and asynchronously processed later by the UI thread. It means that the
restoration request could happen although there was no input item to
restore anymore.

Store the input item currently acknowledged by the UI thread and check
it against the current item being played by the player when the
restoration is requested by the UI. If they don't match, avoid
processing the restoration request.

Note that another solution would have been to use the item currently
acknowledged by the UI and restore it by rolling back the previous
playlist state to ensure the media is current. As it is far more complex
to implement and provide zero benefit in normal playback, where the
media is likely to be restored at the beginning of the playback, this
other implementation was chosen.

Fixes #24347
---
 modules/gui/qt/player/player_controller.cpp   | 17 +++++++++++++++++
 modules/gui/qt/player/player_controller_p.hpp |  5 +++++
 2 files changed, 22 insertions(+)

diff --git a/modules/gui/qt/player/player_controller.cpp b/modules/gui/qt/player/player_controller.cpp
index 724232062b..eccbcd586d 100644
--- a/modules/gui/qt/player/player_controller.cpp
+++ b/modules/gui/qt/player/player_controller.cpp
@@ -241,6 +241,8 @@ static  void on_player_current_media_changed(vlc_player_t *, input_item_t *new_m
     {
         that->callAsync([that] () {
             emit that->q_func()->inputChanged(false);
+            vlc_player_locker lock{ that->m_player };
+            that->m_currentItem.reset(nullptr);
         });
         return;
     }
@@ -252,6 +254,11 @@ static  void on_player_current_media_changed(vlc_player_t *, input_item_t *new_m
         that->UpdateArt( newMediaPtr.get() );
         that->UpdateMeta( newMediaPtr.get() );
 
+        {
+            vlc_player_locker lock{ that->m_player };
+            that->m_currentItem = std::move(newMediaPtr);
+        }
+
         that->m_canRestorePlayback = false;
         emit q->playbackRestoreQueried();
 
@@ -1521,6 +1528,16 @@ void PlayerController::restorePlaybackPos()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+
+    /* The media can change before the UI gets updated and thus a
+     * restorePlayback request can be submitted on the wrong media or worse,
+     * no media at all. Here d->m_currentItem is locked by the player lock. */
+    if (d->m_currentItem.get() == nullptr ||
+        d->m_currentItem.get() != vlc_player_GetCurrentMedia( d->m_player ))
+    {
+        return;
+    }
+
     vlc_player_RestorePlaybackPos( d->m_player );
 }
 
diff --git a/modules/gui/qt/player/player_controller_p.hpp b/modules/gui/qt/player/player_controller_p.hpp
index c3f3dd4ea4..1f8a6020cc 100644
--- a/modules/gui/qt/player/player_controller_p.hpp
+++ b/modules/gui/qt/player/player_controller_p.hpp
@@ -82,6 +82,11 @@ public:
     float           m_position = 0.f;
     VLCTick      m_length= 0;
 
+    using InputItemPtr = vlc_shared_data_ptr_type(input_item_t,
+                                                  input_item_Hold,
+                                                  input_item_Release);
+
+    InputItemPtr    m_currentItem;
     bool            m_canRestorePlayback = false;
 
     int             m_capabilities = 0;
-- 
2.25.2