Merge pull request #23985 from ksooo/pvr-improve-recently-played-widget

[PVR] Fix/Improve recently played channels widget (e.g. respect hidden groups and channels).

7 files changed, 203 insertions, 97 deletions

diff --git a/xbmc/pvr/PVRDatabase.cpp b/xbmc/pvr/PVRDatabase.cpp
index d1ea29a122..d36bbb7b6a 100644
--- a/xbmc/pvr/PVRDatabase.cpp
+++ b/xbmc/pvr/PVRDatabase.cpp
@@ -155,7 +155,8 @@ void CPVRDatabase::CreateTables()
               "idEpg                integer, "
               "bHasArchive          bool, "
               "iClientProviderUid   integer, "
-              "bIsUserSetHidden     bool"
+              "bIsUserSetHidden     bool, "
+              "iLastWatchedGroupId  integer"
               ")");
 
   CLog::LogFC(LOGDEBUG, LOGPVR, "Creating table 'channelgroups'");
@@ -361,6 +362,12 @@ void CPVRDatabase::UpdateTables(int iVersion)
     // Should mostly be the order the groups appeared from backend or were created by user locally.
     m_pDS->exec("UPDATE channelgroups SET iPosition = idGroup");
   }
+
+  if (iVersion < 44)
+  {
+    m_pDS->exec("ALTER TABLE channels ADD iLastWatchedGroupId integer");
+    m_pDS->exec("UPDATE channels SET iLastWatchedGroupId = -1");
+  }
 }
 
 /********** Client methods **********/
@@ -584,6 +591,7 @@ int CPVRDatabase::Get(bool bRadio,
         channel->m_bHasArchive = m_pDS->fv("bHasArchive").get_asBool();
         channel->m_iClientProviderUid = m_pDS->fv("iClientProviderUid").get_asInt();
         channel->m_bIsUserSetHidden = m_pDS->fv("bIsUserSetHidden").get_asBool();
+        channel->m_lastWatchedGroupId = m_pDS->fv("iLastWatchedGroupId").get_asInt();
 
         channel->UpdateEncryptionName();
 
@@ -1029,14 +1037,15 @@ bool CPVRDatabase::Persist(CPVRChannel& channel, bool bCommit)
         "INSERT INTO channels ("
         "iUniqueId, bIsRadio, bIsHidden, bIsUserSetIcon, bIsUserSetName, bIsLocked, "
         "sIconPath, sChannelName, bIsVirtual, bEPGEnabled, sEPGScraper, iLastWatched, iClientId, "
-        "idEpg, bHasArchive, iClientProviderUid, bIsUserSetHidden) "
-        "VALUES (%i, %i, %i, %i, %i, %i, '%s', '%s', %i, %i, '%s', %u, %i, %i, %i, %i, %i)",
+        "idEpg, bHasArchive, iClientProviderUid, bIsUserSetHidden, iLastWatchedGroupId) "
+        "VALUES (%i, %i, %i, %i, %i, %i, '%s', '%s', %i, %i, '%s', %u, %i, %i, %i, %i, %i, %i)",
         channel.UniqueID(), (channel.IsRadio() ? 1 : 0), (channel.IsHidden() ? 1 : 0),
         (channel.IsUserSetIcon() ? 1 : 0), (channel.IsUserSetName() ? 1 : 0),
         (channel.IsLocked() ? 1 : 0), channel.IconPath().c_str(), channel.ChannelName().c_str(), 0,
         (channel.EPGEnabled() ? 1 : 0), channel.EPGScraper().c_str(),
         static_cast<unsigned int>(channel.LastWatched()), channel.ClientID(), channel.EpgID(),
-        channel.HasArchive(), channel.ClientProviderUid(), channel.IsUserSetHidden() ? 1 : 0);
+        channel.HasArchive(), channel.ClientProviderUid(), channel.IsUserSetHidden() ? 1 : 0,
+        channel.LastWatchedGroupId());
   }
   else
   {
@@ -1045,15 +1054,16 @@ bool CPVRDatabase::Persist(CPVRChannel& channel, bool bCommit)
         "REPLACE INTO channels ("
         "iUniqueId, bIsRadio, bIsHidden, bIsUserSetIcon, bIsUserSetName, bIsLocked, "
         "sIconPath, sChannelName, bIsVirtual, bEPGEnabled, sEPGScraper, iLastWatched, iClientId, "
-        "idChannel, idEpg, bHasArchive, iClientProviderUid, bIsUserSetHidden) "
-        "VALUES (%i, %i, %i, %i, %i, %i, '%s', '%s', %i, %i, '%s', %u, %i, %s, %i, %i, %i, %i)",
+        "idChannel, idEpg, bHasArchive, iClientProviderUid, bIsUserSetHidden, iLastWatchedGroupId) "
+        "VALUES (%i, %i, %i, %i, %i, %i, '%s', '%s', %i, %i, '%s', %u, %i, %s, %i, %i, %i, %i, %i)",
         channel.UniqueID(), (channel.IsRadio() ? 1 : 0), (channel.IsHidden() ? 1 : 0),
         (channel.IsUserSetIcon() ? 1 : 0), (channel.IsUserSetName() ? 1 : 0),
         (channel.IsLocked() ? 1 : 0), channel.ClientIconPath().c_str(),
         channel.ChannelName().c_str(), 0, (channel.EPGEnabled() ? 1 : 0),
         channel.EPGScraper().c_str(), static_cast<unsigned int>(channel.LastWatched()),
         channel.ClientID(), strValue.c_str(), channel.EpgID(), channel.HasArchive(),
-        channel.ClientProviderUid(), channel.IsUserSetHidden() ? 1 : 0);
+        channel.ClientProviderUid(), channel.IsUserSetHidden() ? 1 : 0,
+        channel.LastWatchedGroupId());
   }
 
   if (QueueInsertQuery(strQuery))
