Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Stacking and scanning changes for remote sources #1264

Closed
wants to merge 1 commit into from

2 participants

dez-dk jmarshallnz
dez-dk

This pull request is based on the feedback I got from my last pull request (VideoInfoScanner slightly optimized).

The goal is to make (re-)scanning of remote sources (such as HTTP) feasible especially by enabling the use of fast hashing.

XBMC only uses fast hashing if it doesn't find any subdirectories in a movie directory.

  • This pull request makes it possible to use stacking on HTTP sources. This hides CD-subdirs when scanning and enables fast hashing of multi-CD directories.
  • The CanFastHash() method of VideoInfoScanner is modified to exclude subdirs that are not scannable (matches exclude regexps). This makes it possible to fast hash dirs that contain subtitles directories, sample directories etc.

In the wake of these changes the following changes emerges:

  • The Exists() calls to check for DVD-files can be quite slow so this pull request adds the advancedsettings <stackdvds> that lets the user disable stacking of DVDs. Stacking of DVDs is enabled by default.
  • The advancedsetting <usesimplestacking> makes stacking easier for the end user. It doesn't rely on filenames that start with a (movie) title. The filenames are just concatenated and there are no regexps to configure. It results in some less nice looking filenames but when scanning you do not see the filename anyway. It is disabled by default.

Other changes:

  • VideoInfoScanner uses a FileItem so we can use the date we already have in memory instead of doing a stat() call for hashing etc.
dez-dk Enables stacking and optimizes scanning on HTTP
Enables stacking on HTTP sources.
Adds the advancedsettings <usesimplestacking> and <stackdvds>.
Modifies the VideoInfoScanner to use fast hashing when a directory doesn't
contain any scannable directories.
8130936
jmarshallnz

split out the actual change from the cosmetic (indenting)

jmarshallnz

unrequired cosmetic

jmarshallnz

This just stacks everything inside a folder??

It's basically a copy/paste of the original stack method without the regex checking/grouping. I believe the FileItemList only contains video-files, no?

Right, which only applies to the situation where you have a single movie in a folder that is across multiple files that are named so badly that a regexp won't apply? IMO it's not acceptable to include this sort of thing in mainline, even when guarded by yet another advancedsetting.

I honestly believe the regex system is flawed. In the least I should be able define regexes with named groups so that it would be possible to handle files names that do not begin with the movie title (eg. cd1-movie_title). But even if that was possible I would have to define several regexes to what end? This is simpler and easier.

It's simpler and easier except where you have a folder with more than one movie in it. With this setting enabled you can now no longer scan said folder correctly at all.

If the default regexps don't match something that obviously should be matched, then improving the regexps is the better solution.

The shortcomings of simple file stacking would of course be documented in the WIKI so that users would be aware of this before enabling the setting.

Neither solution is perfect.

What if the simple file stacking is only used if the user has chosen the "Movies are in separate folders"-option in the scanner? This would be passed as a bool (which defaults to false) to the Stack() method.

That way users with many movies in a singly dir will not be harmed by the change and users who have movies in separate dirs will have an easier time stacking. We get rid of the advancedsetting and the setting is now on a per dir basis meaning users can have both multi-movie and single-movie directories.

jmarshallnz

This change is OK - please pull it out as a separate commit. Note that if the user alters the regexps after a scan we may miss out on anything that was previously excluded.

jmarshallnz

In general, doing the cosmetics (indent) separately makes the change more obvious in a diff.

jmarshallnz

Looks like you're relying on m_dateTime of this filePtr to be invalid below. It might be useful to intentionally invalidate it here?

I do not understand this comment.

jmarshallnz
Owner

Please split out the various changes into separate commits, as we'll likely take some but not others.

jmarshallnz
Owner

@dez-dk Any chance you're going to revisit this?

dez-dk

@jmarshallnz I have been busy with other things. What parts is that you want? And I'm not sure how I split the changes up now that I have shared them on Github.

jmarshallnz
Owner

Assuming your commit is on top of your branch:

git reset HEAD~1
git add -p files

just add the bits that are related to a single thing (e.g. the skipping of sample folders in CanFastHash), then commit the results. Then repeat the git add -p, git commit until everything is nicely contained in separate commits that do one thing and one thing only.

