make mpd use a reaonable Fisher-Yates shuffle for randomizing play order

instead of an incorrectly implemented Knuth shuffle. Also on openbsd
make sure we use arc4random_uniform for the random choice from the
sequence to make sure we aren't modulo biased.
Noticed when my mpd server played an unnatural amount of Rush out of the
80's playlist.
This commit is contained in:
beck 2009-04-10 00:52:07 +00:00
parent 0e45c641b1
commit 63131ebe8a
2 changed files with 82 additions and 2 deletions

View File

@ -1,8 +1,8 @@
# $OpenBSD: Makefile,v 1.16 2008/10/28 15:21:48 naddy Exp $
# $OpenBSD: Makefile,v 1.17 2009/04/10 00:52:07 beck Exp $
COMMENT= Music Player Daemon
DISTNAME= mpd-0.13.2
PKGNAME= ${DISTNAME}p2
PKGNAME= ${DISTNAME}p3
CATEGORIES= audio
HOMEPAGE= http://www.musicpd.org/
MAINTAINER= Tobias Ulmer <tobiasu@tmux.org>

View File

@ -0,0 +1,80 @@
--- src/playlist.c.orig Fri Jun 13 22:16:16 2008
+++ src/playlist.c Mon Apr 6 08:57:25 2009
@@ -77,6 +77,13 @@
static int playPlaylistOrderNumber(int fd, int orderNum);
static void randomizeOrder(int start, int end);
+#ifdef __OpenBSD__
+#define RANDOM_UB(n) (arc4random_uniform(n))
+#else
+#define RANDOM_UB(n) (random() % (n))
+#endif
+
+
static void incrPlaylistVersion(void)
{
static unsigned long max = ((mpd_uint32) 1 << 31) - 1;
@@ -647,7 +654,7 @@
else
start = playlist.current + 1;
if (start < playlist.length) {
- swap = random() % (playlist.length - start);
+ swap = RANDOM_UB(playlist.length - start);
swap += start;
swapOrder(playlist.length - 1, swap);
}
@@ -1189,15 +1196,23 @@
}
}
- for (i = start; i <= end; i++) {
- ri = random() % (end - start + 1) + start;
- if (ri == playlist.current)
- playlist.current = i;
- else if (i == playlist.current)
- playlist.current = ri;
- swapOrder(i, ri);
+ /*
+ * Shuffle the Order.
+ * Use an unbiased Fisher-Yates shuffle.
+ */
+ i = end + 1;
+ while (i > start + 1) {
+ ri = RANDOM_UB(i - start); /* 0 <= ri <= len */
+ ri += start;
+ i--; /* i is now the last pertinent index */
+ if (i != ri) { /* do nothing if i == ri */
+ if (ri == playlist.current)
+ playlist.current = i;
+ else if (i == playlist.current)
+ playlist.current = ri;
+ swapOrder(i, ri);
+ }
}
-
}
int setPlaylistRandomStatus(int fd, int status)
@@ -1281,12 +1296,17 @@
i = 0;
playlist.current = -1;
}
- /* shuffle the rest of the list */
- for (; i < playlist.length; i++) {
- ri = random() % (playlist.length - 1) + 1;
- swapSongs(i, ri);
+ /*
+ * shuffle the rest of the list
+ * Use an unbiased Fisher-Yates shuffle.
+ */
+ i = playlist.length;
+ while (i > 1) {
+ ri = RANDOM_UB(i); /* 0 <= ri <= len */
+ i--; /* i is now the last pertinent index */
+ if (i != ri) /* do nothing if i == ri */
+ swapSongs(i, ri);
}
-
incrPlaylistVersion();
}