From a45c7d68891f9b7d4dac91f3fb2a0404d1e3834d Mon Sep 17 00:00:00 2001 From: Anna Schumaker Date: Tue, 20 Sep 2016 14:38:46 -0400 Subject: [PATCH] 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 --- core/playlist.c | 2 +- core/playlists/artist.c | 7 ++---- core/playlists/generic.c | 22 +++++++++++++++-- core/playlists/library.c | 2 +- core/playlists/system.c | 20 +++++++-------- core/playlists/user.c | 4 +-- core/queue.c | 42 -------------------------------- gui/model.c | 4 +-- gui/playlist.c | 8 +++--- include/core/playlists/generic.h | 5 +++- include/core/queue.h | 9 ------- include/gui/model.h | 2 +- tests/core/playlist.c | 27 +++++++++++++++++--- tests/core/queue.c | 32 ------------------------ tests/gui/filter.c | 3 --- tests/gui/model.c | 4 +-- 16 files changed, 72 insertions(+), 121 deletions(-) diff --git a/core/playlist.c b/core/playlist.c index 69e75060..f849e334 100644 --- a/core/playlist.c +++ b/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; } diff --git a/core/playlists/artist.c b/core/playlists/artist.c index 058a3c02..1d6832a4 100644 --- a/core/playlists/artist.c +++ b/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 playlist *playlist = (struct playlist *)artist->ar_playlist; - - if (playlist) - playlist_generic_remove_track(playlist, track); + struct artist *artist = track->tr_album->al_artist; + playlist_generic_remove(artist->ar_playlist, track); } diff --git a/core/playlists/generic.c b/core/playlists/generic.c index 51960693..4db2446e 100644 --- a/core/playlists/generic.c +++ b/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) diff --git a/core/playlists/library.c b/core/playlists/library.c index dda2c953..056a932a 100644 --- a/core/playlists/library.c +++ b/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); diff --git a/core/playlists/system.c b/core/playlists/system.c index c7398796..9fec512f 100644 --- a/core/playlists/system.c +++ b/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_track(pl_system_get(SYS_PL_UNPLAYED), track); - playlist_generic_remove_track(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_COLLECTION), track); + playlist_generic_remove(pl_system_get(SYS_PL_UNPLAYED), track); + playlist_generic_remove(pl_system_get(SYS_PL_MOST_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); } diff --git a/core/playlists/user.c b/core/playlists/user.c index 758ea1cf..2ad18678 100644 --- a/core/playlists/user.c +++ b/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); } } diff --git a/core/queue.c b/core/queue.c index de8ba5a7..3d5e9466 100644 --- a/core/queue.c +++ b/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); diff --git a/gui/model.c b/gui/model.c index 6d0e2315..b65bd50a 100644 --- a/gui/model.c +++ b/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); diff --git a/gui/playlist.c b/gui/playlist.c index 67f3ef35..2b958e58 100644 --- a/gui/playlist.c +++ b/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_playlist_update_size(queue->q_private); + gui_model_remove(playlist, track); + __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, }; diff --git a/include/core/playlists/generic.h b/include/core/playlists/generic.h index 92374045..e601c6c2 100644 --- a/include/core/playlists/generic.h +++ b/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 *); diff --git a/include/core/queue.h b/include/core/queue.h index 751baf61..d34bdd11 100644 --- a/include/core/queue.h +++ b/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 *); diff --git a/include/gui/model.h b/include/gui/model.h index 832685bd..38cbb47a 100644 --- a/include/gui/model.h +++ b/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); diff --git a/tests/core/playlist.c b/tests/core/playlist.c index 65787073..754f7298 100644 --- a/tests/core/playlist.c +++ b/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() diff --git a/tests/core/queue.c b/tests/core/queue.c index 6ac2b1d9..00a0c4b3 100644 --- a/tests/core/queue.c +++ b/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); diff --git a/tests/gui/filter.c b/tests/gui/filter.c index ae61442a..58dd92bb 100644 --- a/tests/gui/filter.c +++ b/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, }; diff --git a/tests/gui/model.c b/tests/gui/model.c index 5e334ecd..6c936db0 100644 --- a/tests/gui/model.c +++ b/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, };