@@ -1067,12 +1077,12 @@ bool CPVRDatabase::Persist(CPVRChannel& channel, bool bCommit)
   return bReturn;
 }
 
-bool CPVRDatabase::UpdateLastWatched(const CPVRChannel& channel)
+bool CPVRDatabase::UpdateLastWatched(const CPVRChannel& channel, int groupId)
 {
   std::unique_lock<CCriticalSection> lock(m_critSection);
-  const std::string strQuery =
-      PrepareSQL("UPDATE channels SET iLastWatched = %u WHERE idChannel = %i",
-                 static_cast<unsigned int>(channel.LastWatched()), channel.ChannelID());
+  const std::string strQuery = PrepareSQL(
+      "UPDATE channels SET iLastWatched = %u, iLastWatchedGroupId = %i WHERE idChannel = %i",
+      static_cast<unsigned int>(channel.LastWatched()), groupId, channel.ChannelID());
   return ExecuteQuery(strQuery);
 }
 
diff --git a/xbmc/pvr/PVRDatabase.h b/xbmc/pvr/PVRDatabase.h
index 61b620b8e9..d061d447e3 100644
--- a/xbmc/pvr/PVRDatabase.h
+++ b/xbmc/pvr/PVRDatabase.h
@@ -64,7 +64,7 @@ namespace PVR
      * @brief Get the minimal database version that is required to operate correctly.
      * @return The minimal database version.
      */
-    int GetSchemaVersion() const override { return 43; }
+    int GetSchemaVersion() const override { return 44; }
 
     /*!
      * @brief Get the default sqlite database filename.
@@ -285,9 +285,10 @@ namespace PVR
     /*!
      * @brief Updates the last watched timestamp for the channel
      * @param channel the channel
+     * @param groupId the id of the group used to watch the channel
      * @return whether the update was successful
      */
