gui/playlist: Implement playlist_generic_remove() directly

Rather than iterating over the entire playlist ourselves, we can instead
use the g_queue_remove_all() function to do most of the work for us and
then send an appropriate number of "removed" callbacks.

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

core/playlist.c

@@ -183,7 +183,7 @@ bool playlist_remove(struct playlist *playlist, struct track *track)
 		return false;
 
 	ret = playlist->pl_ops->pl_remove(playlist, track);
-	if (ret)
+	if (ret && playlist->pl_type < PL_MAX_TYPE)
 		playlist_types[playlist->pl_type]->pl_save();
 	return ret;
 }

core/playlists/artist.c

@@ -171,9 +171,6 @@ void pl_artist_new_track(struct track *track)
 
 void pl_artist_delete_track(struct track *track)
 {
-	struct artist   *artist   = track->tr_album->al_artist;
+	struct artist *artist = track->tr_album->al_artist;
-	struct playlist *playlist = (struct playlist *)artist->ar_playlist;
+	playlist_generic_remove(artist->ar_playlist, track);
-
-	if (playlist)
-		playlist_generic_remove_track(playlist, track);
 }

core/playlists/generic.c

@@ -152,9 +152,27 @@ bool playlist_generic_add_track_front(struct playlist *playlist,
 	return true;
 }
 
-bool playlist_generic_remove_track(struct playlist *playlist, struct track *track)
+bool playlist_generic_remove(struct playlist *playlist, struct track *track)
 {
-	return queue_remove_all(&playlist->pl_queue, track);
+	unsigned int i, count;
+
+	if (!playlist || !track)
+		return false;
+
+	while (queue_iter_val(&playlist->pl_queue.q_cur) == track)
+		queue_iter_prev(&playlist->pl_queue.q_cur);
+
+	count = g_queue_remove_all(&playlist->pl_queue.q_tracks, track);
+	playlist->pl_queue.q_length    -= (count * track->tr_length);
+	playlist->pl_queue.q_cur.it_pos = g_queue_link_index(
+					&playlist->pl_queue.q_tracks,
+					playlist->pl_queue.q_cur.it_iter);
+
+	if (callbacks) {
+		for (i = 0; i < count; i++)
+			callbacks->pl_cb_removed(playlist, track);
+	}
+	return count > 0;
 }
 
 void playlist_generic_update(struct playlist *playlist, struct track *track)

core/playlists/library.c

@@ -157,7 +157,7 @@ static bool __lib_pl_update(void *data)
 		if (g_access(path, F_OK) < 0) {
 			pl_system_delete_track(TRACK(dbe));
 			pl_artist_delete_track(TRACK(dbe));
-			queue_remove_all(&playlist->pl_queue, TRACK(dbe));
+			playlist_generic_remove(playlist, TRACK(dbe));
 			track_remove(TRACK(dbe));
 		}
 		g_free(path);

core/playlists/system.c

@@ -51,7 +51,7 @@ static bool sys_pl_update_func(void *data)
 		   !queue_has(&pl_system_get(SYS_PL_HIDDEN)->pl_queue, track))
 			playlist_generic_add_track_front(playlist, track);
 		else
-			playlist_generic_remove_track(playlist, track);
+			playlist_generic_remove(playlist, track);
 	}
 
 	playlist_generic_resort(playlist);
@@ -79,7 +79,7 @@ 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_remove     = playlist_generic_remove_track,
+	.pl_remove     = playlist_generic_remove,
 	.pl_set_random = playlist_generic_set_random,
 	.pl_sort       = playlist_generic_sort,
 };
