Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

GroupUtils: fix URL options when grouping movies into sets #1969

Merged
merged 1 commit into from

4 participants

@Montellese
Owner

This fixes a bug in movie smartplaylists with rules limiting the matching movies. If a rule matches some but not all of the movies in a set created with GroupUtils::Group() and the user opens the set all the movies belonging to that set will be listed and not just the ones matching the smartplaylist rules. That is because we don't append any videodb:// URL options to the URL used for the set. By passing in the original lists URL to GroupUtils::Group() we can extract any URL options (like the smartplaylist definition in "xsp") and also add it to the URL of the dynamically created movie sets.

This bug has been introduced with the move from doing the set grouping in the db to doing the set grouping dynamically outside of the database. Would be great if it can go in for Frodo, but it's not a blocker.

@jmarshallnz
Owner

Looks OK to me - up you you as to whether you merge it.

@Montellese
Owner

@davilla is the decision maker. Without this movie sets in a smartplaylist won't always behave 100% as expected.

@davilla

+1, inject it

@davilla davilla merged commit 31843b4 into xbmc:master
@Montellese Montellese deleted the Montellese:set_grouping_fix branch
@Voyager1
Collaborator

the sets now all get a path like "videodb://1/7/2/?setid=2", with options. As a side effect of this (and a major regression from RC2), new sets don't get any art (poster) loaded. I commented out the line in GroupUtils that adds the options, just to verify and indeed that's it. I guess the fix must be made somewhere in the art handling routines as to ignore any url options.

@Voyager1
Collaborator

Update: forget this for now. I don't think it's related. After deleting the sets and recreating them, it worked fine with the new code. Perhaps it was a glitch in my art table, who knows... I'll keep looking.
Update2: it was the art table indeed. Old entries for new set ids caused this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
22 xbmc/utils/GroupUtils.cpp
@@ -25,6 +25,7 @@
#include "FileItem.h"
#include "utils/StringUtils.h"
#include "utils/Variant.h"
+#include "video/VideoDbUrl.h"
#include "video/VideoInfoTag.h"
#include "utils/URIUtils.h"
#include "filesystem/MultiPathDirectory.h"
@@ -33,7 +34,7 @@ using namespace std;
typedef map<int, set<CFileItemPtr> > SetMap;
-bool GroupUtils::Group(GroupBy groupBy, const CFileItemList &items, CFileItemList &groupedItems, GroupAttribute groupAttributes /* = GroupAttributeNone */)
+bool GroupUtils::Group(GroupBy groupBy, const std::string &baseDir, const CFileItemList &items, CFileItemList &groupedItems, GroupAttribute groupAttributes /* = GroupAttributeNone */)
{
if (items.Size() <= 0 || groupBy == GroupByNone)
return false;
@@ -58,6 +59,10 @@ bool GroupUtils::Group(GroupBy groupBy, const CFileItemList &items, CFileItemLis
if ((groupBy & GroupBySet) && setMap.size() > 0)
{
+ CVideoDbUrl itemsUrl;
+ if (!itemsUrl.FromString(baseDir))
+ return false;
+
for (SetMap::const_iterator set = setMap.begin(); set != setMap.end(); set++)
{
// only one item in the set, so just re-add it
@@ -70,7 +75,16 @@ bool GroupUtils::Group(GroupBy groupBy, const CFileItemList &items, CFileItemLis
CFileItemPtr pItem(new CFileItem((*set->second.begin())->GetVideoInfoTag()->m_strSet));
pItem->GetVideoInfoTag()->m_iDbId = set->first;
pItem->GetVideoInfoTag()->m_type = "set";
- pItem->SetPath(StringUtils::Format("videodb://1/7/%ld/", set->first));
+
+ std::string basePath = StringUtils::Format("videodb://1/7/%ld/", set->first);
+ CVideoDbUrl videoUrl;
+ if (!videoUrl.FromString(basePath))
+ pItem->SetPath(basePath);
+ else
+ {
+ videoUrl.AddOptions(itemsUrl.GetOptionsString());
+ pItem->SetPath(videoUrl.ToString());
+ }
pItem->m_bIsFolder = true;
CVideoInfoTag* setInfo = pItem->GetVideoInfoTag();
@@ -132,9 +146,9 @@ bool GroupUtils::Group(GroupBy groupBy, const CFileItemList &items, CFileItemLis
return true;
}
-bool GroupUtils::GroupAndSort(GroupBy groupBy, const CFileItemList &items, const SortDescription &sortDescription, CFileItemList &groupedItems, GroupAttribute groupAttributes /* = GroupAttributeNone */)
+bool GroupUtils::GroupAndSort(GroupBy groupBy, const std::string &baseDir, const CFileItemList &items, const SortDescription &sortDescription, CFileItemList &groupedItems, GroupAttribute groupAttributes /* = GroupAttributeNone */)
{
- if (!Group(groupBy, items, groupedItems, groupAttributes))
+ if (!Group(groupBy, baseDir, items, groupedItems, groupAttributes))
return false;
groupedItems.Sort(sortDescription);
View
4 xbmc/utils/GroupUtils.h
@@ -37,6 +37,6 @@ typedef enum {
class GroupUtils
{
public:
- static bool Group(GroupBy groupBy, const CFileItemList &items, CFileItemList &groupedItems, GroupAttribute groupAttributes = GroupAttributeNone);
- static bool GroupAndSort(GroupBy groupBy, const CFileItemList &items, const SortDescription &sortDescription, CFileItemList &groupedItems, GroupAttribute groupAttributes = GroupAttributeNone);
+ static bool Group(GroupBy groupBy, const std::string &baseDir, const CFileItemList &items, CFileItemList &groupedItems, GroupAttribute groupAttributes = GroupAttributeNone);
+ static bool GroupAndSort(GroupBy groupBy, const std::string &baseDir, const CFileItemList &items, const SortDescription &sortDescription, CFileItemList &groupedItems, GroupAttribute groupAttributes = GroupAttributeNone);
};
View
2  xbmc/video/VideoDatabase.cpp
@@ -4953,7 +4953,7 @@ bool CVideoDatabase::GetSetsByWhere(const CStdString& strBaseDir, const Filter &
return false;
CFileItemList sets;
- if (!GroupUtils::Group(GroupBySet, items, sets))
+ if (!GroupUtils::Group(GroupBySet, strBaseDir, items, sets))
return false;
items.ClearItems();
View
2  xbmc/video/windows/GUIWindowVideoBase.cpp
@@ -1884,7 +1884,7 @@ void CGUIWindowVideoBase::GetGroupedItems(CFileItemList &items)
g_guiSettings.GetBool("videolibrary.groupmoviesets"))
{
CFileItemList groupedItems;
- if (GroupUtils::Group(GroupBySet, items, groupedItems, GroupAttributeIgnoreSingleItems))
+ if (GroupUtils::Group(GroupBySet, m_strFilterPath, items, groupedItems, GroupAttributeIgnoreSingleItems))
{
items.ClearItems();
items.Append(groupedItems);
Something went wrong with that request. Please try again.