PDA

View Full Version : PATCH: Sort by DISC before TRACK



Caleb Epstein
2003-12-16, 06:52
OK, as promised here is a patch to Slim/Music/Info.pm which
adds some additional logic to the low-level sort routines.
The function getInfoForSort returns a new element which is the
disc number for a given file. The functions sortByTrackAlg
and sortByAlbumAlg have been modified to check this extra
field and sort by it before sorting on track numbers.

This provides the ability to group multiple discs together in
a single ALBUM and have them sorted properly.

Index: Slim/Music/Info.pm
================================================== =================
RCS file: /cvsroot/slim/server/Slim/Music/Info.pm,v
retrieving revision 1.38
diff -u -b -r1.38 Info.pm
--- Slim/Music/Info.pm 15 Dec 2003 22:46:50 -0000 1.38
+++ Slim/Music/Info.pm 16 Dec 2003 13:48:45 -0000
@@ -1500,7 +1500,8 @@
,artistSort($item)
,albumSort($item)
,trackNumber($item)
- ,titleSort($item)];
+ ,titleSort($item)
+ ,disc($item)];
}

#algorithm for sorting by Artist, Album, Track
@@ -1543,6 +1544,12 @@
return 1;
}

+ # compare discs
+ if (defined $j->[6] && defined $k->[6]) {
+ $result = $j->[6] <=> $k->[6];
+ return $result if $result;
+ }
+
#compare track numbers
if ($j->[4] && $k->[4]) {
$result = $j->[4] <=> $k->[4];
@@ -1587,6 +1594,12 @@
return 1;
}