@@ -91,16 +91,16 @@ 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);
-	playlist_generic_remove_track(pl_system_get(SYS_PL_COLLECTION),     track);
+	playlist_generic_remove(pl_system_get(SYS_PL_COLLECTION),     track);
-	playlist_generic_remove_track(pl_system_get(SYS_PL_UNPLAYED),       track);
+	playlist_generic_remove(pl_system_get(SYS_PL_UNPLAYED),       track);
-	playlist_generic_remove_track(pl_system_get(SYS_PL_MOST_PLAYED),    track);
+	playlist_generic_remove(pl_system_get(SYS_PL_MOST_PLAYED),    track);
-	playlist_generic_remove_track(pl_system_get(SYS_PL_LEAST_PLAYED),   track);
+	playlist_generic_remove(pl_system_get(SYS_PL_LEAST_PLAYED),   track);
 	return ret;
 }
 
 static bool sys_pl_hidden_remove(struct playlist *playlist, struct track *track)
 {
-	bool ret = playlist_generic_remove_track(playlist, track);
+	bool ret = playlist_generic_remove(playlist, track);
 	unsigned int average = track_db_average_plays();
 	unsigned int add_id  = SYS_PL_LEAST_PLAYED;
 
@@ -165,7 +165,7 @@ 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_remove     = playlist_generic_remove_track,
+	.pl_remove     = playlist_generic_remove,
 	.pl_set_random = playlist_generic_set_random,
 	.pl_sort       = playlist_generic_sort,
 };
@@ -371,7 +371,7 @@ static void pl_system_selected(struct track *track)
 	unsigned int i;
 
 	queue_iter_prev(&pl_system_get(SYS_PL_QUEUED)->pl_queue.q_cur);
-	queue_remove_all(&pl_system_get(SYS_PL_QUEUED)->pl_queue, track);
+	playlist_generic_remove(pl_system_get(SYS_PL_QUEUED), track);
 	for (i = 0; i < SYS_PL_NUM_PLAYLISTS; i++)
 		playlist_generic_update(pl_system_get(i), track);
 }
@@ -429,5 +429,5 @@ void pl_system_new_track(struct track *track)
 void pl_system_delete_track(struct track *track)
 {
 	for (unsigned int i = 0; i < SYS_PL_NUM_PLAYLISTS; i++)
-		playlist_generic_remove_track(pl_system_get(i), track);
+		playlist_generic_remove(pl_system_get(i), track);
 }

core/playlists/user.c

@@ -80,7 +80,7 @@ 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_remove     = playlist_generic_remove_track,
+	.pl_remove     = playlist_generic_remove,
 	.pl_set_random = playlist_generic_set_random,
 	.pl_sort       = playlist_generic_sort,
 };
@@ -155,6 +155,6 @@ void pl_user_delete_track(struct track *track)
 
 	db_for_each(dbe, next, &user_db) {
 		playlist = &USER_PLAYLIST(dbe)->pl_playlist;
-		playlist_generic_remove_track(playlist, track);
+		playlist_generic_remove(playlist, track);
 	}
 }

core/queue.c

@@ -79,20 +79,6 @@ static inline unsigned int __queue_add_sorted(struct queue *queue,
 	return __queue_add_tail(queue, track);
 }
 
-static inline void __queue_remove(struct queue *queue, struct queue_iter *it)
-{
-	struct track *track = queue_iter_val(it);
-	unsigned int pos = it->it_pos;
-	GList *link = it->it_iter;
-
-	queue_iter_prev(it);
-	g_queue_delete_link(&queue->q_tracks, link);
-
-	queue->q_length -= track->tr_length;
-	if (queue->q_ops)
-		queue->q_ops->qop_removed(queue, pos);
-}
-
 static inline void __queue_clear(struct queue *queue, unsigned int n)
 {
 	queue->q_length = 0;
@@ -132,34 +118,6 @@ unsigned int queue_add_front(struct queue *queue, struct track *track)
 	return __queue_add_head(queue, track);
 }
 
-void queue_remove(struct queue *queue, unsigned int index)
-{
-	struct queue_iter it;
-
-	queue_iter_set(queue, &it, index);
-	__queue_remove(queue, &it);
-}
-
-unsigned int queue_remove_all(struct queue *queue, struct track *track)
-{
-	unsigned int count = 0;
-	struct queue_iter it;
-
-	while (queue_at(queue, 0) == track) {
-		queue_remove(queue, 0);
-		count++;
-	}
-
-	queue_for_each(queue, &it) {
-		if (queue_iter_val(&it) == track) {
-			__queue_remove(queue, &it);
-			count++;
-		}
-	}
-
-	return count;
-}
-
 void queue_clear(struct queue *queue)
 {
 	unsigned int n = queue_size(queue);

gui/model.c

@@ -290,14 +290,14 @@ void gui_model_add(struct playlist *playlist, unsigned int row)
 	gtk_tree_path_free(path);
 }
 
