Merge pull request xbmc#23848 from ksooo/fix-and-refactor-video-select-actions

[listproviders][video][PVR][utils][builtins] Fix and Refactor Video Select Actions

Author: ksooo

addons/resource.language.en_gb/resources/strings.po

@@ -955,9 +955,8 @@ msgstr ""
 #: xbmc/dialogs/GUIDialogPlayEject.cpp
 #: xbmc/games/windows/GUIWindowGames.cpp
 #: xbmc/music/ContextMenus.h
-#: xbmc/music/MusicUtils.cpp
 #: xbmc/video/ContextMenus.cpp
-#: xbmc/video/windows/GUIWindowVideoBase.cpp
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
 #: system/settings/settings.xml
 msgctxt "#208"
 msgid "Play"
@@ -5723,10 +5722,9 @@ msgstr ""
 #: addons/skin.estuary/xml/SkinSettings.xml
 #: addons/skin.estuary/xml/Variables.xml
 #: xbmc/Autorun.cpp
-#: xbmc/pvr/PVRContextMenus.cpp
-#: xbmc/pvr/PVRGUIActions.cpp
+#: xbmc/pvr/PVRGUIActionsPlayback.cpp
 #: xbmc/video/ContextMenus.cpp
-#: xbmc/video/windows/GUIWindowVideoBase.cpp
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
 msgctxt "#12021"
 msgid "Play from beginning"
 msgstr ""
@@ -6928,11 +6926,13 @@ msgctxt "#13346"
 msgid "Movie information"
 msgstr ""
 
+#. Label for 'queue item' (adds an item to a playlist), used at several places
 #: addons/skin.estuary/xml/SkinSettings.xml
 #: addons/skin.estuary/xml/Variables.xml
 #: system/settings/settings.xml
 #: xbmc/music/ContextMenus.h
-#: xbmc/video/ContextMenus.cpp
+#: xbmc/video/ContextMenus.h
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
 msgctxt "#13347"
 msgid "Queue item"
 msgstr ""
@@ -13303,6 +13303,9 @@ msgctxt "#20323"
 msgid "Movie plot"
 msgstr ""
 
+#. Label for context menu entry to start playback of a part of a video stack
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
+#: xbmc/video/windows/GUIWindowVideoBase.cpp
 msgctxt "#20324"
 msgid "Play part..."
 msgstr ""
@@ -15318,11 +15321,17 @@ msgctxt "#22080"
 msgid "Choose"
 msgstr ""
 
+#. Label for video select action 'show info'
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
 #: system/settings/settings.xml
 msgctxt "#22081"
 msgid "Show information"
 msgstr ""
 
+#. Label to denote that there is something 'more'
+#: xbmc/listproviders/DirectoryProvider.cpp
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
+#: system/settings/settings.xml
 msgctxt "#22082"
 msgid "More..."
 msgstr ""
@@ -15342,6 +15351,9 @@ msgctxt "#23050"
 msgid "Activate teletext"
 msgstr ""
 
+#. Label for disc stack part and number
+#: xbmc/video/guilib/VideoSelectActionProcessor.cpp
+#: xbmc/video/VideoUtils.cpp
 msgctxt "#23051"
 msgid "Part {0:d}"
 msgstr ""
@@ -19475,7 +19487,7 @@ msgstr ""
 #. Description of setting with label #22079 "Default select action"
 #: system/settings/settings.xml
 msgctxt "#36177"
-msgid "Toggle between [Choose], [Play] (default), [Resume] and [Show information].[CR][Choose] Will select an item, e.g. open a directory in files mode.[CR][Resume] Will automatically resume videos from the last position that you were viewing them, even after restarting the system."
+msgid "Toggle between [Choose], [Play] (default), [Resume], [Show information] and [Queue item].[CR][Choose] will open a context menu to select an item, e.g. show information.[CR][Play] will automatically play videos from the beginning or if a resume point is present ask whether to play from beginning or to resume.[CR][Resume] Will automatically resume videos from the last position that you were viewing them.[CR][Show information] will open the video's information dialog.[CR][Queue item] will append the video to the video playlist."
 msgstr ""
 
 #. Description of setting with label #20433 "Extract thumbnails and video information"

cmake/treedata/common/video.txt

@@ -1,5 +1,6 @@
 xbmc/video           video
 xbmc/video/dialogs   video/dialogs
+xbmc/video/guilib    video/guilib
 xbmc/video/jobs      video/jobs
 xbmc/video/tags      video/tags
 xbmc/video/windows   video/windows

xbmc/FileItem.cpp

@@ -886,9 +886,9 @@ bool CFileItem::IsVideo() const
   if (HasPictureInfoTag())
     return false;
 
