Merge pull request xbmc#24558 from CrystalP/restore-video-chooser

[video] Restore selection of extras in "Choose" dialog from library selection

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

@@ -24054,8 +24054,9 @@ msgctxt "#40207"
 msgid "When enabled, a video with multiple versions will be shown as a folder in the video library. This folder can then be opened to display the individual video versions. When disabled, the configured select action will be applied."
 msgstr ""
 
-#. Choose video version context menu item label
+#. Choose video version context menu item label and dialog title
 #: xbmc/video/ContextMenus.h
+#: xbmc/video/guilib/VideoVersionHelper.cpp
 msgctxt "#40208"
 msgid "Choose version"
 msgstr ""
@@ -24092,7 +24093,13 @@ msgctxt "#40213"
 msgid "If selected the remote player volume level will be synchronized with the volume of this application.[CR]Warning: Global system volume level may be different than the application volume level - they are controlled independently.[CR]This may lead the remote player to be set to 100% volume level if the application has 100% volume level but the overall system volume level is smaller."
 msgstr ""
 
-#empty strings from id 40214 to 40399
+#. Dialog title for the selection of the video extra
+#: xbmc/video/guilib/VideoVersionHelper.cpp
+msgctxt "#40214"
+msgid "Choose extra"
+msgstr ""
+
+#empty strings from id 40215 to 40399
 
 # Video versions
 

xbmc/video/ContextMenus.cpp

@@ -22,10 +22,12 @@
 #include "utils/StringUtils.h"
 #include "utils/URIUtils.h"
 #include "video/VideoInfoTag.h"
+#include "video/VideoManagerTypes.h"
 #include "video/VideoUtils.h"
 #include "video/dialogs/GUIDialogVideoInfo.h"
 #include "video/guilib/VideoPlayActionProcessor.h"
 #include "video/guilib/VideoSelectActionProcessor.h"
+#include "video/guilib/VideoVersionHelper.h"
 
 #include <utility>
 
@@ -228,16 +230,18 @@ class CVideoSelectActionProcessor : public VIDEO::GUILIB::CVideoSelectActionProc
 
 bool CVideoChooseVersion::IsVisible(const CFileItem& item) const
 {
-  return item.HasVideoVersions();
+  return item.HasVideoVersions() && !VIDEO::IsVideoAssetFile(item);
 }
 
 bool CVideoChooseVersion::Execute(const std::shared_ptr<CFileItem>& item) const
 {
   // force selection dialog, regardless of any settings like 'Select default video version'
-  item->SetProperty("needs_resolved_video_version", true);
+  item->SetProperty("needs_resolved_video_asset", true);
+  item->SetProperty("video_asset_type", static_cast<int>(VideoAssetType::VERSION));
   CVideoSelectActionProcessor proc{item};
   const bool ret = proc.ProcessDefaultAction();
-  item->ClearProperty("needs_resolved_video_version");
+  item->ClearProperty("needs_resolved_video_asset");
+  item->ClearProperty("video_asset_type");
   return ret;
 }
 
@@ -357,12 +361,12 @@ void SetPathAndPlay(const std::shared_ptr<CFileItem>& item, PlayMode mode)
     if (mode == PlayMode::PLAY_VERSION_USING)
     {
       // force video version selection dialog
-      itemCopy->SetProperty("needs_resolved_video_version", true);
+      itemCopy->SetProperty("needs_resolved_video_asset", true);
     }
     else
     {
       // play the given/default video version, if multiple versions are available
-      itemCopy->SetProperty("has_resolved_video_version", true);
+      itemCopy->SetProperty("has_resolved_video_asset", true);
     }
 
     const bool choosePlayer{mode == PlayMode::PLAY_USING || mode == PlayMode::PLAY_VERSION_USING};
@@ -436,12 +440,13 @@ bool CVideoPlayUsing::Execute(const std::shared_ptr<CFileItem>& itemIn) const
 
 bool CVideoPlayVersionUsing::IsVisible(const CFileItem& item) const
 {
-  return item.HasVideoVersions();
+  return item.HasVideoVersions() && !VIDEO::IsVideoAssetFile(item);
 }
 
 bool CVideoPlayVersionUsing::Execute(const std::shared_ptr<CFileItem>& itemIn) const
 {
   const auto item{std::make_shared<CFileItem>(itemIn->GetItemToPlay())};
+  item->SetProperty("video_asset_type", "version");
   SetPathAndPlay(item, PlayMode::PLAY_VERSION_USING);
   return true;
 }