That way we can more easily review + pull in the bits we want.

dez-dk

I haven't looked at the code for a couple of months but it seems some of the changes depend on each other. I don't have the time to abstract these dependencies away and test it.

It seems the only change you are interested in is the change that excludes subdirs when fast-hashing. I'd rather just do another PR with that change and nothing else--when I have some free time.

dez-dk dez-dk closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 8, 2012
  1. Enables stacking and optimizes scanning on HTTP

    dez-dk authored
    Enables stacking on HTTP sources.
    Adds the advancedsettings <usesimplestacking> and <stackdvds>.
    Modifies the VideoInfoScanner to use fast hashing when a directory doesn't
    contain any scannable directories.
This page is out of date. Refresh to see the latest.
129 xbmc/FileItem.cpp
View
@@ -816,6 +816,11 @@ bool CFileItem::IsAfp() const
return URIUtils::IsAfp(m_strPath);
}
+bool CFileItem::IsHttp() const
+{
+ return URIUtils::IsHttp(m_strPath);
+}
+
bool CFileItem::IsOnLAN() const
{
return URIUtils::IsOnLAN(m_strPath);
@@ -2000,7 +2005,12 @@ void CFileItemList::Stack(bool stackFiles /* = true */)
StackFolders();
if (stackFiles)
- StackFiles();
+ {
+ if (g_advancedSettings.m_useSimpleStacking)
+ StackFilesSimple();
+ else
+ StackFiles();
+ }
}
void CFileItemList::StackFolders()
@@ -2031,10 +2041,12 @@ void CFileItemList::StackFolders()
// only check known fast sources?
// NOTES:
// 1. rars and zips may be on slow sources? is this supposed to be allowed?
- if( !item->IsRemote()
+ if(
+ !item->IsRemote()
|| item->IsSmb()
|| item->IsNfs()
|| item->IsAfp()
+ || item->IsHttp()
|| URIUtils::IsInRAR(item->GetPath())
|| URIUtils::IsInZIP(item->GetPath())
|| URIUtils::IsOnLAN(item->GetPath())
@@ -2076,45 +2088,48 @@ void CFileItemList::StackFolders()
}
// check for dvd folders
- if (!bMatch)
+ if (g_advancedSettings.m_stackDvds)
{
- CStdString path;
- CStdString dvdPath;
- URIUtils::AddFileToFolder(item->GetPath(), "VIDEO_TS.IFO", path);
- if (CFile::Exists(path))
- dvdPath = path;
- else
+ if (!bMatch)
{
- URIUtils::AddFileToFolder(item->GetPath(), "VIDEO_TS", dvdPath);
- URIUtils::AddFileToFolder(dvdPath, "VIDEO_TS.IFO", path);
- dvdPath.Empty();
- if (CFile::Exists(path))
- dvdPath = path;
- }
-#ifdef HAVE_LIBBLURAY
- if (dvdPath.IsEmpty())
- {
- URIUtils::AddFileToFolder(item->GetPath(), "index.bdmv", path);
+ CStdString path;
+ CStdString dvdPath;
+ URIUtils::AddFileToFolder(item->GetPath(), "VIDEO_TS.IFO", path);
if (CFile::Exists(path))
dvdPath = path;
else
{
- URIUtils::AddFileToFolder(item->GetPath(), "BDMV", dvdPath);
- URIUtils::AddFileToFolder(dvdPath, "index.bdmv", path);
+ URIUtils::AddFileToFolder(item->GetPath(), "VIDEO_TS", dvdPath);
+ URIUtils::AddFileToFolder(dvdPath, "VIDEO_TS.IFO", path);
dvdPath.Empty();
if (CFile::Exists(path))
dvdPath = path;
}
- }
+#ifdef HAVE_LIBBLURAY
+ if (dvdPath.IsEmpty())
+ {
+ URIUtils::AddFileToFolder(item->GetPath(), "index.bdmv", path);
+ if (CFile::Exists(path))
+ dvdPath = path;
+ else
+ {
+ URIUtils::AddFileToFolder(item->GetPath(), "BDMV", dvdPath);
+ URIUtils::AddFileToFolder(dvdPath, "index.bdmv", path);
+ dvdPath.Empty();
+ if (CFile::Exists(path))
+ dvdPath = path;
+ }
+ }
#endif
- if (!dvdPath.IsEmpty())
- {
- // NOTE: should this be done for the CD# folders too?
- item->m_bIsFolder = false;
- item->SetPath(dvdPath);
- item->SetLabel2("");
- item->SetLabelPreformated(true);
- m_sortMethod = SORT_METHOD_NONE; /* sorting is now broken */
+ if (!dvdPath.IsEmpty())
+ {
+ // NOTE: should this be done for the CD# folders too?
+ item->m_bIsFolder = false;
+ item->SetPath(dvdPath);
+ item->SetLabel2("");
+ item->SetLabelPreformated(true);
+ m_sortMethod = SORT_METHOD_NONE; /* sorting is now broken */
+ }
}
}
}
@@ -2300,6 +2315,60 @@ void CFileItemList::StackFiles()
}
}
+void CFileItemList::StackFilesSimple()
+{
+ int64_t size = 0;
+ vector<int> stack;
+ CStdString stackName = "stack";
+
+ int i = 0;
+ while (i < Size())
+ {
+ CFileItemPtr item1 = Get(i);
+
+ // skip folders, nfo files, playlists
+ if (item1->m_bIsFolder
+ || item1->IsParentFolder()
+ || item1->IsNFO()
+ || item1->IsPlayList()
+ )
+ {
+ // increment index
+ i++;
+ continue;
+ }
+
+ stackName = stackName + "_" + item1->GetLabel();
+ size += item1->m_dwSize;
+ stack.push_back(i);
+
+ i++;
+ }
+
+ if (stack.size() > 1)
+ {
+ CFileItemPtr item1 = Get(stack[0]);
+
+ // dont actually stack a multipart rar set, just remove all items but the first
+ CStdString stackPath;
+ if (Get(stack[0])->IsRAR())
+ stackPath = Get(stack[0])->GetPath();
+ else
+ {
+ CStackDirectory dir;
+ stackPath = dir.ConstructStackPath(*this, stack);
+ }
+ item1->SetPath(stackPath);
+
+ // clean up list
+ for (unsigned k = stack.size(); k > 0; --k)
+ Remove(stack[k]);
+
+ item1->SetLabel(stackName);
+ item1->m_dwSize = size;
+ }
+}
+
bool CFileItemList::Load(int windowID)
{
CFile file;
2  xbmc/FileItem.h
View
@@ -124,6 +124,7 @@ class CFileItem :
bool IsHD() const;
bool IsNfs() const;
bool IsAfp() const;
+ bool IsHttp() const;
bool IsRemote() const;
bool IsSmb() const;
bool IsURL() const;
@@ -469,6 +470,7 @@ class CFileItemList : public CFileItem
\sa Stack
*/
void StackFiles();
+ void StackFilesSimple();
/*!
\brief stack folders in a CFileItemList
14 xbmc/settings/AdvancedSettings.cpp
View
@@ -148,6 +148,10 @@ void CAdvancedSettings::Initialize()
m_moviesExcludeFromScanRegExps.push_back("[!-._ \\\\/]sample[-._ \\\\/]");
m_tvshowExcludeFromScanRegExps.push_back("[!-._ \\\\/]sample[-._ \\\\/]");
+ m_useSimpleStacking = false;
+
+ m_stackDvds = true;
+
m_folderStackRegExps.push_back("((cd|dvd|dis[ck])[0-9]+)$");
m_videoStackRegExps.push_back("(.*?)([ _.-]*(?:cd|dvd|p(?:(?:ar)?t)|dis[ck]|d)[ _.-]*[0-9]+)(.*?)(\\.[^.]+)$");
@@ -821,6 +825,16 @@ void CAdvancedSettings::ParseSettingsFile(const CStdString &file)
m_trailerMatchRegExps.begin(),
m_trailerMatchRegExps.end());
+ XMLUtils::GetBoolean(pRootElement, "usesimplestacking", m_useSimpleStacking);
+
+ if (m_useSimpleStacking)
+ CLog::Log(LOGDEBUG, "Simple file stacking enabled.");
+
+ XMLUtils::GetBoolean(pRootElement, "stackdvds", m_stackDvds);
+
+ if (!m_stackDvds)
+ CLog::Log(LOGDEBUG, "Dvd stacking disabled.");
+
// video stacking regexps
TiXmlElement* pVideoStacking = pRootElement->FirstChildElement("moviestacking");
if (pVideoStacking)
2  xbmc/settings/AdvancedSettings.h
View
@@ -198,6 +198,8 @@ class CAdvancedSettings
CStdStringArray m_audioExcludeFromListingRegExps;
CStdStringArray m_audioExcludeFromScanRegExps;
CStdStringArray m_pictureExcludeFromListingRegExps;
+ bool m_useSimpleStacking;
+ bool m_stackDvds;
CStdStringArray m_videoStackRegExps;
CStdStringArray m_folderStackRegExps;
CStdStringArray m_trailerMatchRegExps;
10 xbmc/utils/URIUtils.cpp
View
@@ -778,6 +778,16 @@ bool URIUtils::IsAfp(const CStdString& strFile)
return strFile2.Left(4).Equals("afp:");
}
+bool URIUtils::IsHttp(const CStdString& strFile)
+{
+ CStdString strFile2(strFile);
+
+ if (IsStack(strFile))
+ strFile2 = CStackDirectory::GetFirstStackedFile(strFile);
+
+ return strFile2.Left(5).Equals("http:") || strFile2.Left(6).Equals("https:");
+}
+
bool URIUtils::IsVideoDb(const CStdString& strFile)
{
1  xbmc/utils/URIUtils.h
View
@@ -73,6 +73,7 @@ class URIUtils
static bool IsMythTV(const CStdString& strFile);
static bool IsNfs(const CStdString& strFile);
static bool IsAfp(const CStdString& strFile);
+ static bool IsHttp(const CStdString& strFile);
static bool IsOnDVD(const CStdString& strFile);
static bool IsOnLAN(const CStdString& strFile);
static bool IsPlugin(const CStdString& strFile);
64 xbmc/video/VideoInfoScanner.cpp
View
@@ -193,6 +193,14 @@ namespace VIDEO
bool CVideoInfoScanner::DoScan(const CStdString& strDirectory)
{
+ CFileItemPtr filePtr = CFileItemPtr(new CFileItem(strDirectory, true));
+ return DoScan(filePtr);
+ }
+
+ bool CVideoInfoScanner::DoScan(const CFileItemPtr& filePtr)
+ {
+ CStdString strDirectory = filePtr->GetPath();
+
if (m_pObserver)
{
m_pObserver->OnDirectoryChanged(strDirectory);
@@ -234,7 +242,7 @@ namespace VIDEO
if (m_pObserver)
m_pObserver->OnStateChanged(content == CONTENT_MOVIES ? FETCHING_MOVIE_INFO : FETCHING_MUSICVIDEO_INFO);
- CStdString fastHash = GetFastHash(strDirectory);
+ CStdString fastHash = GetFastHash(filePtr);
if (m_database.GetPathHash(strDirectory, dbHash) && !fastHash.IsEmpty() && fastHash == dbHash)
{ // fast hashes match - no need to process anything
CLog::Log(LOGDEBUG, "VideoInfoScanner: Skipping dir '%s' due to no change (fasthash)", strDirectory.c_str());
@@ -268,7 +276,7 @@ namespace VIDEO
m_pObserver->OnDirectoryScanned(strDirectory);
}
// update the hash to a fast hash if needed
- if (CanFastHash(items) && !fastHash.IsEmpty())
+ if (CanFastHash(items, content) && !fastHash.IsEmpty())
hash = fastHash;
}
}
@@ -337,7 +345,7 @@ namespace VIDEO
// do not recurse for tv shows - we have already looked recursively for episodes
if (pItem->m_bIsFolder && !pItem->IsParentFolder() && !pItem->IsPlayList() && settings.recurse > 0 && content != CONTENT_TVSHOWS)
{
- if (!DoScan(pItem->GetPath()))
+ if (!DoScan(pItem))
{
m_bStop = true;
}
@@ -1542,32 +1550,46 @@ namespace VIDEO
return count;
}
- bool CVideoInfoScanner::CanFastHash(const CFileItemList &items) const
+ bool CVideoInfoScanner::CanFastHash(const CFileItemList &items, const CONTENT_TYPE content) const
{
- // TODO: Probably should account for excluded folders here (eg samples), though that then
- // introduces possible problems if the user then changes the exclude regexps and
- // expects excluded folders that are inside a fast-hashed folder to then be picked
- // up. The chances that the user has a folder which contains only excluded folders
- // where some of those folders should be scanned recursively is pretty small.
- return items.GetFolderCount() == 0;
+ // exclude folders that match our exclude regexps
+ CStdStringArray regexps = content == CONTENT_TVSHOWS ? g_advancedSettings.m_tvshowExcludeFromScanRegExps
+ : g_advancedSettings.m_moviesExcludeFromScanRegExps;
+
+ for (int i = 0; i < items.Size(); i++)
+ {
+ CFileItemPtr pItem = items[i];
+
+ if (pItem->m_bIsFolder && !CUtil::ExcludeFileOrFolder(pItem->GetPath(), regexps))
+ return false;
+ }
+ return true;
}
- CStdString CVideoInfoScanner::GetFastHash(const CStdString &directory) const
+ CStdString CVideoInfoScanner::GetFastHash(const CFileItemPtr& filePtr) const
{
- struct __stat64 buffer;
- if (XFILE::CFile::Stat(directory, &buffer) == 0)
+ CStdString hash = "";
+ if (filePtr->m_dateTime.IsValid())
+ {
+ time_t timet;
+ filePtr->m_dateTime.GetAsTime(timet);
+ hash.Format("fast%"PRIu32, timet);
+ }
+ else
{
- int64_t time = buffer.st_mtime;
- if (!time)
- time = buffer.st_ctime;
- if (time)
+ struct __stat64 buffer;
+ if (XFILE::CFile::Stat(filePtr->GetPath(), &buffer) == 0)
{
- CStdString hash;
- hash.Format("fast%"PRId64, time);
- return hash;
+ int64_t time = buffer.st_mtime;
+ if (!time)
+ time = buffer.st_ctime;
+ if (time)
+ {
+ hash.Format("fast%"PRId64, time);
+ }
}
}
- return "";
+ return hash;
}
void CVideoInfoScanner::GetSeasonThumbs(const CVideoInfoTag &show, map<int, string> &art, bool useLocal)
5 xbmc/video/VideoInfoScanner.h
View
@@ -137,6 +137,7 @@ namespace VIDEO
protected:
virtual void Process();
bool DoScan(const CStdString& strDirectory);
+ bool DoScan(const CFileItemPtr& filePtr);
INFO_RET RetrieveInfoForTvShow(CFileItemPtr pItem, bool bDirNames, ADDON::ScraperPtr &scraper, bool useLocal, CScraperUrl* pURL, bool fetchEpisodes, CGUIDialogProgress* pDlgProgress);
INFO_RET RetrieveInfoForMovie(CFileItemPtr pItem, bool bDirNames, ADDON::ScraperPtr &scraper, bool useLocal, CScraperUrl* pURL, CGUIDialogProgress* pDlgProgress);
@@ -202,7 +203,7 @@ namespace VIDEO
\param directory folder to hash
\return the hash of the folder of the form "fast<datetime>"
*/
- CStdString GetFastHash(const CStdString &directory) const;
+ CStdString GetFastHash(const CFileItemPtr &filePtr) const;
/*! \brief Decide whether a folder listing could use the "fast" hash
Fast hashing can be done whenever the folder contains no scannable subfolders, as the
@@ -211,7 +212,7 @@ namespace VIDEO
\param items the directory listing
\return true if this directory listing can be fast hashed, false otherwise
*/
- bool CanFastHash(const CFileItemList &items) const;
+ bool CanFastHash(const CFileItemList &items, static CONTENT_TYPE content) const;
/*! \brief Process a series folder, filling in episode details and adding them to the database.
TODO: Ideally we would return INFO_HAVE_ALREADY if we don't have to update any episodes
Something went wrong with that request. Please try again.