-  // only tv recordings are videos...
-  if (IsPVRRecording())
-    return !GetPVRRecordingInfoTag()->IsRadio();
+  // TV recordings are videos...
+  if (!m_bIsFolder && URIUtils::IsPVRTVRecordingFileOrFolder(GetPath()))
+    return true;
 
   // ... all other PVR items are not.
   if (IsPVR())
@@ -1178,7 +1178,7 @@ bool CFileItem::IsNFO() const
 
 bool CFileItem::IsDiscImage() const
 {
-  return URIUtils::HasExtension(GetDynPath(), ".img|.iso|.nrg|.udf");
+  return URIUtils::IsDiscImage(GetDynPath());
 }
 
 bool CFileItem::IsOpticalMediaFile() const
@@ -3741,14 +3741,14 @@ bool CFileItem::LoadDetails()
     return true;
   }
 
-  if (m_bIsFolder && URIUtils::IsPVRRecordingFileOrFolder(GetPath()))
+  if (URIUtils::IsPVRRecordingFileOrFolder(GetPath()))
   {
     if (HasProperty("watchedepisodes") || HasProperty("watched"))
       return true;
 
     const std::string parentPath = URIUtils::GetParentPath(GetPath());
 
-    //! @todo optimize, find a way to set the details of the directory without loading its content.
+    //! @todo optimize, find a way to set the details of the item without loading parent directory.
     CFileItemList items;
     if (CDirectory::GetDirectory(parentPath, items, "", XFILE::DIR_FLAG_DEFAULTS))
     {

xbmc/application/ApplicationStackHelper.cpp

@@ -75,7 +75,7 @@ bool CApplicationStackHelper::InitializeStack(const CFileItem & item)
     SetRegisteredStack(GetStackPartFileItem(i), stack);
     SetRegisteredStackPartNumber(GetStackPartFileItem(i), i);
   }
-  m_currentStackIsDiscImageStack = CFileItem(CStackDirectory::GetFirstStackedFile(item.GetPath()), false).IsDiscImage();
+  m_currentStackIsDiscImageStack = URIUtils::IsDiscImageStack(item.GetDynPath());
 
   return true;
 }

xbmc/dialogs/GUIDialogContextMenu.h

@@ -53,7 +53,6 @@ enum CONTEXT_BUTTON
   CONTEXT_BUTTON_PLAY_WITH,
   CONTEXT_BUTTON_PLAY_PARTYMODE,
   CONTEXT_BUTTON_PLAY_PART,
-  CONTEXT_BUTTON_RESUME_ITEM,
   CONTEXT_BUTTON_EDIT,
   CONTEXT_BUTTON_EDIT_SMART_PLAYLIST,
   CONTEXT_BUTTON_INFO,

xbmc/interfaces/builtins/PlayerBuiltins.cpp

@@ -36,7 +36,7 @@
 #include "utils/log.h"
 #include "video/PlayerController.h"
 #include "video/VideoUtils.h"
-#include "video/windows/GUIWindowVideoBase.h"
+#include "video/guilib/VideoSelectActionProcessor.h"
 
 #include <math.h>
 
@@ -502,14 +502,33 @@ int PlayOrQueueMedia(const std::vector<std::string>& params, bool forcePlay)
   if (!item.m_bIsFolder && item.IsPlugin())
     item.SetProperty("IsPlayable", true);
 
-  if (askToResume == true)
+  if (askToResume)
   {
-    if (CGUIWindowVideoBase::ShowResumeMenu(item) == false)
+    const VIDEO::GUILIB::SelectAction action =
+        VIDEO::GUILIB::CVideoSelectActionProcessorBase::ChoosePlayOrResume(item);
+    if (action == VIDEO::GUILIB::SELECT_ACTION_RESUME)
+    {
+      item.SetStartOffset(STARTOFFSET_RESUME);
+    }
+    else if (action != VIDEO::GUILIB::SELECT_ACTION_PLAY)
+    {
+      // The Resume dialog was closed without any choice
       return false;
+    }
     item.SetProperty("check_resume", false);
   }
 
