From 2fb27178ee21d9a00a5bcf893da863da065666aa Mon Sep 17 00:00:00 2001
From: Anna Schumaker <Anna@NoWheyCreamery.com>
Date: Wed, 21 Sep 2016 10:44:06 -0400
Subject: [PATCH] core/playlist: Implement playlist_add() directly

Signed-off-by: Anna Schumaker <Anna@NoWheyCreamery.com>
---
 core/playlist.c                  |  2 +-
 core/playlists/artist.c          |  2 +-
 core/playlists/generic.c         | 21 ++++++++---
 core/playlists/library.c         |  2 +-
 core/playlists/system.c          | 14 ++++----
 core/playlists/user.c            |  2 +-
 core/queue.c                     | 60 --------------------------------
 gui/playlist.c                   |  6 ----
 include/core/playlists/generic.h |  2 +-
 include/core/queue.h             |  7 ----
 tests/core/playlist.c            | 32 ++++++++++++++---
 tests/core/queue.c               | 52 ---------------------------
 tests/gui/filter.c               |  3 --
 tests/gui/model.c                |  3 --
 14 files changed, 56 insertions(+), 152 deletions(-)

diff --git a/core/playlist.c b/core/playlist.c
index d3930b59..fd4e13cd 100644
--- a/core/playlist.c
+++ b/core/playlist.c
@@ -168,7 +168,7 @@ bool playlist_add(struct playlist *playlist, struct track *track)
 		return false;
 
 	ret = playlist->pl_ops->pl_add(playlist, track);
-	if (ret)
+	if (ret && playlist->pl_type < PL_MAX_TYPE)
 		playlist_types[playlist->pl_type]->pl_save();
 	if (playlist == playlist_lookup(PL_SYSTEM, "Queued Tracks"))
 		playlist_select(playlist);
diff --git a/core/playlists/artist.c b/core/playlists/artist.c
index 91685c23..3084918e 100644
--- a/core/playlists/artist.c
+++ b/core/playlists/artist.c
@@ -166,7 +166,7 @@ void pl_artist_new_track(struct track *track)
 		artist->ar_playlist = playlist;
 	}
 
-	playlist_generic_add_track(playlist, track);
+	playlist_generic_add(playlist, track);
 }
 
 void pl_artist_delete_track(struct track *track)
diff --git a/core/playlists/generic.c b/core/playlists/generic.c
index bc9780eb..e72599c2 100644
--- a/core/playlists/generic.c
+++ b/core/playlists/generic.c
@@ -115,7 +115,7 @@ void playlist_generic_load(struct playlist *playlist, struct file *file,
 		file_readf(file, "%u ", &n);
 		for (i = 0; i < n; i++) {
 			file_readf(file, "%u", &t);
-			queue_add(&playlist->pl_queue, track_get(t));
+			playlist_generic_add(playlist, track_get(t));
 		}
 		if (file_readf(file, "%m\n", &line))
 			g_free(line);
@@ -146,11 +146,24 @@ void playlist_generic_clear(struct playlist *playlist)
 		callbacks->pl_cb_removed(playlist, NULL, n);
 }
 
-bool playlist_generic_add_track(struct playlist *playlist, struct track *track)
+bool playlist_generic_add(struct playlist *playlist, struct track *track)
 {
-	if (playlist_has(playlist, track))
+	if (!playlist || !track || playlist_has(playlist, track))
 		return false;
-	queue_add(&playlist->pl_queue, track);
+
+	playlist->pl_queue.q_length += track->tr_length;
+	if (playlist->pl_queue.q_sort) {
+		g_queue_insert_sorted(&playlist->pl_queue.q_tracks, track,
+				      __playlist_generic_less_than,
+				      playlist->pl_queue.q_sort);
+		playlist->pl_queue.q_cur.it_pos = g_queue_link_index(
+						&playlist->pl_queue.q_tracks,
+						 playlist->pl_queue.q_cur.it_iter);
+	} else
+		g_queue_push_tail(&playlist->pl_queue.q_tracks, track);
+
+	if (callbacks)
+		callbacks->pl_cb_added(playlist, track);
 	return true;
 }
 
