Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

in Files mode sorting by file ends up sorting by filename which leads to #1731

Open
wants to merge 2 commits into from

3 participants

@dragonflight

unexpected results when using DVD foders or movies in Folders.
while browsing folder movies the sort order will be (with the last two in random order)

movies/moviename3/avideo.avi
movies/moviename1/video.avi
movies/moviename2/video.avi

This patch fixes that and one gets the expected order of
movies/moviename1/video.avi
movies/moviename2/video.avi
movies/moviename3/avideo.avi
see http://forum.xbmc.org/showthread.php?tid=143834 for some discussion

I haven't investigated yet, but things are even stranger now as the name of the set the video belongs to has been appended to the filename so if the movie was part of a set it would sort by the set name

@dragonflight dragonflight in Files mode sorting by file ends up sorting by filename which leads to
unexpected results when using DVD foders or movies in Folders.
while browsing folder movies the sort order will be (with the last two in random order)

movies/moviename3/avideo.avi
movies/moviename1/video.avi
movies/moviename2/video.avi

This patch fixes that and one gets the expected order of
movies/moviename1/video.avi
movies/moviename2/video.avi
movies/moviename3/avideo.avi
see http://forum.xbmc.org/showthread.php?tid=143834 for some discussion
3506323
@Montellese Montellese was assigned
@jmarshallnz
Owner

This looks reasonable for database items, but not necessarily for non-database items with a videoinfotag set. Perhaps check if m_basePath is empty, and use m_strFileNameAndPath in that case?

@dragonflight

sorry, which non database items have videoinfo tags, so that I can look up what happens
thanks

@jmarshallnz
Owner

Myth, upnp etc - basically do a git grep for GetVideoInfoTag inside filesystem/

@dragonflight dragonflight in Files mode sorting by file (with a videoinfotag) ends up sorting b…
…y filename

which leads to unexpected results when using DVD folders or movies in Folders.
while browsing folder movies the sort order will be (with the last two in random order)

movies/moviename3/avideo.avi
movies/moviename1/video.avi
movies/moviename2/video.avi

This patch fixes that and one gets the expected order of
movies/moviename1/video.avi
movies/moviename2/video.avi
movies/moviename3/avideo.avi
see http://forum.xbmc.org/showthread.php?tid=143834 for some discussion
For non-database directories that don't set m_basepath (or even m_strFileNameAndPath)
the original item->m_strPath is left
6619406
@dragonflight

I checked all of the file systems and some (can't claim all) of the other uses of videotag and in particular I checked for references to m_strFileNameAndPath and as far as I can tell no-one except for PVRManager sets it, so I'm guessing the files view sorted by file looks somewhat surprising today.
In any case I have added a test for m_basePath and m_strFileNameAndPath being unset that might provide reasonable results in those cases.
It might be nice if we had someone who could test it.

@jmarshallnz
Owner

@Montellese This introduces a subtle (but correct?) change in behaviour in sorting that relies on FieldPath. Previously if there was a CFileItem whose m_videoInfoTag had empty m_strFileNameAndPath, then FieldPath would be empty, even though m_strPath in the CFileItem might be specified. The change in the second commit here will mean it'll fall back to m_strPath which I think is probably the preferred behaviour?

@Montellese
Owner

I don't really have a strong opinion on this as I never use files view. My only concern is that if we change this someone else will come running with a different use case where it doesn't work anymore and there's problably no way to satisfy both. Furthermore this change will not only affect SortByFile but also SortByPath where it is IMO crucial to get the full path to a file and not just the path to the folder containing the file.

@jmarshallnz
Owner

I see the issue with SortByFile and SortByPath changing. To get around this, perhaps we could just introduce a FieldSortFile or similar so that SortByFile does what the user thinks it should in these cases? Obv. would fall back to using FieldPath to grab it out otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 4, 2012
  1. @dragonflight

    in Files mode sorting by file ends up sorting by filename which leads to

    dragonflight authored
    unexpected results when using DVD foders or movies in Folders.
    while browsing folder movies the sort order will be (with the last two in random order)
    
    movies/moviename3/avideo.avi
    movies/moviename1/video.avi
    movies/moviename2/video.avi
    
    This patch fixes that and one gets the expected order of
    movies/moviename1/video.avi
    movies/moviename2/video.avi
    movies/moviename3/avideo.avi
    see http://forum.xbmc.org/showthread.php?tid=143834 for some discussion
Commits on Nov 5, 2012
  1. @dragonflight

    in Files mode sorting by file (with a videoinfotag) ends up sorting b…

    dragonflight authored
    …y filename
    
    which leads to unexpected results when using DVD folders or movies in Folders.
    while browsing folder movies the sort order will be (with the last two in random order)
    
    movies/moviename3/avideo.avi
    movies/moviename1/video.avi
    movies/moviename2/video.avi
    
    This patch fixes that and one gets the expected order of
    movies/moviename1/video.avi
    movies/moviename2/video.avi
    movies/moviename3/avideo.avi
    see http://forum.xbmc.org/showthread.php?tid=143834 for some discussion
    For non-database directories that don't set m_basepath (or even m_strFileNameAndPath)
    the original item->m_strPath is left
This page is out of date. Refresh to see the latest.
Showing with 4 additions and 1 deletion.
  1. +4 −1 xbmc/video/VideoInfoTag.cpp
View
5 xbmc/video/VideoInfoTag.cpp
@@ -503,7 +503,10 @@ void CVideoInfoTag::ToSortable(SortItem& sortable)
sortable[FieldTime] = m_strRuntime;
sortable[FieldFilename] = m_strFile;
sortable[FieldMPAA] = m_strMPAARating;
- sortable[FieldPath] = m_strFileNameAndPath;
+ if (!m_basePath.IsEmpty())
+ sortable[FieldPath] = m_basePath;
+ else if (!m_strFileNameAndPath.IsEmpty())
+ sortable[FieldPath] = m_strFileNameAndPath;
sortable[FieldSortTitle] = m_strSortTitle;
sortable[FieldTvShowStatus] = m_strStatus;
sortable[FieldProductionCode] = m_strProductionCode;
Something went wrong with that request. Please try again.