core/playlist: Implement playlist_next() directly

Let's have the playlist generic functions pick the next track rather
than redirecting to the queue code.

Signed-off-by: Anna Schumaker <Anna@NoWheyCreamery.com>

core/playlist.c

@@ -149,21 +149,15 @@ bool playlist_select(struct playlist *playlist)
 
 struct track *playlist_next(void)
 {
-	struct track *track;
-
-	if (!current)
-		return NULL;
-
-	track = current->pl_ops->pl_next(current);
-	if (track)
+	struct track *track = playlist_generic_next(current);
+	if (track && current->pl_type < PL_MAX_TYPE)
 		playlist_types[current->pl_type]->pl_save();
 	return track;
 }
 
 struct track *playlist_prev(void)
 {
-	struct playlist *history = playlist_lookup(PL_SYSTEM, "History");
-	return history->pl_ops->pl_next(history);
+	return playlist_generic_next(playlist_lookup(PL_SYSTEM, "History"));
 }
 
 bool playlist_add(struct playlist *playlist, struct track *track)
@@ -210,7 +204,8 @@ void playlist_set_random(struct playlist *playlist, bool enabled)
 {
 	if (playlist && playlist->pl_ops->pl_set_flag) {
 		playlist->pl_ops->pl_set_flag(playlist, Q_RANDOM, enabled);
-		playlist_types[playlist->pl_type]->pl_save();
+		if (playlist->pl_type < PL_MAX_TYPE)
+			playlist_types[playlist->pl_type]->pl_save();
 	}
 }
 

core/playlists/artist.c

@@ -10,7 +10,6 @@ static struct file       artist_file = FILE_INIT("playlist.artist", 0);
 
 static struct playlist_ops pl_artist_ops = {
 	.pl_can_select = playlist_generic_can_select,
-	.pl_next       = playlist_generic_next,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
 };

core/playlists/generic.c

@@ -195,5 +195,22 @@ void playlist_generic_resort(struct playlist *playlist)
 
 struct track *playlist_generic_next(struct playlist *playlist)
 {
-	return queue_next(&playlist->pl_queue);
+	unsigned int pos, size = playlist ? queue_size(&playlist->pl_queue) : 0;
+
+	if (size == 0)
+		return NULL;
+	else if (queue_has_flag(&playlist->pl_queue, Q_RANDOM)) {
+		pos = g_random_int_range(1, queue_size(&playlist->pl_queue));
+		pos += playlist->pl_queue.q_cur.it_pos;
+		queue_iter_set(&playlist->pl_queue,
+			       &playlist->pl_queue.q_cur, pos % size);
+	} else {
+		queue_iter_next(&playlist->pl_queue.q_cur);
+		if (!playlist->pl_queue.q_cur.it_iter)
+			queue_iter_set(&playlist->pl_queue,
+				       &playlist->pl_queue.q_cur, 0);
+	}
+
+	return queue_selected(&playlist->pl_queue,
+			       playlist->pl_queue.q_cur.it_pos);
 }

core/playlists/library.c

@@ -193,7 +193,6 @@ static bool pl_library_delete(struct playlist *playlist)
 static struct playlist_ops pl_library_ops = {
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = pl_library_delete,
-	.pl_next       = playlist_generic_next,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
 };

core/playlists/system.c

