View Full Version : Music Library, Albums, Sort by Artist + Date
Following from this thread: http://xbmc.org/forum/showthread.php?t=23574&page=2
I've just tested some changes that seem to work well for me, resulting in this view:
http://www.illhostit.net/files/443/screenshot027.jpg
(http://www.illhostit.net)
As you can see, all albums for an artist are arranged in chronological order, with the year displayed. Every record, cassette, and cd collection I've had I've arranged on the shelf this way.
This mod was done by editing:
SortFileItem.cpp - SortArtistAscending
bool SSortFileItem::SongArtistAscending(CFileItem *left, CFileItem *right)
{
// special cases
if (left->IsParentFolder()) return true;
if (right->IsParentFolder()) return false;
// only if they're both folders or both files do we do the full comparison
if (left->m_bIsFolder == right->m_bIsFolder)
{
char *l = (char *)left->m_musicInfoTag.GetArtist().c_str();
char *r = (char *)right->m_musicInfoTag.GetArtist().c_str();
int result = StringUtils::AlphaNumericCompare(l, r);
if (result < 0) return true;
if (result > 0) return false;
// begin edit ------------------------------------------------------
//gb 20061212: comment out the secondary album sort...
// artists agree, test the album
//l = (char *)left->m_musicInfoTag.GetAlbum().c_str();
//r = (char *)right->m_musicInfoTag.GetAlbum().c_str();
//result = StringUtils::AlphaNumericCompare(l, r);
//if (result < 0) return true;
//if (result > 0) return false;
//...and sort by album date (I'm not even sure where this gets populated...)
SYSTEMTIME ldateTime;
SYSTEMTIME rdateTime;
left->m_musicInfoTag.GetReleaseDate(ldateTime);
right->m_musicInfoTag.GetReleaseDate(rdateTime);
return ldateTime.wYear <= rdateTime.wYear;
// end edit ------------------------------------------------------
// artist and album agree, test the track number
return left->m_musicInfoTag.GetTrackAndDiskNumber() <= right->m_musicInfoTag.GetTrackAndDiskNumber();
}
return left->m_bIsFolder;
}
and similarly for SortArtistAscendingNoThe, and the corresponding Descending functions.
and for the display, I replaced "%B" with "%Y - %B" in GUIViewStateMusic - CGUIViewStateMusicDatabase:
case NODE_TYPE_ALBUM:
{
if (g_guiSettings.GetBool("filelists.ignorethewhensorting"))
AddSortMethod(SORT_METHOD_ALBUM_IGNORE_THE, 270, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
else
AddSortMethod(SORT_METHOD_ALBUM, 270, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
if (g_guiSettings.GetBool("filelists.ignorethewhensorting"))
AddSortMethod(SORT_METHOD_ARTIST_IGNORE_THE, 269, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
else
AddSortMethod(SORT_METHOD_ARTIST, 269, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
SetSortMethod(g_stSettings.m_MyMusicNavAlbumsSortM ethod);
It seems to work. I'll do a little more work to try to figure out how to make this an advancedsettings.xml option. Can anybody else who finds this useful, and can compile, try this? If it continues to perform properly, I'd like to see it get into the svn (as an advancedsetting, of course. I know some people don't need this type of sorting).
Thanks!
p.s.
Can somebody give me a pointer to creating an advancedsetting??
The proper thing to do is to add a second "sort by" control to the musicnav window so that its user selectable based on the current node and primary sort method. example:
at the "artists" listing, the second sort is empty and unselectable.
at an "albums" listing, you would have these primary -> secondary sort options:
album title -> artist
album title -> album year
artist -> album title
artist -> album year
album year -> album title
(album year -> artist seems like a wierd combination to me.)
and then there could be something similar for a songs listing.
(this is something which has been on my mind for a while now. your earlier post reminded me of it.)
jmarshall
2006-12-14, 02:33
I'd do alphabetical after year + before track as well (2 albums in one year) as most tags can't refine better than 1 year.
Also, see if you can find out why GetYear() isn't working. It'll be populated from the GetAlbumFromDataset() routine most likely (MusicDatabase.cpp)
Cheers,
Jonathan
GetYear and GetReleaseDate refer to the same piece of tag information. If GetReleaseDate is working, then GetAlbumFromDataset is working correctly. I suspect the problem in that StringUtils::AlphaNumericCompare function. It does a wierd comparison when digits are found.
I'd do alphabetical after year + before track as well (2 albums in one year) as most tags can't refine better than 1 year.
yeah, I was just thinking that
Also, see if you can find out why GetYear() isn't working. It'll be populated from the GetAlbumFromDataset() routine most likely (MusicDatabase.cpp)
thanks. I did notice that what I used seemed to be geared towards videos, so maybe it's left out of music (but that releasedate seems to have worked perfectly for my entire collection)
also, kraqh3d, maybe year -> artist isn't so weird. e.g. end of year party, you've got a bunch of 2006 releases and you want to find Britney Spears' contribution...don't have a clue what it was called but you know you've got it. Makes sense to me. (in fact, to me, album year -> album title, or album title -> <anything> makes little sense! but that's a philosophical discussion.)
ok, I've made some changes, hopefully enough to act as a testbed or proof-of-concept for anybody who wants to try this:
added an advancedsetting:
<albumssortbyartistanddate>true</albumssortbyartistanddate>
and made these additions to the code:
----------------------------------------------------------------------------
settings.cpp
----------------------------------------------------------------------------
- csettings
added: g_advancedSettings.m_albumssortbyartistanddate = false;
- loadadvancedsettings
added: XMLUtils::GetBoolean(pRootElement, "albumssortbyartistanddate", g_advancedSettings.m_albumssortbyartistanddate);
----------------------------------------------------------------------------
settings.h
----------------------------------------------------------------------------
- struct advancedsettings
added: bool m_albumssortbyartistanddate;
----------------------------------------------------------------------------
sortfileitem.cpp
----------------------------------------------------------------------------
- songartistascending / descending / nothe
changed from:
// artists agree, test the album
to:
if (g_advancedSettings.m_albumssortbyartistanddate)
{
SYSTEMTIME ldateTime;
SYSTEMTIME rdateTime;
left->m_musicInfoTag.GetReleaseDate(ldateTime);
right->m_musicInfoTag.GetReleaseDate(rdateTime);
return ldateTime.wYear <= rdateTime.wYear;
}
// artists agree, test the album
----------------------------------------------------------------------------
guiviewstatemusic.cpp
----------------------------------------------------------------------------
- cguiviewstatemusicdatabase
changed "%B%" to "%Y% - B%" here:
case NODE_TYPE_ALBUM:
{
if (g_guiSettings.GetBool("filelists.ignorethewhensorting"))
AddSortMethod(SORT_METHOD_ALBUM_IGNORE_THE, 270, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
else
AddSortMethod(SORT_METHOD_ALBUM, 270, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
if (g_guiSettings.GetBool("filelists.ignorethewhensorting"))
AddSortMethod(SORT_METHOD_ARTIST_IGNORE_THE, 269, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
else
AddSortMethod(SORT_METHOD_ARTIST, 269, LABEL_MASKS("%F", "", "%Y - %B", "%A")); // Filename, empty | Album, Artist
Ignore all my above code postings - I've cleaned things up and submitted a patch (first-timer, I hope everything's in order). Tracker ID 1618284
SVN default behaviours are not changed, everything is via advancedsettings.
In Library-Albums, Sort by Artist, the patch will secondary-sort by year, and tertiary-sort by album name (as jmarshall pointed out was still needed).
in Library-Artists...select an artist...Sort by Artist will list albums in date order; Sort by Album will list in alpha order.
Another option allows configuration of the Album display, i.e. svn hardcodes "%B", my option allows user defined e.g. "%B (%Y)" or "%Y - %B". As implemented, this only affect the display in the Library-Albums and Library-Artists...Albums views as discussed above.
IM(not so humble)O, this is the only way to browse a large music collection. I hope my patch is simple enough to find its way into the svn. I think it adds enough options to keep most people happy. Of course, as discussed, various new sort options ideally will find their way into the gui.
cheers. i want default behaviour modified as well, as i totally agree with you.
however i don't see this in entering the gui. most users won't give a damn, we should just find sensible defaults and keep it advanced.
advancedsettings should really be reserved for those configuration options which remain static and dont need to change in a given "session".
as it stands, this just scratches the surface. its nothing more than hack which changes one of the currently static sorting methods. (and i have no problem with that. this is the beauty of opensource software. everyone has the freedom to change how xbmc works.)
however, there is alot of potential in this. and if grows and expands to account for more than just two options, it should really be changed to a live in-gui option. i can think of alot of reasons how this could be useful, especially when at a "songs" list. (otherwise you'd have to reboot to resort, and thats sort of silly.)
and i have to look into why GetYear isnt working. its really bugging me. it *should* work. the code that creates the text for the info labels uses it. and it really only returns the wYear portion of the release date as a string. (if its less than 1900, it returns a blank string.)
also... this would inherently effect songs listing when sorting by artist as well, so you may want to change the label there too.
so kraqh, you envision several sort by buttons? sounds ugly to me. i'm ofc open for suggestions, but this would have to be done in a very slick way, or it will end up being awk and confusing for newbs.
and i have to look into why GetYear isnt working. its really bugging me. it *should* work. the code that creates the text for the info labels uses it. and it really only returns the wYear portion of the release date as a string. (if its less than 1900, it returns a blank string.)
yeah, I couldn't figure it out, but then again I couldn't even figure out how to write a value to the debug log without crashing. (That's probably a fairly good indicator that I won't be the guy making extensive gui options - at least, don't hold your breath).
you prob gave a cstdstring as a %s option, you need to use .c_str() to get the char*
yes, youre correct that it gets ugly. this is why i'd say it would be best to limit it to two levels of sorting.
this could be done today with the current "sort by" button. instead of it saying just "Artist", "Album", etc, it could say "Artist->Album", "Artist->Year", "Genre->Year". a while back i played around with this. the biggest problem was that the text was too wide.
so just kind of thinking outloud, i can picture two sort buttons: "sort by:" and "then by:". and i would expect that the options available in the "then by" button would be dependant on the "sort by" button. (ie, a secondary sort really only applies to an album list or a songs list.)
jmarshall
2006-12-20, 00:33
I dislike having 2 buttons. Instead, we should look at techniques to compress the text (which is the only real problem).
One option is to have a popup menu with the sort (triggered via the Sort button) or a select button. This solves the long text issue, but has the disadvantage of not easily showing the current sort mode.
We really need to think about just when this will be useful though - if it's only going to be useful in a few views, and only then really a one or the other choice, then cluttering the GUI with it is not such a good option.
Cheers,
Jonathan
How about making a button with more height, using an over/under scheme. Text would look like this....
Artist
-Album
you prob gave a cstdstring as a %s option, you need to use .c_str() to get the char*
yeah something like that...the only c++ I know is what I can copy from other parts of the code (I was primarily a VB6 programmer). anyway, GetYear does work in the sort, and I did it like this:
(the commented stuff is what I had before - both methods seem to work)
// artists agree, test the date
if (g_advancedSettings.m_albumssortbyartistanddate)
{
//SYSTEMTIME ldateTime;
//SYSTEMTIME rdateTime;
//left->m_musicInfoTag.GetReleaseDate(ldateTime);
//right->m_musicInfoTag.GetReleaseDate(rdateTime);
//if (ldateTime.wYear < rdateTime.wYear) return true;
//if (ldateTime.wYear > rdateTime.wYear) return false;
CStdString l = left->m_musicInfoTag.GetYear();
CStdString r = right->m_musicInfoTag.GetYear();
CLog::Log(LOGNOTICE,"year %s > %s?", l.c_str(), r.c_str());
int result = StringUtils::AlphaNumericCompare(l, r);
if (result < 0) return true;
if (result > 0) return false;
}
ythe only c++ I know is what I can copy from other parts of the code
...and I'm an idiot! can't even do that right - now I see this in SSortFileItem::MovieYearAscending
int result = StringUtils::AlphaNumericCompare(left->m_musicInfoTag.GetYear().c_str(), right->m_musicInfoTag.GetYear().c_str());
I just converted the old "Hide *All Items" and "Sort *All Items on Bottom" options into proper advanced settings options, and then added the "Hide Compilation Artists" option which only shows primary album artists in the unfiltered artists list. (Nice option by the way. I never realized how many random artists my soundtracks and other VA albums add to the list.) I can also incorporate this option (as-is for now).
I'm also playing with removing the "Unknown" tags from the database. This was originally done because the virtual paths used by the music library were text based. (like this: "Genre/Unknown/Artist/Foo/Album/Bar".) Now that the path uses database id's, its not required anymore and a NULL string would be fine. The listing will still fill in the word "Unknown" as the label for a genre, artist, or album, but the song tags wont inherit it (unless of course they were actually tagged as such.)
good stuff, can't wait for the hide compilation artist option :)
gergtreble
2006-12-21, 00:09
Are these modifications ever going to be included in an official release?
Or am I gonna have to learn how to program?
Eitherway, keep up the good work lads!
i just committed those options i spoke of earlier and updated the wiki... this album sorting option is next on my list. (you'll need to wait for the next T3CH release, or build your own xbe)
if anybody is playing along, I just updated the patch on the tracker to work with the svn as of now.
... this album sorting option is next on my list.
I see the secondary sort by year got committed (thanks, that'll be great), but it wasn't implemented in the "ignorethe" parts. That's the way I had it in the patch I've been using, and it all worked great.
(SongArtistAscendingNoThe, ...DescendingNoThe)
Thanks! :D
...and, now that I've really tried it, I don't think the svn version works. See my post #17 where I stumbled upon this code from sortmovieascending, that's what finally worked for me:
int result = StringUtils::AlphaNumericCompare(left->m_musicInfoTag.GetYear().c_str(), right->m_musicInfoTag.GetYear().c_str());
thanks, i missed that... i dont use "ingore the"... but other than that, its working fine for me... i actually left my xbmc configured as such.
** edit **
the you posted and the code in svn are the same... what you posted is a short cut.
i'm dumping my db and rescanning again to see if theres any effect.
** edit 2 **
i appologize, you are correct. its not working. the funny thing is that on my first page of ablums sorted by artist, by chance all the albums listed by date are the same exact order when listed by album title... jumping down a few pages showed the problem.
this is bugging me. that should work. ill correct it shortly. (and i already added in the code for the "ignore the" sections so both will be fixed.)
one more thing... i just noticed that "(Year)" doesnt cope well when there's no year tag. you just get blank parens because the naming code isnt as robust in determining if something was done. Putting the year on the end with nothing after it will work, ie "Album - Year", or "Album, Year". What would be best?
one more thing... i just noticed that "(Year)" doesnt cope well when there's no year tag. you just get blank parens because the naming code isnt as robust in determining if something was done. Putting the year on the end with nothing after it will work, ie "Album - Year", or "Album, Year". What would be best?
- yeah, I agree, () does look weird
- what would be best? user-configurable :p
- if year is blank and people are bugged by having (), maybe that'll motivate them to tag properly... (I've only got like 3/1000 albums with unknown year)
- special handling to test the Year value before the label is changed?
I still like Album (Year) because it looks clean, but I suppose Album - Year would be a good second choice. (or third choice, with user-config first :p )
After further testing, it seems something is broken in the label parser. "Album - Year" and "Year - Album" have a problem as well. It used to account for missing tags. I try to look into that later to see whats up.
I'm now going to add advanced settings for album format left and right so that its configurable and it'll always be used. Making each sort type configurable to use a different format adds way too much complexity. So every album listing (albums, recent albums, top 100 albums), regardless of sort will use this new format tag.
And I still have no idea why the other way doesnt work. It winds up deleting the strings causing the comparison on two null strings.
sorry about that... i just updated svn with a corrected version which includes user configurable album naming templates via advanced settings.xml. the naming template is used universally when at an albums listing in the library.
i also corrected the bug with the template parser, and added a little hack to the "unknown" album. when a song is found without an album tag, it gets filed into the unknown album. previously, the unknown album had a null artist string. now it displays "various artists" but it will still not show up as a compilation album.
Sweet. This is nice to have, thanks.