xbmc/video/VideoDatabase.cpp

@@ -8177,7 +8177,7 @@ bool CVideoDatabase::GetMoviesByWhere(const std::string& strBaseDir, const Filte
             path = RewriteVideoVersionURL(strBaseDir, movie);
           }
           // this is a certain version, no need to resolve (e.g. no version chooser on select)
-          pItem->SetProperty("has_resolved_video_version", true);
+          pItem->SetProperty("has_resolved_video_asset", true);
         }
 
         if (path.empty())

xbmc/video/VideoManagerTypes.h

@@ -16,7 +16,7 @@ enum class VideoAssetTypeOwner
   USER = 2
 };
 
-enum class VideoAssetType
+enum class VideoAssetType : int
 {
   UNKNOWN = -1,
   VERSION = 0,

xbmc/video/dialogs/GUIDialogVideoInfo.cpp

@@ -805,7 +805,7 @@ void CGUIDialogVideoInfo::Play(bool resume)
   Close(true);
 
   // play the current video version, even if multiple versions are available
-  m_movieItem->SetProperty("has_resolved_video_version", true);
+  m_movieItem->SetProperty("has_resolved_video_asset", true);
 
   if (resume)
   {
@@ -834,7 +834,7 @@ void CGUIDialogVideoInfo::Play(bool resume)
     }
   }
 
-  m_movieItem->ClearProperty("has_resolved_video_version");
+  m_movieItem->ClearProperty("has_resolved_video_asset");
 }
 
 namespace

xbmc/video/guilib/VideoPlayActionProcessor.cpp

@@ -35,7 +35,7 @@ bool CVideoPlayActionProcessorBase::ProcessAction(Action action)
 {
   m_userCancelled = false;
 
-  const auto movie{CVideoVersionHelper::ChooseMovieFromVideoVersions(m_item)};
+  const auto movie{CVideoVersionHelper::ChooseVideoFromAssets(m_item)};
   if (movie)
     m_item = movie;
   else

xbmc/video/guilib/VideoVersionHelper.cpp

@@ -34,7 +34,8 @@ class CVideoChooser
   explicit CVideoChooser(const std::shared_ptr<const CFileItem>& item) : m_item(item) {}
   virtual ~CVideoChooser() = default;
 
-  void EnableExtras(bool enable) { m_enableExtras = enable; }
+  void EnableTypeSwitch(bool enable) { m_enableTypeSwitch = enable; }
+  void SetInitialAssetType(VideoAssetType type) { m_initialAssetType = type; }
 
   std::shared_ptr<const CFileItem> ChooseVideo();
 
@@ -49,7 +50,8 @@ class CVideoChooser
                                                const CFileItemList& itemsToSwitchTo);
 
   const std::shared_ptr<const CFileItem> m_item;
-  bool m_enableExtras{false};
+  bool m_enableTypeSwitch{false};
+  VideoAssetType m_initialAssetType{VideoAssetType::UNKNOWN};
   bool m_switchType{false};
   CFileItemList m_videoVersions;
   CFileItemList m_videoExtras;
@@ -62,7 +64,15 @@ std::shared_ptr<const CFileItem> CVideoChooser::ChooseVideo()
   m_videoExtras.Clear();
 
   std::shared_ptr<const CFileItem> result;
-  if (!m_item->HasVideoVersions())
+  if (m_enableTypeSwitch && !m_item->HasVideoVersions() && !m_item->HasVideoExtras())
+    return result;
+
+  if (!m_enableTypeSwitch && m_initialAssetType == VideoAssetType::VERSION &&
+      !m_item->HasVideoVersions())
+    return result;
+
+  if (!m_enableTypeSwitch && m_initialAssetType == VideoAssetType::EXTRA &&
+      !m_item->HasVideoExtras())
     return result;
 
   CVideoDatabase db;