-void gui_model_remove(struct playlist *playlist, unsigned int row)
+void gui_model_remove(struct playlist *playlist, struct track *track)
 {
 	GtkTreePath *path;
 
 	if (cur_playlist != playlist)
 		return;
 
-	path = gtk_tree_path_new_from_indices(row, -1);
+	path = gtk_tree_path_new_from_indices(0, -1);
 	gtk_tree_model_row_deleted(GTK_TREE_MODEL(gui_model), path);
 	__gui_model_set_runtime();
 	gtk_tree_path_free(path);

gui/playlist.c

@@ -48,10 +48,10 @@ static void __gui_playlist_added(struct queue *queue, unsigned int row)
 	__gui_playlist_update_size(queue->q_private);
 }
 
-static void __gui_playlist_removed(struct queue *queue, unsigned int row)
+static void __gui_playlist_removed(struct playlist *playlist, struct track *track)
 {
-	gui_model_remove(queue->q_private, row);
+	gui_model_remove(playlist, track);
-	__gui_playlist_update_size(queue->q_private);
+	__gui_playlist_update_size(playlist);
 }
 
 static void __gui_playlist_cleared(struct queue *queue, unsigned int n)
@@ -65,11 +65,11 @@ struct queue_ops playlist_ops = {
 	.qop_init    = __gui_playlist_init,
 	.qop_deinit  = __gui_playlist_deinit,
 	.qop_added   = __gui_playlist_added,
-	.qop_removed = __gui_playlist_removed,
 	.qop_cleared = __gui_playlist_cleared,
 };
 
 struct playlist_callbacks playlist_cb = {
+	.pl_cb_removed = __gui_playlist_removed,
 	.pl_cb_updated = gui_model_update,
 };
 

include/core/playlists/generic.h

