Update sbPropertyManager.cpp

[MarjonW]

Since all other SecondartSorts have disc no, THEN track no, I propose to use track no as the first SecondSort criterium for disc no as well. To me this makes sense: see https://dl.dropboxusercontent.com/u/5341072/Capture.JPG
FYI this is my first ever attempt at open source coding

[MarjonW]

Extra info:
when this request got status failed, I created another one, following the instructions "If building, use the sb-trunk-oldxul (development) branch, with the tag 1.12.1 tag for stable, for now. The master-xul-9.0.1 branch is the current progress in building Nightingale with XULRunner 9 and builds, but is broken."
That one failed as well and got closed as a double (#310)

[freaktechnik]

We can choose where we merge the pull request to, so that's no problem. I noticed the commit ID changed. The next time when porting a commit to another branch I suggest using git's cherrry-pick.
Regarding the contents of the pull I fully agree with it.

[freaktechnik]

For reference: https://s3.amazonaws.com/archive.travis-ci.org/jobs/30382788/log.txt
Or in words, this doesn't break any unit test.

[MarjonW]

Great, please would you merge then to 1.12.1? Or will that be overwritten again at some point by the stable version? I guess you know best where to merge it to :) Thanks in advance!

[rsjtdrjgfuzkfg]

While I agree with the content as well, I remember that Ezekiel000 had to clean his profile with his changes applied; does this work with "old" profiles?

[Ezekiel000]

I would think any changes to the sorting would only affect new files added to your library, which is why I had to remove all music in my library and rescan.

[rsjtdrjgfuzkfg]

From looking at the code, I would guess that it should work without re-importing media. My comment was mainly to bring up the point that there could be issues and the one merging it should verify it works before doing the actual merge.

[freaktechnik]

After doing a testbuild today, understanding what exactly this is about and then seeing that this patch indeed does not work for the existing libraries (probably need some db updating code), I came to the conclusion that I do not agree with this sorting. I think the current way makes a bit more sense, though it is useless for compilations. If we ever support compilations, those should postpone the artist to the lowest priority, so the track numbers inside actually match. Sadly compilations are the albums that normally contain multiple discs.

This patch would result in a situation with a similiar mess, as you would have all the track ones on disk one first and so on. That's confusing too.

I guess a workaround would be to sort by album artist, so you could unify compilations by setting an album artist. Actually an album artist SHOULD unify albums when not sorting by artist, in my opinion.

TL;DR: after really understanding what this wants to do I do not agree with it anymore.

[MarjonW]

Thanks for doing the test build!

I see your point about an album with multiple discs, but I think that any music player should support compilations.
And it should support a sort on track no, which currently is impossible. You get ALL the 1s, then ALL the 2s etcetera which is totally unuseable.

All I want to do is play back my old cassette mix tapes in the order that I remember them in.

Changing the 2nd sort for disc no to album artist->disc no->album->track no->track name would work for me personally. I could leave my album artists blank.
But for other people with the same issue, who do not know this, it is no solution.

I would still favour the second sort for disc no to be track no, I think that is the most logical thing and what users would expect.

Ezekiel000 and rsjtdrjgfuzkfg, what are your opinions?

[Ezekiel000]

The change makes sense to me but unless the problem with updating the database is sorted it can't be included in an official release as it would mess up everyone's library with any new additions to the library being sorted differently.

[MarjonW]

OK, thanks for your feedback! I don't know how to do that, so I will see how this goes and if anyone can make those adjustments

[rsjtdrjgfuzkfg]

@MarjonW I personally would prefer the sort order suggested in this thread, however it is not a big deal for me. I wouldn't agree with a database migration step for such a small issue, as I think migration steps should be hold to a minimum. I personally would thus not merge this at all, and prefer a customizeable sort order in the future (once somebody cleans up the db to support multiple tags, etc; we surely need a migration step for that). We can argue about the default orders, though.

[MarjonW]

Thanks for the feedback!
I would also prefer a customizeable sort order, and for this to get a high priority.
But as this has not happened so far I began my own attempt to better the situation.
The poor sorting possibilities have chased people away (https://getsatisfaction.com/songbird/topics/can_i_choose_how_to_sort_my_library). If it wasn't for the MLyrics add-on, I would have left Nightingale for Foobar 2000 a long time ago myself. Every time I use it it irritates me. So, not a small issue in my opinion.

[rsjtdrjgfuzkfg]

@MarjonW we do not have priorities, each developer may freely deceide which issues he/she wishes to target (as each patch is basically a donation of time). You could, however, add an issue with your feature request and place a bounty on it (that is, money any developer closing the issue will get, if we accept his/her fix). That might make it more likely that somebody works on it soon.

Or you could start to work on the customizeable sort order by yourself, I'm sure others would appreciate that :)

[freaktechnik]

As this has gone off topic anyways here my further thoughts about sorting:
I guess I don't even care about the actual sort order of sorting by disc number, as I only sort by album. This gives me the perfect sort, each distinct album (merged by album artist), sorted by disc number then track number.

I guess if you really want to have all track ones from your disc ones it's only possible by using for example a smart playlist right now and you'd have to change playlist for each disk song position.

Also, we do support compilations, just not as simple as for example iTunes does. iTunes has redundant functionality by having bot the Compilation checkbox and the Album Artist checkbox, though from a metadata representation standpoint it's better to say "this album has no album artist, but totally is an union" than to assign a random, or just meanlingless album artist. Hoverver the Album Artist field, when containing the same value for all songs of a compilation acts like iTune's compilation checkbox. Further it allows you to have multiple compilations with the same album name, as you can differentiate them by album artist. But in turn your tags aren't 100% representative of the actual song.

[MarjonW]

Thanks

I guess it won't happen any time soon then

I will keep on using 2 players

[johnmurrayvi]

Quick thought about existing libraries and changes like this: we should be able write a migration handler? But if both the current sort option and an additional sort (like this one) were given as a choice, it seems painful to have to re-run the migration handler each time it was changed.

[rsjtdrjgfuzkfg]

@johnmurrayvi migration handlers exist afaik, and are invoked on profile updates as they should. We did not yet need to use them, though. My comment on customizeability included a complete restructuring of the database (supporting multiple values per tag, etc); if we do that we should prepare other sort orders to be used imho (even if uncommon combinations won't get an db-level index if we decide to keep them static).

[freaktechnik]

Yup, there's those: https://github.com/nightingale-media-player/nightingale-hacking/tree/sb-trunk-oldxul/components/library/localdatabase/src/migrationhandlers

[MarjonW]

Regarding the cleaning of the db, if there is a simple bulk job that needs doing and if some one could give me a few pointers on that, then I could help out

[johnmurrayvi]

(Replying now so I don't forget to do this) I think I remember seeing some things that could be useful. I'll need to try to find them.