@@ -79,7 +79,6 @@ static struct playlist_ops favorites_ops = {
 	.pl_add        = playlist_generic_add_track,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = sys_pl_delete_clear,
-	.pl_next       = playlist_generic_next,
 	.pl_remove     = playlist_generic_remove_track,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
@@ -130,7 +129,6 @@ static struct playlist_ops hidden_ops = {
 	.pl_add        = sys_pl_hidden_add,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = sys_pl_hidden_clear,
-	.pl_next       = playlist_generic_next,
 	.pl_remove     = sys_pl_hidden_remove,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
@@ -167,7 +165,6 @@ static struct playlist_ops queued_ops = {
 	.pl_add        = playlist_generic_add_track,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = sys_pl_delete_clear,
-	.pl_next       = playlist_generic_next,
 	.pl_remove     = playlist_generic_remove_track,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
@@ -192,7 +189,6 @@ static bool sys_pl_collection_load()
 
 static struct playlist_ops collection_ops = {
 	.pl_can_select = playlist_generic_can_select,
-	.pl_next       = playlist_generic_next,
 	.pl_remove     = sys_pl_hidden_add,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
@@ -211,7 +207,6 @@ static bool sys_pl_history_add(struct playlist *playlist, struct track *track)
 
 static struct playlist_ops history_ops = {
 	.pl_add  = sys_pl_history_add,
-	.pl_next = playlist_generic_next,
 };
 
 
@@ -220,7 +215,6 @@ static struct playlist_ops history_ops = {
  */
 static struct playlist_ops dynamic_ops = {
 	.pl_can_select = playlist_generic_can_select,
-	.pl_next       = playlist_generic_next,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,
 };

core/playlists/user.c

@@ -80,7 +80,6 @@ static struct playlist_ops user_ops = {
 	.pl_add        = playlist_generic_add_track,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = pl_user_delete,
-	.pl_next       = playlist_generic_next,
 	.pl_remove     = playlist_generic_remove_track,
 	.pl_set_flag   = playlist_generic_set_flag,
 	.pl_sort       = playlist_generic_sort,

core/queue.c

@@ -225,24 +225,3 @@ struct track *queue_selected(struct queue *queue, unsigned int index)
 		queue_iter_set(queue, &queue->q_cur, index);
 	return __queue_selected(queue, index);
 }
-
-struct track *queue_next(struct queue *queue)
-{
-	unsigned int pos, size = queue_size(queue);
-
-	if (size == 0)
-		return NULL;
-	else if (size == 1)
-		queue_iter_set(queue, &queue->q_cur, 0);
-	else if (queue_has_flag(queue, Q_RANDOM)) {
-		pos = g_random_int_range(1, (size < 15) ? size : size / 3);
-		pos += queue->q_cur.it_pos;
-		queue_iter_set(queue, &queue->q_cur, pos % size);
-	} else {
-		queue_iter_next(&queue->q_cur);
-		if (!queue->q_cur.it_iter)
-			queue_iter_set(queue, &queue->q_cur, 0);
-	}
-
-	return __queue_selected(queue, queue->q_cur.it_pos);
-}

include/core/playlists/playlist.h

@@ -28,9 +28,6 @@ struct playlist_ops {
 	/* Called to delete a playlist. */
 	bool (*pl_delete)(struct playlist *);
 
-	/* Called to pick the next track from a playlist. */
-	struct track *(*pl_next)(struct playlist *);
-
 	/* Called to remove a track from the playlist. */
 	bool (*pl_remove)(struct playlist *, struct track *);
 

include/core/queue.h

@@ -164,7 +164,4 @@ void queue_updated(struct queue *, struct track *);
 /* Called to tell the queue that a specific index has been selected. */
 struct track *queue_selected(struct queue *, unsigned int);
 
-/* Called to pick the next track from the queue. */
-struct track *queue_next(struct queue *);
-
 #endif /* OCARINA_CORE_QUEUE_H */

tests/core/playlist.c

@@ -13,7 +13,9 @@ static void test_pl_sorted(struct playlist *playlist)
 
 static struct playlist_ops test_noop;
 static struct playlist_ops test_ops = {
-	.pl_sort = playlist_generic_sort,
+	.pl_can_select = playlist_generic_can_select,
+	.pl_set_flag   = playlist_generic_set_flag,
+	.pl_sort       = playlist_generic_sort,
 };
 static struct playlist_callbacks test_cb = {
 	.pl_cb_sorted = test_pl_sorted,
@@ -32,6 +34,7 @@ static void test_null()
 	g_assert(playlist_current() == playlist_lookup(PL_SYSTEM, "Collection"));
 	g_assert_false(playlist_select(NULL));
 	g_assert(playlist_current() == playlist_lookup(PL_SYSTEM, "Collection"));
+	g_assert_null(playlist_next());
 	playlist_selected(NULL);
 	playlist_played(NULL);
 
@@ -116,6 +119,44 @@ static void test_sorting()
 	}
 }
 
+static void test_next()
+{
+	struct playlist p = DEFINE_PLAYLIST(PL_MAX_TYPE, "Test", 0, &test_ops);
+	struct track *track;
+	unsigned int i;
+
+	g_random_set_seed(0);
+	playlist_generic_init(&p, 0, NULL);
+
+	for (i = 0; i < 13; i++)
+		playlist_generic_add_track(&p, track_get(i));
+
+	g_assert_true(playlist_select(&p));
+	g_assert(playlist_current() == &p);
+	g_assert_cmpuint(p.pl_queue.q_cur.it_pos, ==, UINT_MAX);
+
+	/* Test playlist_next() with wraparound, but no random. */
+	for (i = 0; i < (13 * 2); i++) {
+		track = playlist_next();
+		g_assert_nonnull(track);
+		g_assert_cmpuint(track->tr_track, ==, (i % 13) + 1);
+		g_assert_cmpuint(playlist_size(&p), ==, 13);
+	}
+
+	playlist_set_random(&p, true);
+	g_assert_true(playlist_get_random(&p));
+
+	/* rand() = { 10, 4, 6, 1, 8, 4, 1 } */
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 9);
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 13);
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 6);
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 7);
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 2);
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 6);
+	g_assert_cmpuint(playlist_next()->tr_track, ==, 1);
+	g_assert_cmpuint(playlist_size(&p), ==, 13);
+}
+
 static void test_save_load()
 {
 	struct playlist p = DEFINE_PLAYLIST(PL_MAX_TYPE, "Test",   0, &test_ops);
@@ -190,6 +231,7 @@ int main(int argc, char **argv)
 	g_test_init(&argc, &argv, NULL);
 	g_test_add_func("/Core/Playlist/NULL", test_null);
 	g_test_add_func("/Core/Playlists/Sorting", test_sorting);
+	g_test_add_func("/Core/Playlists/Next Track", test_next);
 	g_test_add_func("/Core/Playlist/Save and Load", test_save_load);
 	ret = g_test_run();
 

tests/core/queue.c

@@ -71,7 +71,6 @@ static void test_init()
 	g_assert_cmpuint(q.q_length, ==, 0);
 	g_assert_null(q.q_sort);
 	g_assert_null(q.q_ops);
-	g_assert_null(queue_next(&q));
 
 	queue_iter_init(&q, &it);
 	g_assert_null(it.it_iter);
@@ -90,7 +89,6 @@ static void test_init()
 	g_assert_cmpuint(q.q_length, ==, 0);
 	g_assert_null(q.q_sort);
 	g_assert(q.q_ops == &test_ops);
-	g_assert_null(queue_next(&q));
 
 	queue_deinit(&q);
 	g_assert_cmpuint(count_deinit, ==, 1);
@@ -191,10 +189,8 @@ static void test_queue(gconstpointer arg)
 
 	/* Tracks should not be removed. */
 	for (i = 0; i < ex_size; i++) {
-		g_assert(queue_next(&q) == track_get((i % 11) + 2));
-		g_assert_cmpuint(count_updated, ==, (N / 13) + (2 * i) + 1);
 		queue_selected(&q, i);
-		g_assert_cmpuint(count_updated, ==, (N / 13) + (2 * i) + 2);
+		g_assert_cmpuint(count_updated,  ==, (N / 13) + (i + 1));
 		g_assert_cmpuint(queue_size(&q), ==, ex_size);
 	}
 
@@ -216,12 +212,6 @@ static void test_rand_select()
 	g_random_set_seed(0);
 	queue_init(&q, Q_RANDOM, &test_ops, NULL);
 
-	/* Call next() on an empty queue. */
-	for (i = 0; i < 13; i++) {
-		g_assert_null(queue_next(&q));
-		g_assert_cmpuint(queue_size(&q), ==, 0);
-	}
-
 	q.q_sort = g_slist_append(q.q_sort, GINT_TO_POINTER(COMPARE_TRACK));
 	for (i = 0; i < 13; i++)
 		queue_add(&q, track_get(i));
@@ -233,21 +223,12 @@ static void test_rand_select()
 	 * 	[val]: The value picked by q.next().
 	 */
 
-	/* rand() = 10, track = 9 */
-	g_assert_cmpuint(queue_next(&q)->tr_track, ==, 9);
-
 	/* select = 6, track = 7 */
 	g_assert_cmpuint(queue_selected(&q, 6)->tr_track, ==, 7);
 
-	/* rand() = 4, track = 11 */
-	g_assert_cmpuint(queue_next(&q)->tr_track, ==, 11);
-
 	/* select = 7, track = 8 */
 	g_assert_cmpuint(queue_selected(&q, 7)->tr_track, ==, 8);
 
-	/* rand() = 6, track = 1 */
-	g_assert_cmpuint(queue_next(&q)->tr_track, ==, 1);
-
 	/* select = 2, track = 3 */
 	g_assert_cmpuint(queue_selected(&q, 2)->tr_track, ==, 3);