-  if (item.m_bIsFolder || item.IsPlayList())
+  if (item.IsStack())
+  {
+    const VIDEO_UTILS::ResumeInformation resumeInfo =
+        VIDEO_UTILS::GetStackPartResumeInformation(item, playOffset + 1);
+
+    if (item.GetStartOffset() == STARTOFFSET_RESUME)
+      item.SetStartOffset(resumeInfo.startOffset);
+
+    item.m_lStartPartNumber = resumeInfo.partNumber;
+  }
+  else if (!forcePlay /* queue */ || item.m_bIsFolder || item.IsPlayList())
   {
     CFileItemList items;
     GetItemsForPlayList(std::make_shared<CFileItem>(item), items);
@@ -774,17 +793,17 @@ static int SubtitleShiftDown(const std::vector<std::string>& params)
 ///   \table_row2_l{
 ///     <b>`PlayMedia(media[\,isdir][\,1]\,[playoffset=xx])`</b>
 ///     ,
-///     Plays the media. This can be a playlist\, music\, or video file\, directory\,
-///     plugin or an Url. The optional parameter "\,isdir" can be used for playing
-///     a directory. "\,1" will start the media without switching to fullscreen.
-///     If media is a playlist\, you can use playoffset=xx where xx is
-///     the position to start playback from.
+///     Plays the given media. This can be a playlist\, music\, or video file\, directory\,
+///     plugin\, disc image stack\, video file stack or an URL. The optional parameter `,isdir` can
+///     be used for playing a directory. `,1` will start the media without switching to fullscreen.
+///     If media is a playlist or a disc image stack or a video file stack\, you can use
+///     playoffset=xx where xx is the position to start playback from.
 ///     @param[in] media                 URL to media to play (optional).
-///     @param[in] isdir                 Set "isdir" if media is a directory (optional).
-///     @param[in] windowed              Set "1" to start playback without switching to fullscreen (optional).
-///     @param[in] resume                Set "resume" to force resuming (optional).
-///     @param[in] noresume              Set "noresume" to force not resuming (optional).
-///     @param[in] playeroffset          Set "playoffset=<offset>" to start playback from a given position in a playlist (optional).
+///     @param[in] isdir                 Set `isdir` if media is a directory (optional).
+///     @param[in] windowed              Set `1` to start playback without switching to fullscreen (optional).
+///     @param[in] resume                Set `resume` to force resuming (optional).
+///     @param[in] noresume              Set `noresume` to force not resuming (optional).
+///     @param[in] playoffset            Set `playoffset=<offset>` to start playback from a given position in a playlist or stack (optional).
 ///   }
 ///   \table_row2_l{
 ///     <b>`PlayWith(core)`</b>
@@ -803,17 +822,18 @@ static int SubtitleShiftDown(const std::vector<std::string>& params)
 ///     <b>`QueueMedia(media[\,isdir][\,1][\,playnext]\,[playoffset=xx])`</b>
 ///     \anchor Builtin_QueueMedia,
 ///     Queues the given media. This can be a playlist\, music\, or video file\, directory\,
-///     plugin or an Url. The optional parameter "\,isdir" can be used for playing
-///     a directory. "\,1" will start the media without switching to fullscreen.
-///     If media is a playlist\, you can use playoffset=xx where xx is
-///     the position to start playback from.
+///     plugin\, disc image stack\, video file stack or an URL. The optional parameter `,isdir` can
+///     be used for playing a directory. `,1` will start the media without switching to fullscreen.
+///     If media is a playlist or a disc image stack or a video file stack\, you can use
+///     playoffset=xx where xx is the position to start playback from.
+///     where xx is the position to start playback from.
 ///     @param[in] media                 URL of media to queue.
-///     @param[in] isdir                 Set "isdir" if media is a directory (optional).
-///     @param[in] 1                     Set "1" to start playback without switching to fullscreen (optional).
-///     @param[in] resume                Set "resume" to force resuming (optional).
-///     @param[in] noresume              Set "noresume" to force not resuming (optional).
-///     @param[in] playeroffset          Set "playoffset=<offset>" to start playback from a given position in a playlist (optional).
-///     @param[in] playnext              Set "playnext" to play the media right after the currently playing item, if player is currently
+///     @param[in] isdir                 Set `isdir` if media is a directory (optional).
+///     @param[in] 1                     Set `1` to start playback without switching to fullscreen (optional).
+///     @param[in] resume                Set `resume` to force resuming (optional).
+///     @param[in] noresume              Set `noresume` to force not resuming (optional).
+///     @param[in] playoffset            Set `playoffset=<offset>` to start playback from a given position in a playlist or stack (optional).
+///     @param[in] playnext              Set `playnext` to play the media right after the currently playing item, if player is currently
 ///     playing. If player is not playing, append media to current playlist (optional).
 ///     <p><hr>
 ///     @skinning_v20 **[New builtin]** \link Builtin_QueueMedia `QueueMedia(media[\,isdir][\,1][\,playnext]\,[playoffset=xx])`\endlink

xbmc/listproviders/DirectoryProvider.cpp

@@ -36,9 +36,10 @@
 #include "utils/Variant.h"
 #include "utils/XMLUtils.h"
 #include "utils/log.h"
+#include "video/VideoInfoTag.h"
 #include "video/VideoThumbLoader.h"
 #include "video/dialogs/GUIDialogVideoInfo.h"
-#include "video/windows/GUIWindowVideoBase.h"
+#include "video/guilib/VideoSelectActionProcessor.h"
 
 #include <memory>
 #include <mutex>
@@ -465,16 +466,77 @@ bool ExecuteAction(const std::string& execute)
   }
   return false;
 }