diff --git a/core/playlists/library.c b/core/playlists/library.c
index 1f36812f..02a46599 100644
--- a/core/playlists/library.c
+++ b/core/playlists/library.c
@@ -107,7 +107,7 @@ static void __lib_pl_read_path(struct scan_data *scan, const gchar *name)
 	else {
 		track = track_add(scan->sd_library, path);
 		if (track) {
-			queue_add(&playlist->pl_queue, track);
+			playlist_generic_add(playlist, track);
 			pl_system_new_track(track);
 			pl_artist_new_track(track);
 		}
diff --git a/core/playlists/system.c b/core/playlists/system.c
index b8bbd080..c9050f6d 100644
--- a/core/playlists/system.c
+++ b/core/playlists/system.c
@@ -76,7 +76,7 @@ static void sys_pl_update(struct playlist *playlist)
  * Favorite tracks playlist operations.
  */
 static struct playlist_ops favorites_ops = {
-	.pl_add        = playlist_generic_add_track,
+	.pl_add        = playlist_generic_add,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = sys_pl_delete_clear,
 	.pl_remove     = playlist_generic_remove,
@@ -90,7 +90,7 @@ static struct playlist_ops favorites_ops = {
  */
 static bool sys_pl_hidden_add(struct playlist *playlist, struct track *track)
 {
-	bool ret = playlist_generic_add_track(pl_system_get(SYS_PL_HIDDEN), track);
+	bool ret = playlist_generic_add(pl_system_get(SYS_PL_HIDDEN), track);
 	playlist_generic_remove(pl_system_get(SYS_PL_COLLECTION),     track);
 	playlist_generic_remove(pl_system_get(SYS_PL_UNPLAYED),       track);
 	playlist_generic_remove(pl_system_get(SYS_PL_MOST_PLAYED),    track);
@@ -107,8 +107,8 @@ static bool sys_pl_hidden_remove(struct playlist *playlist, struct track *track)
 	add_id = (track->tr_count == 0)      ? SYS_PL_UNPLAYED    : add_id;
 	add_id = (track->tr_count > average) ? SYS_PL_MOST_PLAYED : add_id;
 
-	playlist_generic_add_track(pl_system_get(SYS_PL_COLLECTION), track);
-	playlist_generic_add_track(pl_system_get(add_id),            track);
+	playlist_generic_add(pl_system_get(SYS_PL_COLLECTION), track);
+	playlist_generic_add(pl_system_get(add_id),            track);
 	return ret;
 }
 
@@ -162,7 +162,7 @@ static bool sys_pl_queued_load()
 }
 
 static struct playlist_ops queued_ops = {
-	.pl_add        = playlist_generic_add_track,
+	.pl_add        = playlist_generic_add,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = sys_pl_delete_clear,
 	.pl_remove     = playlist_generic_remove,
@@ -422,8 +422,8 @@ void pl_system_deinit()
 
 void pl_system_new_track(struct track *track)
 {
-	playlist_generic_add_track(pl_system_lookup("Collection"), track);
-	playlist_generic_add_track(pl_system_lookup("Unplayed"),   track);
+	playlist_generic_add(pl_system_lookup("Collection"), track);
+	playlist_generic_add(pl_system_lookup("Unplayed"),   track);
 }
 
 void pl_system_delete_track(struct track *track)
diff --git a/core/playlists/user.c b/core/playlists/user.c
index 2ad18678..da91647f 100644
--- a/core/playlists/user.c
+++ b/core/playlists/user.c
@@ -77,7 +77,7 @@ static bool pl_user_delete(struct playlist *playlist)
 
 
 static struct playlist_ops user_ops = {
-	.pl_add        = playlist_generic_add_track,
+	.pl_add        = playlist_generic_add,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_delete     = pl_user_delete,
 	.pl_remove     = playlist_generic_remove,
diff --git a/core/queue.c b/core/queue.c
index 36338ebe..e7c3cc1d 100644
--- a/core/queue.c
+++ b/core/queue.c
@@ -6,28 +6,6 @@
 #include <stdlib.h>
 
 
-static int track_less_than(const void *a, const void *b, void *data)
-{
-	struct track *lhs   = (struct track *)a;
-	struct track *rhs   = (struct track *)b;
-	GSList *cur = (GSList *)data;
-	int res, field;
-
-	while (cur) {
-		field = GPOINTER_TO_INT(cur->data);
-		if (field > 0)
-			res = track_compare(lhs, rhs, field);
-		else
-			res = track_compare(rhs, lhs, abs(field));
-		if (res == 0) {
-			cur = g_slist_next(cur);
-			continue;
-		}
-		break;
-	};
-	return res;
-}
-
 static inline void *__queue_init(struct queue *queue, void *data)
 {
 	if (queue->q_ops)
@@ -41,37 +19,6 @@ static inline void __queue_deinit(struct queue *queue)
 		queue->q_ops->qop_deinit(queue);
 }
 
-static inline unsigned int __queue_added(struct queue *queue,
-					 struct track *track,
-					 unsigned int pos)
-{
-	queue->q_length += track->tr_length;
-	if (queue->q_ops)
-		queue->q_ops->qop_added(queue, pos);
-	return pos;
-}
-
-static inline unsigned int __queue_add_tail(struct queue *queue,
-					    struct track *track)
-{
-	g_queue_push_tail(&queue->q_tracks, track);
-	return __queue_added(queue, track, g_queue_get_length(&queue->q_tracks) - 1);
-}
-
-static inline unsigned int __queue_add_sorted(struct queue *queue,
-					      struct track *track)
-{
-	struct queue_iter it;
-
-	queue_for_each(queue, &it) {
-		if (track_less_than(queue_iter_val(&it), track, queue->q_sort) > 0) {
-			g_queue_insert_before(&queue->q_tracks, it.it_iter, track);
-			return __queue_added(queue, track, it.it_pos);
-		}
-	}
-	return __queue_add_tail(queue, track);
-}
-
 void queue_init(struct queue *queue, const struct queue_ops *ops, void *data)
 {
 	queue->q_length = 0;
@@ -91,10 +38,3 @@ void queue_deinit(struct queue *queue)
 	g_slist_free(queue->q_sort);
 	queue->q_sort   = NULL;
 }
-
-unsigned int queue_add(struct queue *queue, struct track *track)
-{
-	if (queue->q_sort)
-		return __queue_add_sorted(queue, track);
-	return __queue_add_tail(queue, track);
-}
diff --git a/gui/playlist.c b/gui/playlist.c
index ae08b875..1bf2242b 100644
--- a/gui/playlist.c
+++ b/gui/playlist.c
@@ -48,11 +48,6 @@ static void __gui_playlist_added(struct playlist *playlist, struct track *track)
 	__gui_playlist_update_size(playlist);
 }
 
-static void __gui_playlist_track_added(struct queue *queue, unsigned int row)
-{
-	__gui_playlist_added(queue->q_private, queue_at(queue, row));
-}
-
 static void __gui_playlist_removed(struct playlist *playlist, struct track *track,
 				   unsigned int n)
 {
@@ -64,7 +59,6 @@ static void __gui_playlist_removed(struct playlist *playlist, struct track *trac
 struct queue_ops playlist_ops = {
 	.qop_init    = __gui_playlist_init,
 	.qop_deinit  = __gui_playlist_deinit,
-	.qop_added   = __gui_playlist_track_added,
 };
 
 struct playlist_callbacks playlist_cb = {
diff --git a/include/core/playlists/generic.h b/include/core/playlists/generic.h
index b6f2fe1e..c584c128 100644
--- a/include/core/playlists/generic.h
+++ b/include/core/playlists/generic.h
@@ -51,7 +51,7 @@ bool playlist_generic_can_select(struct playlist *);
 void playlist_generic_clear(struct playlist *);
 
 /* Generic playlist add track operations. */
-bool playlist_generic_add_track(struct playlist *, struct track *);
+bool playlist_generic_add(struct playlist *, struct track *);
 bool playlist_generic_add_front(struct playlist *, struct track *);
 
 /* Generic playlist remove track operation. */
diff --git a/include/core/queue.h b/include/core/queue.h
index e1510730..474b769c 100644
--- a/include/core/queue.h
+++ b/include/core/queue.h
@@ -20,9 +20,6 @@ struct queue_ops {
 
 	/* Called to tell a higher layer that a queue is deinitializing. */
 	void (*qop_deinit)(struct queue *);
-
-	/* Called to tell a higher layer that a track has been added. */
-	void (*qop_added)(struct queue *, unsigned int);
 };
 
 
@@ -97,8 +94,4 @@ static inline struct track *queue_at(struct queue *queue, unsigned int index)
 	return (struct track *)g_queue_peek_nth(&queue->q_tracks, index);
 }
 
-
-/* Called to add a track to the queue. */
-unsigned int queue_add(struct queue *, struct track *);
-
 #endif /* OCARINA_CORE_QUEUE_H */
diff --git a/tests/core/playlist.c b/tests/core/playlist.c
index ac8eced5..7c1812da 100644
--- a/tests/core/playlist.c
+++ b/tests/core/playlist.c
@@ -24,6 +24,7 @@ static void test_pl_callback(struct playlist *playlist, struct track *track)
 
 static struct playlist_ops test_noop;
 static struct playlist_ops test_ops = {
+	.pl_add        = playlist_generic_add,
 	.pl_can_select = playlist_generic_can_select,
 	.pl_remove     = playlist_generic_remove,
 	.pl_set_random = playlist_generic_set_random,
@@ -66,6 +67,7 @@ static void test_null()
 	playlist_generic_resort(NULL);
 	playlist_clear_sort(NULL);
 
+	g_assert_false(playlist_generic_add(NULL, NULL));
 	g_assert_false(playlist_generic_add_front(NULL, NULL));
 	g_assert_false(playlist_generic_remove(NULL, NULL));
 	playlist_generic_update(NULL, NULL);
@@ -130,10 +132,17 @@ static void test_playlist()
 
 	/* Re-add the tracks! */
 	for (i = 0; i < 13; i++) {
-		playlist_generic_add_front(&p, track_get(i));
-		g_assert_true(playlist_has(&p, track_get(i)));
 		ex_length += track_get(i)->tr_length;
+		g_assert_true( playlist_add(&p, track_get(i)));
+		g_assert_false(playlist_add(&p, track_get(i)));
+		g_assert_true(playlist_has(&p, track_get(i)));
+		g_assert_cmpuint(p.pl_queue.q_length, ==, ex_length);
+		g_assert_cmpuint(playlist_size(&p), ==, i + 1);
+		g_assert(cb_playlist == &p);
+		g_assert(cb_track    == track_get(i));
 	}
+	g_assert_false(playlist_generic_add(&p, NULL));
+	g_assert_cmpuint(p.pl_queue.q_length, ==, ex_length);
 
 	/* Now clear the playlist. */
 	queue_iter_set(&p.pl_queue, &p.pl_queue.q_cur, 12);
@@ -205,6 +214,19 @@ static void test_sorting()
 		else
 			g_assert_cmpuint(track->tr_track, ==, (2 * i) - 12);
 	}
+
+	/* Test inserting at a sorted position. */
+	playlist_generic_clear(&p);
+	playlist_clear_sort(&p);
+	playlist_sort(&p, COMPARE_TRACK);
+	playlist_sort(&p, COMPARE_TRACK);
+	for (i = 0; i < 13; i++) {
+		g_assert_false(playlist_has(&p, track_get(i)));
+		g_assert_true( playlist_add(&p, track_get(i)));
+		g_assert_false(playlist_add(&p, track_get(i)));
+	}
+	for (i = 0; i < 13; i++)
+		g_assert_cmpuint(queue_at(&p.pl_queue, i)->tr_track, ==, 13 - i);
 }
 
 static void test_next()
@@ -217,7 +239,7 @@ static void test_next()
 	playlist_generic_init(&p, NULL);
 
 	for (i = 0; i < 13; i++)
-		playlist_generic_add_track(&p, track_get(i));
+		playlist_generic_add(&p, track_get(i));
 
 	g_assert_true(playlist_select(&p));
 	g_assert(playlist_current() == &p);
@@ -255,8 +277,8 @@ static void test_save_load()
 	unsigned int    i;
 
 	for (i = 0; i < 13; i++) {
-		queue_add(&p.pl_queue, track_get(i));
-		queue_add(&q.pl_queue, track_get(i));
+		playlist_generic_add(&p, track_get(i));
+		playlist_generic_add(&q, track_get(i));
 	}
 	playlist_set_random(&p, true);
 	playlist_clear_sort(&p);
diff --git a/tests/core/queue.c b/tests/core/queue.c
index 18cb54cf..5865caaf 100644
--- a/tests/core/queue.c
+++ b/tests/core/queue.c
@@ -9,7 +9,6 @@
 
 unsigned int count_init    = 0;
 unsigned int count_deinit  = 0;
-unsigned int count_added   = 0;
 
 
 static void *queue_op_init(struct queue *queue, void *data)
@@ -23,16 +22,10 @@ static void queue_op_deinit(struct queue *queue)
 	count_deinit++;
 }
 
-static void queue_op_added(struct queue *queue, unsigned int pos)
-{
-	count_added++;
-}
-
 
 static const struct queue_ops test_ops = {
 	.qop_init    = queue_op_init,
 	.qop_deinit  = queue_op_deinit,
-	.qop_added   = queue_op_added,
 };
 
 
@@ -71,48 +64,6 @@ static void test_init()
 	g_assert_cmpuint(count_deinit, ==, 1);
 }
 
-static void test_queue(gconstpointer arg)
-{
-	unsigned int N = GPOINTER_TO_UINT(arg);
-	unsigned int ex_length = 0;
-	struct queue_iter it;
-	struct track *track;
-	unsigned int i;
-	struct queue q;
-
-	count_added   = 0;
-
-	queue_init(&q, &test_ops, NULL);
-
-	/* queue_add() */
-	for (i = 0; i < N; i++) {
-		track = track_get(i % 13);
-		ex_length += track->tr_length;
-		g_assert_cmpuint(queue_add(&q, track), ==, i);
-		g_assert_cmpuint(count_added, ==, i + 1);
-	}
-	g_assert_cmpuint(q.q_length,     ==, ex_length);
-
-	/* queue_iter_init() */
-	if (N > 0) {
-		queue_iter_init(&q, &it);
-		g_assert_nonnull(it.it_iter);
-		g_assert_cmpuint(it.it_pos, ==, 0);
-	}
-
-	/* queue_for_each() */
-	i = 0;
-	queue_for_each(&q, &it) {
-		g_assert_cmpuint(it.it_pos, ==, i);
-		g_assert(queue_iter_val(&it) == track_get(i % 13));
-		i++;
-	}
-	g_assert_cmpuint(i, ==, N);
-
-	queue_deinit(&q);
-	g_assert_null(q.q_sort);
-}
-
 int main(int argc, char **argv)
 {
 	struct library *library;
@@ -140,9 +91,6 @@ int main(int argc, char **argv)
 
 	g_test_init(&argc, &argv, NULL);
 	g_test_add_func("/Core/Queue/Initialization", test_init);
-	g_test_add_data_func("/Core/Queue/n = 0",        GUINT_TO_POINTER(     0), test_queue);
-	g_test_add_data_func("/Core/Queue/n = 13",       GUINT_TO_POINTER(    13), test_queue);
-	g_test_add_data_func("/Core/Queue/n = 100,009)", GUINT_TO_POINTER(100009), test_queue);
 	ret = g_test_run();
 
 	tags_deinit();
diff --git a/tests/gui/filter.c b/tests/gui/filter.c
index e4931f26..fbe4fada 100644
--- a/tests/gui/filter.c
+++ b/tests/gui/filter.c
@@ -12,13 +12,10 @@ void *test_queue_init(struct queue *queue, void *data)
 	{ return NULL; }
 void test_queue_deinit(struct queue *queue)
 	{ gui_filter_clear_search(queue->q_private); }
-void test_queue_add(struct queue *queue, unsigned int n)
-	{ gui_model_add(queue->q_private, queue_at(queue, n)); }
 
 struct queue_ops test_ops = {
 	.qop_init    = test_queue_init,
 	.qop_deinit  = test_queue_deinit,
-	.qop_added   = test_queue_add,
 };
 
 void test_on_load(struct track *track) {}
diff --git a/tests/gui/model.c b/tests/gui/model.c
index 19405267..1dab9dd7 100644
--- a/tests/gui/model.c
+++ b/tests/gui/model.c
@@ -27,8 +27,6 @@ void *test_queue_init(struct queue *queue, void *data)
 	{ return NULL; }
 void test_queue_deinit(struct queue *queue)
 	{ }
-void test_queue_add(struct queue *queue, unsigned int n)
-	{ gui_model_add(queue->q_private, queue_at(queue, n)); }
 void test_on_load(struct track *track) {}
 void test_on_state_change(GstState state) {}
 void test_on_config_pause(int count) {}
@@ -36,7 +34,6 @@ void test_on_config_pause(int count) {}
 struct queue_ops test_ops = {
 	.qop_init    = test_queue_init,
 	.qop_deinit  = test_queue_deinit,
-	.qop_added   = test_queue_add,
 };
 
 struct playlist_callbacks test_cb = {