@@ -14,6 +14,9 @@ enum playlist_save_flags {
 #define PL_SAVE_ALL      (PL_SAVE_TRACKS | PL_SAVE_METADATA)
 
 struct playlist_callbacks {
+	/* Called to notify that a track has been removed. */
+	void (*pl_cb_removed)(struct playlist *, struct track *);
+
 	/*
 	 * Called to notify that a track has been updated.
 	 * If the track is NULL, then the entire playlist should be updated.
@@ -45,7 +48,7 @@ bool playlist_generic_add_track(struct playlist *, struct track *);
 bool playlist_generic_add_track_front(struct playlist *, struct track *);
 
 /* Generic playlist remove track operation. */
-bool playlist_generic_remove_track(struct playlist *, struct track *);
+bool playlist_generic_remove(struct playlist *, struct track *);
 
 /* Generic playlist update track operation. */
 void playlist_generic_update(struct playlist *, struct track *);

include/core/queue.h

@@ -24,9 +24,6 @@ struct queue_ops {
 	/* Called to tell a higher layer that a track has been added. */
 	void (*qop_added)(struct queue *, unsigned int);
 
-	/* Called to tell a higher layer that a track has been removed. */
-	void (*qop_removed)(struct queue *, unsigned int);
-
 	/* Called to tell a higher layer that the queue has been cleared. */
 	void (*qop_cleared)(struct queue *, unsigned int);
 };
@@ -116,12 +113,6 @@ unsigned int queue_add(struct queue *, struct track *);
 /* Called to add a track to the front of the queue. */
 unsigned int queue_add_front(struct queue *, struct track *);
 
-/* Called to remove a track from the queue by index. */
-void queue_remove(struct queue *, unsigned int);
-
-/* Called to remove all instances of the track from the queue. */
-unsigned int queue_remove_all(struct queue *, struct track *);
-
 /* Called to remove all tracks from the queue. */
 void queue_clear(struct queue *);
 

include/gui/model.h

@@ -55,7 +55,7 @@ GType gui_model_get_type();
 void gui_model_add(struct playlist *, unsigned int);
 
 /* Called to remove a row from the model */
-void gui_model_remove(struct playlist *, unsigned int);
+void gui_model_remove(struct playlist *, struct track *);
 
 /* Called to remove all rows from the model */
 void gui_model_clear(struct playlist *, unsigned int);

tests/core/playlist.c

@@ -9,7 +9,7 @@
 static struct playlist *cb_playlist = NULL;
 static struct track    *cb_track    = NULL;
 
-static void test_pl_updated(struct playlist *playlist, struct track *track)
+static void test_pl_callback(struct playlist *playlist, struct track *track)
 {
 	cb_playlist = playlist;
 	cb_track    = track;
@@ -18,11 +18,13 @@ static void test_pl_updated(struct playlist *playlist, struct track *track)
 static struct playlist_ops test_noop;
 static struct playlist_ops test_ops = {
 	.pl_can_select = playlist_generic_can_select,
+	.pl_remove     = playlist_generic_remove,
 	.pl_set_random = playlist_generic_set_random,
 	.pl_sort       = playlist_generic_sort,
 };
 static struct playlist_callbacks test_cb = {
-	.pl_cb_updated = test_pl_updated,
+	.pl_cb_removed = test_pl_callback,
+	.pl_cb_updated = test_pl_callback,
 };
 
 static void test_null()
@@ -56,6 +58,7 @@ static void test_null()
 	playlist_generic_resort(NULL);
 	playlist_clear_sort(NULL);
 
+	g_assert_false(playlist_generic_remove(NULL, NULL));
 	playlist_generic_update(NULL, NULL);
 
 	playlist_generic_save(NULL, NULL, PL_SAVE_ALL);
@@ -65,12 +68,15 @@ static void test_null()
 static void test_playlist()
 {
 	struct playlist p = DEFINE_PLAYLIST(PL_MAX_TYPE, "Test", 0, &test_ops);
+	unsigned int ex_length = 0;
 	unsigned int i;
 
 	playlist_generic_init(&p, NULL);
 
-	for (i = 0; i < 13; i++)
+	for (i = 0; i < 13; i++) {
 		playlist_generic_add_track_front(&p, track_get(i));
+		ex_length += track_get(i)->tr_length;
+	}
 
 	/* Trigger an update for each track. */
 	for (i = 0; i < 13; i++) {
@@ -81,6 +87,21 @@ static void test_playlist()
 	playlist_generic_update(&p, NULL);
 	g_assert(cb_playlist == &p);
 	g_assert_null(cb_track);
+
+	/* Remove all tracks. */
+	queue_iter_set(&p.pl_queue, &p.pl_queue.q_cur, 12);
+	for (i = 0; i < 13; i++) {
+		ex_length -= track_get(i)->tr_length;
+		g_assert_true(playlist_remove(&p, track_get(i)));
+		g_assert(cb_playlist == &p);
+		g_assert(cb_track    == track_get(i));
+		g_assert_cmpuint(p.pl_queue.q_length, ==, ex_length);
+		g_assert_cmpuint(p.pl_queue.q_cur.it_pos, ==, (11 - i));
+	}
+	g_assert_false(playlist_generic_remove(&p, NULL));
+	g_assert(cb_playlist == &p);
+	g_assert(cb_track    == track_get(12));
+	g_assert_cmpuint(p.pl_queue.q_length, ==, ex_length);
 }
 
 static void test_sorting()

tests/core/queue.c

@@ -10,7 +10,6 @@
 unsigned int count_init    = 0;
 unsigned int count_deinit  = 0;
 unsigned int count_added   = 0;
-unsigned int count_deleted = 0;
 unsigned int count_cleared = 0;
 
 
@@ -30,11 +29,6 @@ static void queue_op_added(struct queue *queue, unsigned int pos)
 	count_added++;
 }
 
-static void queue_op_removed(struct queue *queue, unsigned int pos)
-{
-	count_deleted++;
-}
-
 static void queue_op_cleared(struct queue *queue, unsigned int n)
 {
 	count_cleared++;
@@ -45,7 +39,6 @@ static const struct queue_ops test_ops = {
 	.qop_init    = queue_op_init,
 	.qop_deinit  = queue_op_deinit,
 	.qop_added   = queue_op_added,
-	.qop_removed = queue_op_removed,
 	.qop_cleared = queue_op_cleared,
 };
 
@@ -96,7 +89,6 @@ static void test_queue(gconstpointer arg)
 	struct queue q;
 
 	count_added   = 0;
-	count_deleted = 0;
 	count_cleared = 0;
 
 	queue_init(&q, &test_ops, NULL);
@@ -128,30 +120,6 @@ static void test_queue(gconstpointer arg)
 	g_assert_cmpuint(i, ==, N);
 	g_assert_cmpuint(queue_size(&q), ==, ex_size);
 
-	/* queue_remove_all() and queue_has() */
-	track = track_get(0);
-	ex_length -= track->tr_length * (N / 13);
-	ex_size   -= (N / 13);
-	if (N > 0)
-		g_assert_true(queue_has(&q, track));
-	else
-		g_assert_false(queue_has(&q, track));
-	g_assert_cmpuint(queue_remove_all(&q, track), ==, N / 13);
-	g_assert_cmpuint(q.q_length,     ==, ex_length);
-	g_assert_cmpuint(queue_size(&q), ==, ex_size);
-	g_assert_false(queue_has(&q, track));
-
-	/* queue_remove() and queue_erase() == true */
-	track = track_get(1);
-	ex_length -= track->tr_length * (N / 13);
-	ex_size   -= (N / 13);
-	for (i = 0; i < ex_size; i += 11) {
-		g_assert(queue_at(&q, i) == track);
-		queue_remove(&q, i);
-	}
-	g_assert_cmpuint(q.q_length,     ==, ex_length);
-	g_assert_cmpuint(queue_size(&q), ==, ex_size);
-
 	queue_clear(&q);
 	g_assert_cmpuint(count_cleared,  ==, 1);
 	g_assert_cmpuint(queue_size(&q), ==, 0);

tests/gui/filter.c

@@ -14,8 +14,6 @@ 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, n); }
-void test_queue_remove(struct queue *queue, unsigned int n)
-	{ gui_model_remove(queue->q_private, n); }
 void test_queue_clear(struct queue *queue, unsigned int n)
 	{ gui_model_clear(queue->q_private, n); }
 
@@ -23,7 +21,6 @@ struct queue_ops test_ops = {
 	.qop_init    = test_queue_init,
 	.qop_deinit  = test_queue_deinit,
 	.qop_added   = test_queue_add,
-	.qop_removed = test_queue_remove,
 	.qop_cleared = test_queue_clear,
 };
 

tests/gui/model.c

@@ -29,8 +29,6 @@ void test_queue_deinit(struct queue *queue)
 	{ }
 void test_queue_add(struct queue *queue, unsigned int n)
 	{ gui_model_add(queue->q_private, n); }
-void test_queue_remove(struct queue *queue, unsigned int n)
-	{ gui_model_remove(queue->q_private, n); }
 void test_queue_clear(struct queue *queue, unsigned int n)
 	{ gui_model_clear(queue->q_private, n); }
 void test_on_load(struct track *track) {}
@@ -41,11 +39,11 @@ struct queue_ops test_ops = {
 	.qop_init    = test_queue_init,
 	.qop_deinit  = test_queue_deinit,
 	.qop_added   = test_queue_add,
-	.qop_removed = test_queue_remove,
 	.qop_cleared = test_queue_clear,
 };
 
 struct playlist_callbacks test_cb = {
+	.pl_cb_removed = gui_model_remove,
 	.pl_cb_updated = gui_model_update,
 };