+ # compare discs
+ if (defined $j->[6] && defined $k->[6]) {
+ $result = $j->[6] <=> $k->[6];
+ return $result if $result;
+ }
+
#compare track numbers
if ($j->[4] && $k->[4]) {
$result = $j->[4] <=> $k->[4];


--
Caleb Epstein | bklyn . org | QOTD:
cae at | Brooklyn Dust | "I only touch base with reality on an
bklyn dot org | Bunny Mfg. | as-needed basis!"

dean
2003-12-16, 09:05
I'm a little confused by this. Since we munge the album name to
include the disk number, shouldn't this be unnecessary?


On Dec 16, 2003, at 5:52 AM, Caleb Epstein wrote:

>
> OK, as promised here is a patch to Slim/Music/Info.pm which
> adds some additional logic to the low-level sort routines.
> The function getInfoForSort returns a new element which is the
> disc number for a given file. The functions sortByTrackAlg
> and sortByAlbumAlg have been modified to check this extra
> field and sort by it before sorting on track numbers.
>
> This provides the ability to group multiple discs together in
> a single ALBUM and have them sorted properly.
>
> Index: Slim/Music/Info.pm
> ================================================== =================
> RCS file: /cvsroot/slim/server/Slim/Music/Info.pm,v
> retrieving revision 1.38
> diff -u -b -r1.38 Info.pm
> --- Slim/Music/Info.pm 15 Dec 2003 22:46:50 -0000 1.38
> +++ Slim/Music/Info.pm 16 Dec 2003 13:48:45 -0000
> @@ -1500,7 +1500,8 @@
> ,artistSort($item)
> ,albumSort($item)
> ,trackNumber($item)
> - ,titleSort($item)];
> + ,titleSort($item)
> + ,disc($item)];
> }
>
> #algorithm for sorting by Artist, Album, Track
> @@ -1543,6 +1544,12 @@
> return 1;
> }
>
> + # compare discs
> + if (defined $j->[6] && defined $k->[6]) {
> + $result = $j->[6] <=> $k->[6];
> + return $result if $result;
> + }
> +
> #compare track numbers
> if ($j->[4] && $k->[4]) {
> $result = $j->[4] <=> $k->[4];
> @@ -1587,6 +1594,12 @@
> return 1;
> }
>
> + # compare discs
> + if (defined $j->[6] && defined $k->[6]) {
> + $result = $j->[6] <=> $k->[6];
> + return $result if $result;
> + }
> +
> #compare track numbers
> if ($j->[4] && $k->[4]) {
> $result = $j->[4] <=> $k->[4];
>
>
> --
> Caleb Epstein | bklyn . org | QOTD:
> cae at | Brooklyn Dust | "I only touch base with reality
> on an
> bklyn dot org | Bunny Mfg. | as-needed basis!"
>

Caleb Epstein
2003-12-16, 09:22
On Tue, Dec 16, 2003 at 08:05:27AM -0800, dean blackketter wrote:

> I'm a little confused by this. Since we munge the album name to
> include the disk number, shouldn't this be unnecessary?

Well, I take issue with the code that adds the " (Disc N)" or
" (Disc N of M)" to the album name. I think it unnecessarily
fragments the album into separate pieces, and this makes
navigation unnecessarily difficult when you have a lot of
multi-disc sets from a single artist.

Also, the " (Disc N of M)" logic only kicks in if the "SET"
tag appears on a track. If the underlying format passes up
the "DISC" tag without using "SET" (as I have done with my
most recent change to Slim/Formats/FLAC.pm), this logic is
bypassed and all the tracks wind up in the same album where
they belong IMHO. Then you need this sorting patch to ensure
that all of the tracks in the multi-disc album are sorted in
the proper order (e.g. all tracks on D1 before all tracks on
D2 ...)

I would recommend that this patch be incorporated and the
logic which adds " (Disc N of M)" to the album name be
removed. Either that or this behavior be user-configurable.

--
Caleb Epstein | bklyn . org |
cae at | Brooklyn Dust | Why are you so hard to ignore?
bklyn dot org | Bunny Mfg. |

dean
2003-12-16, 18:09
Ok, I get it now.

Can you make an updated patch that creates an option to add that string
to the album name, along with this change?


On Dec 16, 2003, at 8:22 AM, Caleb Epstein wrote:

> On Tue, Dec 16, 2003 at 08:05:27AM -0800, dean blackketter wrote:
>
>> I'm a little confused by this. Since we munge the album name to
>> include the disk number, shouldn't this be unnecessary?
>
> Well, I take issue with the code that adds the " (Disc N)" or
> " (Disc N of M)" to the album name. I think it unnecessarily
> fragments the album into separate pieces, and this makes
> navigation unnecessarily difficult when you have a lot of
> multi-disc sets from a single artist.
>
> Also, the " (Disc N of M)" logic only kicks in if the "SET"
> tag appears on a track. If the underlying format passes up
> the "DISC" tag without using "SET" (as I have done with my
> most recent change to Slim/Formats/FLAC.pm), this logic is
> bypassed and all the tracks wind up in the same album where
> they belong IMHO. Then you need this sorting patch to ensure
> that all of the tracks in the multi-disc album are sorted in
> the proper order (e.g. all tracks on D1 before all tracks on
> D2 ...)
>
> I would recommend that this patch be incorporated and the
> logic which adds " (Disc N of M)" to the album name be
> removed. Either that or this behavior be user-configurable.
>
> --
> Caleb Epstein | bklyn . org |
> cae at | Brooklyn Dust | Why are you so hard to ignore?
> bklyn dot org | Bunny Mfg. |
>

Caleb Epstein
2003-12-17, 10:09
On Tue, Dec 16, 2003 at 05:09:29PM -0800, dean blackketter wrote:

> Ok, I get it now. Can you make an updated patch that creates an
> option to add that string to the album name, along with this change?

OK, working on it. Should the default behavior be to mangle
the ALBUM tag (e.g. current behavior) or not? With the
addition of proper sorting within a multi-disc album, I don't
see the need for keeping the current behavior, but I don't
want to suprise anyone.

FYI, the CVS server seems to be down at the moment:

cvs [update aborted]: connect to cvs.slimdevices.com(64.71.158.5):2401 failed: Connection refused

--
Caleb Epstein | bklyn . org | Keep emotionally active. Cater to your
cae at | Brooklyn Dust | favorite neurosis.
bklyn dot org | Bunny Mfg. |