@@ -72,22 +82,23 @@ std::shared_ptr<const CFileItem> CVideoChooser::ChooseVideo()
     return result;
   }
 
-  db.GetAssetsForVideo(m_item->GetVideoContentType(), m_item->GetVideoInfoTag()->m_iDbId,
-                       VideoAssetType::VERSION, m_videoVersions);
-
-  if (m_enableExtras)
+  if (m_initialAssetType == VideoAssetType::VERSION || m_enableTypeSwitch)
+  {
     db.GetAssetsForVideo(m_item->GetVideoContentType(), m_item->GetVideoInfoTag()->m_iDbId,
-                         VideoAssetType::EXTRA, m_videoExtras);
-  else
-    m_videoExtras.Clear();
+                         VideoAssetType::VERSION, m_videoVersions);
 
-  // find default version item in list and select it
-  for (const auto& item : m_videoVersions)
-  {
-    item->Select(item->GetVideoInfoTag()->IsDefaultVideoVersion());
+    // find default version item in list and select it
+    for (const auto& item : m_videoVersions)
+    {
+      item->Select(item->GetVideoInfoTag()->IsDefaultVideoVersion());
+    }
   }
 
-  VideoAssetType itemType{VideoAssetType::VERSION};
+  if (m_initialAssetType == VideoAssetType::EXTRA || m_enableTypeSwitch)
+    db.GetAssetsForVideo(m_item->GetVideoContentType(), m_item->GetVideoInfoTag()->m_iDbId,
+                         VideoAssetType::EXTRA, m_videoExtras);
+
+  VideoAssetType itemType{m_initialAssetType};
   while (true)
   {
     if (itemType == VideoAssetType::VERSION)
@@ -119,7 +130,7 @@ std::shared_ptr<const CFileItem> CVideoChooser::ChooseVideoVersion()
     return {};
   }
 
-  return ChooseVideo(*dialog, 40210 /* Versions */, 40211 /* Extras */, m_videoVersions,
+  return ChooseVideo(*dialog, 40208 /* Choose version */, 40211 /* Extras */, m_videoVersions,
                      m_videoExtras);
 }
 
@@ -133,7 +144,7 @@ std::shared_ptr<const CFileItem> CVideoChooser::ChooseVideoExtra()
     return {};
   }
 
-  return ChooseVideo(*dialog, 402011 /* Extras */, 40210 /* Versions */, m_videoExtras,
+  return ChooseVideo(*dialog, 40214 /* Choose extra */, 40210 /* Versions */, m_videoExtras,
                      m_videoVersions);
 }
 
@@ -156,7 +167,7 @@ std::shared_ptr<const CFileItem> CVideoChooser::ChooseVideo(CGUIDialogSelect& di
       StringUtils::Format(g_localizeStrings.Get(headingId), m_item->GetVideoInfoTag()->GetTitle())};
   dialog.SetHeading(heading);
 
-  dialog.EnableButton(!itemsToSwitchTo.IsEmpty(), buttonId);
+  dialog.EnableButton(m_enableTypeSwitch && !itemsToSwitchTo.IsEmpty(), buttonId);
   dialog.SetUseDetails(true);
   dialog.SetMultiSelection(false);
   dialog.SetItems(itemsToDisplay);
@@ -171,21 +182,51 @@ std::shared_ptr<const CFileItem> CVideoChooser::ChooseVideo(CGUIDialogSelect& di
 }
 } // unnamed namespace
 
