[vlc-devel] [PATCH v2] qt: player_controller: check item before usage

Alexandre Janniaux ajanni at videolabs.io
Fri Mar 20 16:42:39 CET 2020


Request like restoration of the last playback position are 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 UI could request the player to do
something which would be executed on the wrong item, or no item at all.

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

Some requests are not concerned by these change, either because they are
able to affect the following media (change rate) or because they could
potentially be triggered near the end of a media and already handle a
nullptr input (record, a-b loop).

Fixes #24347
---
 modules/gui/qt/player/player_controller.cpp   | 82 ++++++++++++++++---
 modules/gui/qt/player/player_controller.hpp   |  2 +
 modules/gui/qt/player/player_controller_p.hpp |  5 ++
 3 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/modules/gui/qt/player/player_controller.cpp b/modules/gui/qt/player/player_controller.cpp
index 724232062b..8517fad582 100644
--- a/modules/gui/qt/player/player_controller.cpp
+++ b/modules/gui/qt/player/player_controller.cpp
@@ -65,6 +65,19 @@ PlayerControllerPrivate::~PlayerControllerPrivate()
     vlc_player_RemoveTimer( m_player, m_player_timer );
 }

+bool PlayerController::isCurrentItemSynced()
+{
+    Q_D(PlayerController);
+
+    /* The media can change before the UI gets updated and thus a player
+     * request can be submitted on the wrong media or worse, no media at
+     * all. Here d->m_currentItem is read and modified under the player
+     * lock. */
+
+    return d->m_currentItem.get() == vlc_player_GetCurrentMedia( d->m_player );
+}
+
+
 void PlayerControllerPrivate::UpdateName(input_item_t* media)
 {
     Q_Q(PlayerController);
@@ -241,6 +254,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,11 +267,16 @@ static  void on_player_current_media_changed(vlc_player_t *, input_item_t *new_m
         that->UpdateArt( newMediaPtr.get() );
         that->UpdateMeta( newMediaPtr.get() );

+        RecentsMRL::getInstance( that->p_intf )->addRecent( newMediaPtr.get()->psz_uri );
+
+        {
+            vlc_player_locker lock{ that->m_player };
+            that->m_currentItem = std::move(newMediaPtr);
+        }
+
         that->m_canRestorePlayback = false;
         emit q->playbackRestoreQueried();

-        RecentsMRL::getInstance( that->p_intf )->addRecent( newMediaPtr.get()->psz_uri );
-
         emit q->inputChanged(true);
     });
 }
@@ -1129,6 +1149,8 @@ void PlayerController::setTime(VLCTick new_time)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     vlc_player_SetTime( d->m_player, new_time );
 }

@@ -1136,6 +1158,8 @@ void PlayerController::setPosition(float position)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     vlc_player_SetPosition( d->m_player, position );
 }

@@ -1144,10 +1168,10 @@ void PlayerController::jumpFwd()
     Q_D(PlayerController);
     msg_Dbg( d->p_intf, "jumpFwd");
     int i_interval = var_InheritInteger( d->p_intf, "short-jump-size" );
-    {
-        vlc_player_locker lock{ d->m_player };
-        vlc_player_JumpTime( d->m_player, vlc_tick_from_sec( i_interval ) );
-    }
+    vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
+    vlc_player_JumpTime( d->m_player, vlc_tick_from_sec( i_interval ) );
 }

 void PlayerController::jumpBwd()
@@ -1155,10 +1179,10 @@ void PlayerController::jumpBwd()
     Q_D(PlayerController);
     msg_Dbg( d->p_intf, "jumpBwd");
     int i_interval = var_InheritInteger( d->p_intf, "short-jump-size" );
-    {
-        vlc_player_locker lock{ d->m_player };
-        vlc_player_JumpTime( d->m_player, vlc_tick_from_sec( -i_interval ) );
-    }
+    vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
+    vlc_player_JumpTime( d->m_player, vlc_tick_from_sec( -i_interval ) );
 }

 void PlayerController::jumpToTime(VLCTick i_time)
@@ -1166,6 +1190,8 @@ void PlayerController::jumpToTime(VLCTick i_time)
     Q_D(PlayerController);
     msg_Dbg( d->p_intf, "jumpToTime");
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     vlc_player_JumpTime( d->m_player, vlc_tick_from_sec( i_time ) );
 }

@@ -1174,6 +1200,8 @@ void PlayerController::jumpToPos( float new_pos )
     Q_D(PlayerController);
     {
         vlc_player_locker lock{ d->m_player };
+        if( !isCurrentItemSynced() )
+            return;
         if( vlc_player_IsStarted( d->m_player ) )
             vlc_player_SetPosition( d->m_player, new_pos );
     }