-    bool UpdateLastWatched(const CPVRChannel& channel);
+    bool UpdateLastWatched(const CPVRChannel& channel, int groupId);
 
     /*!
      * @brief Updates the last watched timestamp for the channel group
diff --git a/xbmc/pvr/PVRPlaybackState.cpp b/xbmc/pvr/PVRPlaybackState.cpp
index aa3d0aefa2..eef29bd970 100644
--- a/xbmc/pvr/PVRPlaybackState.cpp
+++ b/xbmc/pvr/PVRPlaybackState.cpp
@@ -437,10 +437,47 @@ bool CPVRPlaybackState::CanRecordOnPlayingChannel() const
   return currentChannel && currentChannel->CanRecord();
 }
 
+namespace
+{
+std::shared_ptr<CPVRChannelGroup> GetFirstNonDeletedAndNonHiddenChannelGroup(
+    const std::shared_ptr<CPVRChannelGroupMember>& groupMember)
+{
+  CPVRChannelGroups* groups{
+      CServiceBroker::GetPVRManager().ChannelGroups()->Get(groupMember->IsRadio())};
+  if (groups)
+  {
+    const std::vector<std::shared_ptr<CPVRChannelGroup>> members{
+        groups->GetMembers(true /* exclude hidden */)};
+
+    for (const auto& member : members)
+    {
+      if (member->IsDeleted())
+        continue;
+
+      if (member->GetByUniqueID(groupMember->Channel()->StorageId()))
+        return member;
+    }
+  }
+
+  CLog::LogFC(LOGERROR, LOGPVR,
+              "Failed to obtain non-deleted and non-hidden group for channel '{}‘",
+              groupMember->Channel()->ChannelName());
+  return {};
+}
+} // unnamed namespace
+
 void CPVRPlaybackState::SetActiveChannelGroup(const std::shared_ptr<CPVRChannelGroup>& group)
 {
   if (group)
   {
+    if (group->IsHidden() || group->IsDeleted())
+    {
+      CLog::LogFC(LOGERROR, LOGPVR,
+                  "Rejecting to make hidden or deleted group '{}‘ the active group.",
+                  group->GroupName());
+      return;
+    }
+
     if (group->IsRadio())
       m_activeGroupRadio = group;
     else
@@ -456,57 +493,21 @@ void CPVRPlaybackState::SetActiveChannelGroup(
     const std::shared_ptr<CPVRChannelGroupMember>& channel)
 {
   const bool bRadio = channel->Channel()->IsRadio();
-  const std::shared_ptr<CPVRChannelGroup> group =
-      CServiceBroker::GetPVRManager().ChannelGroups()->Get(bRadio)->GetById(channel->GroupID());
+  std::shared_ptr<CPVRChannelGroup> group{
+      CServiceBroker::GetPVRManager().ChannelGroups()->Get(bRadio)->GetById(channel->GroupID())};
 
-  SetActiveChannelGroup(group);
-}
+  if (group && (group->IsHidden() || group->IsDeleted()))
+    group = GetFirstNonDeletedAndNonHiddenChannelGroup(channel);
 
-namespace
-{
-std::shared_ptr<CPVRChannelGroup> GetFirstNonDeletedAndNonHiddenChannelGroup(bool bRadio)
-{
-  CPVRChannelGroups* groups = CServiceBroker::GetPVRManager().ChannelGroups()->Get(bRadio);
-  if (groups)
-  {
-    const std::vector<std::shared_ptr<CPVRChannelGroup>> members =
-        groups->GetMembers(true); // exclude hidden
-
-    const auto it = std::find_if(members.cbegin(), members.cend(),
-                                 [](const auto& group) { return !group->IsDeleted(); });
-    if (it != members.cend())
-      return (*it);
-  }
-
-  CLog::LogFC(LOGERROR, LOGPVR, "Failed to obtain any non-deleted and non-hidden group");
-  return {};
+  SetActiveChannelGroup(group);
 }
-} // unnamed namespace
 
 std::shared_ptr<CPVRChannelGroup> CPVRPlaybackState::GetActiveChannelGroup(bool bRadio) const
 {
   if (bRadio)
-  {
-    if (m_activeGroupRadio && (m_activeGroupRadio->IsDeleted() || m_activeGroupRadio->IsHidden()))
-    {
-      // switch to first non-deleted and non-hidden group
-      const auto group = GetFirstNonDeletedAndNonHiddenChannelGroup(bRadio);
-      if (group)
-        const_cast<CPVRPlaybackState*>(this)->SetActiveChannelGroup(group);
-    }
     return m_activeGroupRadio;
-  }
   else
-  {
-    if (m_activeGroupTV && (m_activeGroupTV->IsDeleted() || m_activeGroupTV->IsHidden()))
-    {
-      // switch to first non-deleted and non-hidden group
-      const auto group = GetFirstNonDeletedAndNonHiddenChannelGroup(bRadio);
-      if (group)
-        const_cast<CPVRPlaybackState*>(this)->SetActiveChannelGroup(group);
-    }
     return m_activeGroupTV;
-  }
 }
 
 CDateTime CPVRPlaybackState::GetPlaybackTime(int iClientID, int iUniqueChannelID) const
@@ -543,7 +544,7 @@ void CPVRPlaybackState::UpdateLastWatched(const std::shared_ptr<CPVRChannelGroup
   time_t iTime;
   time.GetAsTime(iTime);
 
-  channel->Channel()->SetLastWatched(iTime);
+  channel->Channel()->SetLastWatched(iTime, channel->GroupID());
 
   // update last watched timestamp for group
   const bool bRadio = channel->Channel()->IsRadio();
diff --git a/xbmc/pvr/channels/PVRChannel.cpp b/xbmc/pvr/channels/PVRChannel.cpp
index a65f323474..3c56072faa 100644
--- a/xbmc/pvr/channels/PVRChannel.cpp
+++ b/xbmc/pvr/channels/PVRChannel.cpp
@@ -391,16 +391,17 @@ bool CPVRChannel::SetChannelName(const std::string& strChannelName, bool bIsUser
   return false;
 }
 
-bool CPVRChannel::SetLastWatched(time_t iLastWatched)
+bool CPVRChannel::SetLastWatched(time_t lastWatched, int groupId)
 {
   {
     std::unique_lock<CCriticalSection> lock(m_critSection);
-    m_iLastWatched = iLastWatched;
+    m_iLastWatched = lastWatched;
+    m_lastWatchedGroupId = groupId;
   }
 
   const std::shared_ptr<CPVRDatabase> database = CServiceBroker::GetPVRManager().GetTVDatabase();
   if (database)
-    return database->UpdateLastWatched(*this);
+    return database->UpdateLastWatched(*this, groupId);
 
   return false;
 }
@@ -718,6 +719,12 @@ bool CPVRChannel::IsUserSetHidden() const
   return m_bIsUserSetHidden;
 }
 
+int CPVRChannel::LastWatchedGroupId() const
+{
+  std::unique_lock<CCriticalSection> lock(m_critSection);
+  return m_lastWatchedGroupId;
+}
+
 std::string CPVRChannel::ChannelName() const
 {
   std::unique_lock<CCriticalSection> lock(m_critSection);
diff --git a/xbmc/pvr/channels/PVRChannel.h b/xbmc/pvr/channels/PVRChannel.h
index 8ff50547bb..65039751fb 100644
--- a/xbmc/pvr/channels/PVRChannel.h
+++ b/xbmc/pvr/channels/PVRChannel.h
@@ -182,6 +182,11 @@ public:
   bool IsUserSetHidden() const;
 
   /*!
+   * @return the id of the channel group the channel was watched from the last time; -1 if unknown.
+   */
+  int LastWatchedGroupId() const;
+
+  /*!
    * @brief Set the path to the icon for this channel.
    * @param strIconPath The new path.
    * @param bIsUserSetIcon true if user changed the icon via GUI, false otherwise.
@@ -208,11 +213,12 @@ public:
   time_t LastWatched() const;
 
   /*!
-   * @brief Last time channel has been watched
-   * @param iLastWatched The new value.
+   * @brief Set the last time the channel has been watched and the channel group used to watch.
+   * @param lastWatched The new last watched time value.
+   * @param groupId the id of the group used to watch the channel.
    * @return True if the something changed, false otherwise.
    */
-  bool SetLastWatched(time_t iLastWatched);
+  bool SetLastWatched(time_t lastWatched, int groupId);
 
   /*!
    * @brief Check whether this channel has unpersisted data changes.
@@ -543,6 +549,8 @@ private:
   int m_iClientOrder = 0; /*!< the order from this channels group member */
   int m_iClientProviderUid =
       PVR_PROVIDER_INVALID_UID; /*!< the unique id for this provider from the client */
+  int m_lastWatchedGroupId{
+      -1}; /*!< the id of the channel group the channel was watched from the last time */
   //@}
 
   mutable CCriticalSection m_critSection;
diff --git a/xbmc/pvr/channels/PVRChannelGroup.h b/xbmc/pvr/channels/PVRChannelGroup.h
index 605908663a..3792816e53 100644
--- a/xbmc/pvr/channels/PVRChannelGroup.h
+++ b/xbmc/pvr/channels/PVRChannelGroup.h
@@ -31,6 +31,8 @@ static constexpr int PVR_GROUP_TYPE_LOCAL = 2;
 static constexpr int PVR_GROUP_CLIENT_ID_UNKNOWN = -2;
 static constexpr int PVR_GROUP_CLIENT_ID_LOCAL = -1;
 
+static constexpr int PVR_GROUP_ID_UNNKOWN{-1};
+
 enum class PVREvent;
 
 class CPVRChannel;
diff --git a/xbmc/pvr/filesystem/PVRGUIDirectory.cpp b/xbmc/pvr/filesystem/PVRGUIDirectory.cpp
index 8928cc0539..513a1964b2 100644
--- a/xbmc/pvr/filesystem/PVRGUIDirectory.cpp
+++ b/xbmc/pvr/filesystem/PVRGUIDirectory.cpp
@@ -458,6 +458,109 @@ bool CPVRGUIDirectory::GetChannelGroupsDirectory(bool bRadio,
   return false;
 }
 
+namespace
+{
+std::shared_ptr<CPVRChannelGroupMember> GetLastWatchedChannelGroupMember(
+    const std::shared_ptr<CPVRChannel>& channel)
+{
+  const int lastGroupId{channel->LastWatchedGroupId()};
+  if (lastGroupId != PVR_GROUP_ID_UNNKOWN)
+  {
+    const std::shared_ptr<CPVRChannelGroup> lastGroup{
+        CServiceBroker::GetPVRManager().ChannelGroups()->GetByIdFromAll(lastGroupId)};
+    if (lastGroup && !lastGroup->IsHidden() && !lastGroup->IsDeleted())
+      return lastGroup->GetByUniqueID(channel->StorageId());
+  }
+  return {};
+}
+
+std::shared_ptr<CPVRChannelGroupMember> GetFirstMatchingGroupMember(
+    const std::shared_ptr<CPVRChannel>& channel)
+{
+  CPVRChannelGroups* groups{
+      CServiceBroker::GetPVRManager().ChannelGroups()->Get(channel->IsRadio())};
+  if (groups)
+  {
+    const std::vector<std::shared_ptr<CPVRChannelGroup>> channelGroups{
+        groups->GetMembers(true /* exclude hidden */)};
+
+    for (const auto& channelGroup : channelGroups)
+    {
+      if (channelGroup->IsDeleted())
+        continue;
+
+      const std::shared_ptr<CPVRChannelGroupMember> groupMember{
+          channelGroup->GetByUniqueID(channel->StorageId())};
+      if (groupMember)
+        return groupMember;
+    }
+  }
+  return {};
+}
+
+std::vector<std::shared_ptr<CPVRChannelGroupMember>> GetChannelGroupMembers(
+    const CPVRChannelsPath& path)
+{
+  const std::string& groupName{path.GetGroupName()};
+
+  std::shared_ptr<CPVRChannelGroup> group;
+  if (path.IsHiddenChannelGroup()) // hidden channels from the 'all channels' group
+  {
+    group = CServiceBroker::GetPVRManager().ChannelGroups()->GetGroupAll(path.IsRadio());
+  }
+  else if (groupName == "*") // all channels across all groups
+  {
+    group = CServiceBroker::GetPVRManager().ChannelGroups()->GetGroupAll(path.IsRadio());
+    if (group)
+    {
+      std::vector<std::shared_ptr<CPVRChannelGroupMember>> result;
+
+      const std::vector<std::shared_ptr<CPVRChannelGroupMember>> allGroupMembers{
+          group->GetMembers(CPVRChannelGroup::Include::ONLY_VISIBLE)};
+      for (const auto& allGroupMember : allGroupMembers)
+      {
+        std::shared_ptr<CPVRChannelGroupMember> member{
+            GetLastWatchedChannelGroupMember(allGroupMember->Channel())};
+        if (member)
+        {
+          result.emplace_back(member);
+          continue; // Process next 'All channels' group member.
+        }
+
+        if (group->IsHidden())
+        {
+          // Very special case. 'All channels' group is hidden. Let's see what we get iterating all
+          // non-hidden / non-deleted groups. We must not return any 'All channels' group members,
+          // because their path is invalid (it contains the group).
+          member = GetFirstMatchingGroupMember(allGroupMember->Channel());
+          if (member)
+            result.emplace_back(member);
+        }
+        else
+        {
+          // Use the 'All channels' group member.
+          result.emplace_back(allGroupMember);
+        }
+      }
+      return result;
+    }
+  }
+  else
+  {
+    group = CServiceBroker::GetPVRManager()
+                .ChannelGroups()
+                ->Get(path.IsRadio())
+                ->GetByName(groupName, path.GetGroupClientID());
+  }
+
+  if (group)
+    return group->GetMembers(CPVRChannelGroup::Include::ALL);
+
+  CLog::LogF(LOGERROR, "Unable to obtain members for channel group '{}'", groupName);
+  return {};
+}
+} // unnamed namespace
+
 bool CPVRGUIDirectory::GetChannelsDirectory(CFileItemList& results) const
 {
   const CPVRChannelsPath path(m_url.GetWithoutOptions());
@@ -487,46 +590,20 @@ bool CPVRGUIDirectory::GetChannelsDirectory(CFileItemList& results) const
     }
     else if (path.IsChannelGroup())
     {
-      const std::string& strGroupName = path.GetGroupName();
-      bool bShowHiddenChannels = path.IsHiddenChannelGroup();
-
-      std::shared_ptr<CPVRChannelGroup> group;
-      if (bShowHiddenChannels || strGroupName == "*") // all channels
+      const bool playedOnly{(m_url.HasOption("view") && (m_url.GetOption("view") == "lastplayed"))};
+      const bool showHiddenChannels{path.IsHiddenChannelGroup()};
+      const std::vector<std::shared_ptr<CPVRChannelGroupMember>> groupMembers{
+          GetChannelGroupMembers(path)};
+      for (const auto& groupMember : groupMembers)
       {
-        group = CServiceBroker::GetPVRManager().ChannelGroups()->GetGroupAll(path.IsRadio());
-      }
-      else
-      {
-        group = CServiceBroker::GetPVRManager()
-                    .ChannelGroups()
-                    ->Get(path.IsRadio())
-                    ->GetByName(strGroupName, path.GetGroupClientID());
-      }
-
-      if (group)
-      {
-        const bool playedOnly =
-            (m_url.HasOption("view") && (m_url.GetOption("view") == "lastplayed"));
-
-        const std::vector<std::shared_ptr<CPVRChannelGroupMember>> groupMembers =
-            group->GetMembers();
-        for (const auto& groupMember : groupMembers)
-        {
-          if (bShowHiddenChannels != groupMember->Channel()->IsHidden())
-            continue;
+        if (showHiddenChannels != groupMember->Channel()->IsHidden())
+          continue;
 
-          if (playedOnly && !groupMember->Channel()->LastWatched())
-            continue;
+        if (playedOnly && !groupMember->Channel()->LastWatched())
+          continue;
 
-          results.Add(std::make_shared<CFileItem>(groupMember));
-        }
+        results.Add(std::make_shared<CFileItem>(groupMember));
       }
-      else
-      {
-        CLog::LogF(LOGERROR, "Unable to obtain members of channel group '{}'", strGroupName);
-        return false;
-      }
-
       return true;
     }
   }