-std::shared_ptr<CFileItem> CVideoVersionHelper::ChooseMovieFromVideoVersions(
+std::shared_ptr<CFileItem> CVideoVersionHelper::ChooseVideoFromAssets(
     const std::shared_ptr<CFileItem>& item)
 {
-  std::shared_ptr<const CFileItem> videoVersion;
-  if (item->HasVideoVersions())
+  std::shared_ptr<const CFileItem> video;
+
+  VideoAssetType assetType{static_cast<int>(
+      item->GetProperty("video_asset_type").asInteger(static_cast<int>(VideoAssetType::UNKNOWN)))};
+  bool allAssetTypes{false};
+  bool hasMultipleChoices{false};
+
+  switch (assetType)
   {
-    if (!item->GetProperty("needs_resolved_video_version").asBoolean(false))
+    case VideoAssetType::UNKNOWN:
+      // asset type not provided means all types are allowed and the user can switch between types
+      allAssetTypes = true;
+      if (item->HasVideoVersions() || item->HasVideoExtras())
+        hasMultipleChoices = true;
+      break;
+
+    case VideoAssetType::VERSION:
+      if (item->HasVideoVersions())
+        hasMultipleChoices = true;
+      break;
+
+    case VideoAssetType::EXTRA:
+      if (item->HasVideoExtras())
+        hasMultipleChoices = true;
+      break;
+
+    default:
+      CLog::LogF(LOGERROR, "unknown asset type ({})", static_cast<int>(assetType));
+      return {};
+  }
+
+  if (hasMultipleChoices)
+  {
+    if (!item->GetProperty("needs_resolved_video_asset").asBoolean(false))
     {
       // auto select the default video version
       const auto settings{CServiceBroker::GetSettingsComponent()->GetSettings()};
       if (settings->GetBool(CSettings::SETTING_MYVIDEOS_SELECTDEFAULTVERSION))
       {
         if (item->GetVideoInfoTag()->IsDefaultVideoVersion())
         {
-          videoVersion = std::make_shared<const CFileItem>(*item);
+          video = std::make_shared<const CFileItem>(*item);
         }
         else
         {
@@ -201,27 +242,38 @@ std::shared_ptr<CFileItem> CVideoVersionHelper::ChooseMovieFromVideoVersions(
                                               item->GetVideoInfoTag()->m_iDbId, defaultVersion))
               CLog::LogF(LOGERROR, "Unable to get default version from video database!");
             else
-              videoVersion = std::make_shared<const CFileItem>(defaultVersion);
+              video = std::make_shared<const CFileItem>(defaultVersion);
           }
         }
       }
     }
 
-    if (!videoVersion && (item->GetProperty("needs_resolved_video_version").asBoolean(false) ||
-                          !item->GetProperty("has_resolved_video_version").asBoolean(false)))
+    if (!video && (item->GetProperty("needs_resolved_video_asset").asBoolean(false) ||
+                   !item->GetProperty("has_resolved_video_asset").asBoolean(false)))
     {
       CVideoChooser chooser{item};
-      chooser.EnableExtras(false);
+
+      if (allAssetTypes)
+      {
+        chooser.EnableTypeSwitch(true);
+        chooser.SetInitialAssetType(VideoAssetType::VERSION);
+      }
+      else
+      {
+        chooser.EnableTypeSwitch(false);
+        chooser.SetInitialAssetType(assetType);
+      }
+
       const auto result{chooser.ChooseVideo()};
       if (result)
-        videoVersion = result;
+        video = result;
       else
         return {};
     }
   }
 
-  if (videoVersion)
-    return std::make_shared<CFileItem>(*videoVersion);
+  if (video)
+    return std::make_shared<CFileItem>(*video);
 
   return item;
 }

xbmc/video/guilib/VideoVersionHelper.h

@@ -19,8 +19,7 @@ namespace GUILIB
 class CVideoVersionHelper
 {
 public:
-  static std::shared_ptr<CFileItem> ChooseMovieFromVideoVersions(
-      const std::shared_ptr<CFileItem>& item);
+  static std::shared_ptr<CFileItem> ChooseVideoFromAssets(const std::shared_ptr<CFileItem>& item);
 };
 } // namespace GUILIB
 

xbmc/video/windows/GUIWindowVideoBase.cpp

@@ -1474,7 +1474,7 @@ void CGUIWindowVideoBase::UpdateVideoVersionItems()
       int videoVersionId{-1};
       if (item->GetVideoInfoTag()->HasVideoVersions())
       {
-        if (item->GetProperty("has_resolved_video_version").asBoolean(false))
+        if (item->GetProperty("has_resolved_video_asset").asBoolean(false))
         {
           // certain version of the movie
           videoVersionId = item->GetVideoInfoTag()->GetAssetInfo().GetId();