From ca6c5293c61aefb226c0398069bbb72859c3e608 Mon Sep 17 00:00:00 2001 From: Anna Schumaker Date: Tue, 20 Sep 2016 11:01:32 -0400 Subject: [PATCH] core/playlist: Add a playlist_generic_update() function This function simply triggers the "updated" callback for the given playlist and track. I updated the gui model to handle taking tracks instead of row indexes, since this lets me reuse the same for-each function that we do for sorting. Additionally, this prevents UI updates for playlists that aren't currently visible. Signed-off-by: Anna Schumaker --- core/playlists/artist.c | 6 ++--- core/playlists/generic.c | 9 +++++-- core/playlists/library.c | 6 ++--- core/playlists/system.c | 4 +-- core/playlists/user.c | 8 ++---- core/queue.c | 16 ------------ gui/model.c | 24 +++++------------ gui/playlist.c | 8 +----- include/core/playlists/generic.h | 10 ++++++-- include/core/queue.h | 6 ----- include/gui/model.h | 10 ++++---- tests/core/playlist.c | 44 +++++++++++++++++++++++++++----- tests/core/queue.c | 13 ---------- tests/gui/filter.c | 3 --- tests/gui/model.c | 5 +--- 15 files changed, 73 insertions(+), 99 deletions(-) diff --git a/core/playlists/artist.c b/core/playlists/artist.c index 51ab92f0..058a3c02 100644 --- a/core/playlists/artist.c +++ b/core/playlists/artist.c @@ -112,10 +112,8 @@ static struct playlist *pl_artist_get(unsigned int id) static void pl_artist_played(struct track *track) { - struct artist *artist = track->tr_album->al_artist; - struct playlist *playlist = (struct playlist *)artist->ar_playlist; - if (playlist) - queue_updated(&playlist->pl_queue, track); + struct artist *artist = track->tr_album->al_artist; + playlist_generic_update(artist->ar_playlist, track); } diff --git a/core/playlists/generic.c b/core/playlists/generic.c index dc8749eb..51960693 100644 --- a/core/playlists/generic.c +++ b/core/playlists/generic.c @@ -157,6 +157,12 @@ bool playlist_generic_remove_track(struct playlist *playlist, struct track *trac return queue_remove_all(&playlist->pl_queue, track); } +void playlist_generic_update(struct playlist *playlist, struct track *track) +{ + if (playlist && callbacks) + callbacks->pl_cb_updated(playlist, track); +} + void playlist_generic_set_random(struct playlist *playlist, bool enabled) { playlist->pl_random = enabled; @@ -185,8 +191,7 @@ void playlist_generic_resort(struct playlist *playlist) g_queue_sort(&playlist->pl_queue.q_tracks, __playlist_generic_less_than, playlist->pl_queue.q_sort); - if (callbacks) - callbacks->pl_cb_sorted(playlist); + playlist_generic_update(playlist, NULL); } struct track *playlist_generic_next(struct playlist *playlist) diff --git a/core/playlists/library.c b/core/playlists/library.c index 12b68bd2..dda2c953 100644 --- a/core/playlists/library.c +++ b/core/playlists/library.c @@ -243,10 +243,8 @@ static struct playlist *pl_library_new(const gchar *name) static void pl_library_played(struct track *track) { - struct library *library = track->tr_library; - struct playlist *playlist = (struct playlist *)library->li_playlist; - if (playlist) - queue_updated(&playlist->pl_queue, track); + struct library *library = track->tr_library; + playlist_generic_update(library->li_playlist, track); } diff --git a/core/playlists/system.c b/core/playlists/system.c index 4c7862a4..c7398796 100644 --- a/core/playlists/system.c +++ b/core/playlists/system.c @@ -359,7 +359,7 @@ static void pl_system_played(struct track *track) { unsigned int i; for (i = 0; i < SYS_PL_NUM_PLAYLISTS; i++) - queue_updated(&pl_system_get(i)->pl_queue, track); + playlist_generic_update(pl_system_get(i), track); sys_pl_update(pl_system_lookup("Unplayed")); sys_pl_update(pl_system_lookup("Most Played")); @@ -373,7 +373,7 @@ static void pl_system_selected(struct track *track) 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); for (i = 0; i < SYS_PL_NUM_PLAYLISTS; i++) - queue_updated(&pl_system_get(i)->pl_queue, track); + playlist_generic_update(pl_system_get(i), track); } diff --git a/core/playlists/user.c b/core/playlists/user.c index db03c75c..758ea1cf 100644 --- a/core/playlists/user.c +++ b/core/playlists/user.c @@ -116,12 +116,8 @@ static struct playlist *pl_user_new(const gchar *name) static void pl_user_played(struct track *track) { struct db_entry *dbe, *next; - struct playlist *playlist; - - db_for_each(dbe, next, &user_db) { - playlist = &USER_PLAYLIST(dbe)->pl_playlist; - queue_updated(&playlist->pl_queue, track); - } + db_for_each(dbe, next, &user_db) + playlist_generic_update(&USER_PLAYLIST(dbe)->pl_playlist, track); } diff --git a/core/queue.c b/core/queue.c index b93b1373..de8ba5a7 100644 --- a/core/queue.c +++ b/core/queue.c @@ -100,12 +100,6 @@ static inline void __queue_clear(struct queue *queue, unsigned int n) queue->q_ops->qop_cleared(queue, n); } -static inline void __queue_updated(struct queue *queue, unsigned int pos) -{ - if (queue->q_ops) - queue->q_ops->qop_updated(queue, pos); -} - void queue_init(struct queue *queue, const struct queue_ops *ops, void *data) { queue->q_length = 0; @@ -184,13 +178,3 @@ bool queue_has(struct queue *queue, struct track *track) } return false; } - -void queue_updated(struct queue *queue, struct track *track) -{ - struct queue_iter it; - - queue_for_each(queue, &it) { - if (queue_iter_val(&it) == track) - __queue_updated(queue, it.it_pos); - } -} diff --git a/gui/model.c b/gui/model.c index e5e9e8aa..6d0e2315 100644 --- a/gui/model.c +++ b/gui/model.c @@ -260,7 +260,8 @@ static void __gui_model_set_runtime(void) static gboolean __gui_model_foreach_changed(GtkTreeModel *model, GtkTreePath *path, GtkTreeIter *iter, gpointer data) { - gtk_tree_model_row_changed(model, path, iter); + if (!data || data == gui_model_iter_get_track(iter)) + gtk_tree_model_row_changed(model, path, iter); return FALSE; } @@ -320,26 +321,13 @@ void gui_model_clear(struct playlist *playlist, unsigned int n) gtk_tree_path_free(path); } -void gui_model_update(struct playlist *playlist, unsigned int row) -{ - GtkTreePath *path; - GtkTreeIter iter; - - if (cur_playlist != playlist) - return; - - path = gtk_tree_path_new_from_indices(row, -1); - __gui_model_get_iter(GTK_TREE_MODEL(gui_model), &iter, path); - gtk_tree_model_row_changed(GTK_TREE_MODEL(gui_model), path, &iter); - __gui_model_set_runtime(); - gtk_tree_path_free(path); -} - -void gui_model_update_all(struct playlist *playlist) +void gui_model_update(struct playlist *playlist, struct track *track) { if (cur_playlist == playlist) gtk_tree_model_foreach(GTK_TREE_MODEL(gui_model), - __gui_model_foreach_changed, NULL); + __gui_model_foreach_changed, track); + + __gui_model_set_runtime(); } void gui_model_set_playlist(struct playlist *playlist) diff --git a/gui/playlist.c b/gui/playlist.c index 3baf0ff5..67f3ef35 100644 --- a/gui/playlist.c +++ b/gui/playlist.c @@ -60,11 +60,6 @@ static void __gui_playlist_cleared(struct queue *queue, unsigned int n) __gui_playlist_update_size(queue->q_private); } -static void __gui_playlist_updated(struct queue *queue, unsigned int n) -{ - gui_model_update(queue->q_private, n); -} - struct queue_ops playlist_ops = { .qop_init = __gui_playlist_init, @@ -72,11 +67,10 @@ struct queue_ops playlist_ops = { .qop_added = __gui_playlist_added, .qop_removed = __gui_playlist_removed, .qop_cleared = __gui_playlist_cleared, - .qop_updated = __gui_playlist_updated, }; struct playlist_callbacks playlist_cb = { - .pl_cb_sorted = gui_model_update_all, + .pl_cb_updated = gui_model_update, }; diff --git a/include/core/playlists/generic.h b/include/core/playlists/generic.h index 48a8ece2..92374045 100644 --- a/include/core/playlists/generic.h +++ b/include/core/playlists/generic.h @@ -14,8 +14,11 @@ enum playlist_save_flags { #define PL_SAVE_ALL (PL_SAVE_TRACKS | PL_SAVE_METADATA) struct playlist_callbacks { - /* Called to notify that a playlist has been sorted. */ - void (*pl_cb_sorted)(struct playlist *); + /* + * Called to notify that a track has been updated. + * If the track is NULL, then the entire playlist should be updated. + */ + void (*pl_cb_updated)(struct playlist *, struct track *); }; @@ -44,6 +47,9 @@ bool playlist_generic_add_track_front(struct playlist *, struct track *); /* Generic playlist remove track operation. */ bool playlist_generic_remove_track(struct playlist *, struct track *); +/* Generic playlist update track operation. */ +void playlist_generic_update(struct playlist *, struct track *); + /* Generic playlist set_random operation. */ void playlist_generic_set_random(struct playlist *, bool); diff --git a/include/core/queue.h b/include/core/queue.h index 49f4306d..751baf61 100644 --- a/include/core/queue.h +++ b/include/core/queue.h @@ -29,9 +29,6 @@ struct queue_ops { /* Called to tell a higher layer that the queue has been cleared. */ void (*qop_cleared)(struct queue *, unsigned int); - - /* Called to tell a higher layer that a track has been updated. */ - void (*qop_updated)(struct queue *, unsigned int); }; @@ -131,7 +128,4 @@ void queue_clear(struct queue *); /* Called to check if a queue has a track. */ bool queue_has(struct queue *, struct track *); -/* Called to tell the queue that a track has been updated. */ -void queue_updated(struct queue *, struct track *); - #endif /* OCARINA_CORE_QUEUE_H */ diff --git a/include/gui/model.h b/include/gui/model.h index e0aa8ea5..832685bd 100644 --- a/include/gui/model.h +++ b/include/gui/model.h @@ -60,11 +60,11 @@ void gui_model_remove(struct playlist *, unsigned int); /* Called to remove all rows from the model */ void gui_model_clear(struct playlist *, unsigned int); -/* Called to update a row in the model */ -void gui_model_update(struct playlist *, unsigned int); - -/* Called to update all rows in the model */ -void gui_model_update_all(struct playlist *); +/* + * Called to update a row in the model + * If track is NULL, then all rows will be updated + */ +void gui_model_update(struct playlist *, struct track *); /* Called to change the queue represented by the model. */ void gui_model_set_playlist(struct playlist *); diff --git a/tests/core/playlist.c b/tests/core/playlist.c index c635adba..65787073 100644 --- a/tests/core/playlist.c +++ b/tests/core/playlist.c @@ -6,10 +6,14 @@ #include #include -static struct playlist *last_sort = NULL; +static struct playlist *cb_playlist = NULL; +static struct track *cb_track = NULL; -static void test_pl_sorted(struct playlist *playlist) - { last_sort = playlist; } +static void test_pl_updated(struct playlist *playlist, struct track *track) +{ + cb_playlist = playlist; + cb_track = track; +} static struct playlist_ops test_noop; static struct playlist_ops test_ops = { @@ -18,7 +22,7 @@ static struct playlist_ops test_ops = { .pl_sort = playlist_generic_sort, }; static struct playlist_callbacks test_cb = { - .pl_cb_sorted = test_pl_sorted, + .pl_cb_updated = test_pl_updated, }; static void test_null() @@ -52,10 +56,33 @@ static void test_null() playlist_generic_resort(NULL); playlist_clear_sort(NULL); + playlist_generic_update(NULL, NULL); + playlist_generic_save(NULL, NULL, PL_SAVE_ALL); playlist_generic_load(NULL, NULL, PL_SAVE_ALL); } +static void test_playlist() +{ + struct playlist p = DEFINE_PLAYLIST(PL_MAX_TYPE, "Test", 0, &test_ops); + unsigned int i; + + playlist_generic_init(&p, NULL); + + for (i = 0; i < 13; i++) + playlist_generic_add_track_front(&p, track_get(i)); + + /* Trigger an update for each track. */ + for (i = 0; i < 13; i++) { + playlist_generic_update(&p, track_get(i)); + g_assert(cb_playlist == &p); + g_assert(cb_track == track_get(i)); + } + playlist_generic_update(&p, NULL); + g_assert(cb_playlist == &p); + g_assert_null(cb_track); +} + static void test_sorting() { struct playlist p = DEFINE_PLAYLIST(PL_MAX_TYPE, "Test", 0, &test_ops); @@ -66,7 +93,7 @@ static void test_sorting() g_assert_cmpuint(g_slist_length(p.pl_queue.q_sort), ==, 3); playlist_clear_sort(&p); g_assert_cmpuint(g_slist_length(p.pl_queue.q_sort), ==, 0); - last_sort = NULL; + cb_playlist = NULL; for (i = 0; i < 13; i++) { track = track_get(i); @@ -76,11 +103,13 @@ static void test_sorting() p.pl_ops = &test_noop; g_assert_false(playlist_sort(&p, COMPARE_TRACK)); - g_assert_null(last_sort); + g_assert_null(cb_playlist); + g_assert_null(cb_track); p.pl_ops = &test_ops; g_assert_true(playlist_sort(&p, COMPARE_TRACK)); - g_assert(last_sort == &p); + g_assert(cb_playlist == &p); + g_assert_null(cb_track); for (i = 0; i < 13; i++) { track = queue_at(&p.pl_queue, i); g_assert_cmpuint(track->tr_track, ==, i + 1); @@ -227,6 +256,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/General", test_playlist); 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); diff --git a/tests/core/queue.c b/tests/core/queue.c index e917357e..6ac2b1d9 100644 --- a/tests/core/queue.c +++ b/tests/core/queue.c @@ -12,7 +12,6 @@ unsigned int count_deinit = 0; unsigned int count_added = 0; unsigned int count_deleted = 0; unsigned int count_cleared = 0; -unsigned int count_updated = 0; static void *queue_op_init(struct queue *queue, void *data) @@ -41,11 +40,6 @@ static void queue_op_cleared(struct queue *queue, unsigned int n) count_cleared++; } -static void queue_op_updated(struct queue *queue, unsigned int pos) -{ - count_updated++; -} - static const struct queue_ops test_ops = { .qop_init = queue_op_init, @@ -53,7 +47,6 @@ static const struct queue_ops test_ops = { .qop_added = queue_op_added, .qop_removed = queue_op_removed, .qop_cleared = queue_op_cleared, - .qop_updated = queue_op_updated, }; @@ -105,7 +98,6 @@ static void test_queue(gconstpointer arg) count_added = 0; count_deleted = 0; count_cleared = 0; - count_updated = 0; queue_init(&q, &test_ops, NULL); @@ -160,11 +152,6 @@ static void test_queue(gconstpointer arg) g_assert_cmpuint(q.q_length, ==, ex_length); g_assert_cmpuint(queue_size(&q), ==, ex_size); - /* queue_updated() */ - track = track_get(2); - queue_updated(&q, track); - g_assert_cmpuint(count_updated, ==, N / 13); - 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 4ad19d14..ae61442a 100644 --- a/tests/gui/filter.c +++ b/tests/gui/filter.c @@ -18,8 +18,6 @@ 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_queue_update(struct queue *queue, unsigned int n) - { gui_model_update(queue->q_private, n); } struct queue_ops test_ops = { .qop_init = test_queue_init, @@ -27,7 +25,6 @@ struct queue_ops test_ops = { .qop_added = test_queue_add, .qop_removed = test_queue_remove, .qop_cleared = test_queue_clear, - .qop_updated = test_queue_update, }; void test_on_load(struct track *track) {} diff --git a/tests/gui/model.c b/tests/gui/model.c index f925e350..5e334ecd 100644 --- a/tests/gui/model.c +++ b/tests/gui/model.c @@ -33,8 +33,6 @@ 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_queue_update(struct queue *queue, unsigned int n) - { gui_model_update(queue->q_private, n); } void test_on_load(struct track *track) {} void test_on_state_change(GstState state) {} void test_on_config_pause(int count) {} @@ -45,11 +43,10 @@ struct queue_ops test_ops = { .qop_added = test_queue_add, .qop_removed = test_queue_remove, .qop_cleared = test_queue_clear, - .qop_updated = test_queue_update, }; struct playlist_callbacks test_cb = { - .pl_cb_sorted = gui_model_update_all, + .pl_cb_updated = gui_model_update, }; struct audio_ops test_audio_ops = {