-} // namespace
 
-bool CDirectoryProvider::OnClick(const CGUIListItemPtr &item)
+class CVideoSelectActionProcessor : public VIDEO::GUILIB::CVideoSelectActionProcessorBase
 {
-  CFileItem fileItem(*std::static_pointer_cast<CFileItem>(item));
+public:
+  CVideoSelectActionProcessor(CDirectoryProvider& provider, CFileItem& item)
+    : CVideoSelectActionProcessorBase(item), m_provider(provider)
+  {
+  }
+
+protected:
+  bool OnPlayPartSelected(unsigned int part) override
+  {
+    // part numbers are 1-based
+    BuildAndExecAction("PlayMedia", StringUtils::Format("playoffset={}", part - 1));
+    return true;
+  }
 
-  if (fileItem.HasVideoInfoTag()
-      && CServiceBroker::GetSettingsComponent()->GetSettings()->GetInt(CSettings::SETTING_MYVIDEOS_SELECTACTION) == SELECT_ACTION_INFO
-      && OnInfo(item))
+  bool OnResumeSelected() override
+  {
+    BuildAndExecAction("PlayMedia", "resume");
+    return true;
+  }
+
+  bool OnPlaySelected() override
+  {
+    BuildAndExecAction("PlayMedia", "noresume");
     return true;
+  }
+
+  bool OnQueueSelected() override
+  {
+    BuildAndExecAction("QueueMedia", "");
+    return true;
+  }
+
+  bool OnInfoSelected() override
+  {
+    m_provider.OnInfo(std::make_shared<CFileItem>(m_item));
+    return true;
+  }
+
+  bool OnMoreSelected() override
+  {
+    m_provider.OnContextMenu(std::make_shared<CFileItem>(m_item));
+    return true;
+  }
+
+private:
+  void BuildAndExecAction(const std::string& method, const std::string& param)
+  {
+    std::vector<std::string> params{StringUtils::Paramify(m_item.GetPath())};
+    if (!param.empty())
+      params.emplace_back(param);
+
+    const CExecString es{method, params};
+    ExecuteAction(es.GetExecString());
+  }
+
+  CDirectoryProvider& m_provider;
+};
+} // namespace
+
+bool CDirectoryProvider::OnClick(const CGUIListItemPtr& item)
+{
+  CFileItem fileItem(*std::static_pointer_cast<CFileItem>(item));
+  if (fileItem.HasVideoInfoTag())
+  {
+    CVideoSelectActionProcessor proc{*this, fileItem};
+    if (proc.Process())
+      return true;
+  }
 
   if (fileItem.HasProperty("node.target_url"))
     fileItem.SetPath(fileItem.GetProperty("node.target_url").asString());
@@ -519,10 +581,8 @@ bool CDirectoryProvider::OnPlay(const CGUIListItemPtr& item)
   return true;
 }
 
-bool CDirectoryProvider::OnInfo(const CGUIListItemPtr& item)
+bool CDirectoryProvider::OnInfo(const std::shared_ptr<CFileItem>& fileItem)
 {
-  auto fileItem = std::static_pointer_cast<CFileItem>(item);
-
   if (fileItem->HasAddonInfo())
   {
     return CGUIDialogAddonInfo::ShowForItem(fileItem);
@@ -551,17 +611,27 @@ bool CDirectoryProvider::OnInfo(const CGUIListItemPtr& item)
   return false;
 }
 
-bool CDirectoryProvider::OnContextMenu(const CGUIListItemPtr& item)
+bool CDirectoryProvider::OnInfo(const CGUIListItemPtr& item)
 {
   auto fileItem = std::static_pointer_cast<CFileItem>(item);
+  return OnInfo(fileItem);
+}
 
+bool CDirectoryProvider::OnContextMenu(const std::shared_ptr<CFileItem>& fileItem)
+{
   const std::string target = GetTarget(*fileItem);
   if (!target.empty())
     fileItem->SetProperty("targetwindow", target);
 
   return CONTEXTMENU::ShowFor(fileItem);
 }
 
+bool CDirectoryProvider::OnContextMenu(const CGUIListItemPtr& item)
+{
+  auto fileItem = std::static_pointer_cast<CFileItem>(item);
+  return OnContextMenu(fileItem);
+}
+
 bool CDirectoryProvider::IsUpdating() const
 {
   std::unique_lock<CCriticalSection> lock(m_section);