@@ -1184,6 +1212,8 @@ void PlayerController::frameNext()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     vlc_player_NextVideoFrame( d->m_player );
 }

@@ -1193,6 +1223,8 @@ void PlayerController::setAudioDelay(VLCTick delay)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     vlc_player_SetAudioDelay( d->m_player, delay, VLC_PLAYER_WHENCE_ABSOLUTE );
 }

@@ -1200,6 +1232,8 @@ void PlayerController::setSubtitleDelay(VLCTick delay)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if(!isCurrentItemSynced() )
+        return;
     vlc_player_SetSubtitleDelay( d->m_player, delay, VLC_PLAYER_WHENCE_ABSOLUTE );
 }

@@ -1207,6 +1241,8 @@ void PlayerController::setSecondarySubtitleDelay(VLCTick delay)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (d->m_secondarySpuEsId.get() != NULL)
         vlc_player_SetEsIdDelay(d->m_player, d->m_secondarySpuEsId.get(),
                                 delay, VLC_PLAYER_WHENCE_ABSOLUTE);
@@ -1216,6 +1252,8 @@ void PlayerController::setSubtitleFPS(float fps)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     vlc_player_SetAssociatedSubsFPS( d->m_player, fps );
 }

@@ -1225,6 +1263,8 @@ void PlayerController::sectionPrev()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if( vlc_player_IsStarted( d->m_player ) )
     {
         if (vlc_player_GetSelectedChapter( d->m_player ) != NULL)
@@ -1238,6 +1278,8 @@ void PlayerController::sectionNext()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if( vlc_player_IsStarted( d->m_player ) )
     {
         if (vlc_player_GetSelectedChapter( d->m_player ) != NULL)
@@ -1251,6 +1293,8 @@ void PlayerController::sectionMenu()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if( vlc_player_IsStarted( d->m_player ) )
         vlc_player_Navigate( d->m_player, VLC_PLAYER_NAV_MENU );
 }
@@ -1259,6 +1303,8 @@ void PlayerController::chapterNext()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsStarted(d->m_player ))
         vlc_player_SelectNextChapter( d->m_player );
 }
@@ -1267,6 +1313,8 @@ void PlayerController::chapterPrev()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsStarted(d->m_player ))
         vlc_player_SelectPrevChapter( d->m_player );
 }
@@ -1275,6 +1323,8 @@ void PlayerController::titleNext()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsStarted(d->m_player ))
         vlc_player_SelectNextTitle( d->m_player );
 }
@@ -1283,6 +1333,8 @@ void PlayerController::titlePrev()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsStarted(d->m_player ))
         vlc_player_SelectPrevTitle( d->m_player );
 }
@@ -1293,6 +1345,8 @@ void PlayerController::changeProgram( int program )
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if( vlc_player_IsStarted( d->m_player ) )
         vlc_player_SelectProgram( d->m_player, program );
 }
@@ -1304,6 +1358,8 @@ void PlayerController::enableTeletext( bool enable )
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsStarted(d->m_player ))
         vlc_player_SetTeletextEnabled( d->m_player, enable );
 }
@@ -1312,6 +1368,8 @@ void PlayerController::setTeletextPage(int page)
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsTeletextEnabled( d->m_player ))
         vlc_player_SelectTeletextPage( d->m_player, page );
 }
@@ -1320,6 +1378,8 @@ void PlayerController::setTeletextTransparency( bool transparent )
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if( !isCurrentItemSynced() )
+        return;
     if (vlc_player_IsTeletextEnabled( d->m_player ))
         vlc_player_SetTeletextTransparency( d->m_player, transparent );
 }
@@ -1521,6 +1581,8 @@ void PlayerController::restorePlaybackPos()
 {
     Q_D(PlayerController);
     vlc_player_locker lock{ d->m_player };
+    if (!isCurrentItemSynced())
+        return;
     vlc_player_RestorePlaybackPos( d->m_player );
 }

diff --git a/modules/gui/qt/player/player_controller.hpp b/modules/gui/qt/player/player_controller.hpp
index 3c792b47a9..99c0f0f296 100644
--- a/modules/gui/qt/player/player_controller.hpp
+++ b/modules/gui/qt/player/player_controller.hpp
@@ -431,6 +431,8 @@ private slots:
     void menusUpdateAudio( const QString& );

 private:
+    bool isCurrentItemSynced();
+
     Q_DECLARE_PRIVATE(PlayerController)
     QScopedPointer<PlayerControllerPrivate> d_ptr;
     QSignalMapper *menusAudioMapper; //used by VLCMenuBar
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


More information about the vlc-devel mailing list