From 8bf5aefd1af7797995e122035a86ca5c78089e06 Mon Sep 17 00:00:00 2001 From: Anna Schumaker Date: Thu, 22 Sep 2016 10:54:08 -0400 Subject: [PATCH] core/playlist: Replace queue_iter with a playlist_iter The iterator only needs to point to the current position on the GQueue, so we can do away with manual position tracking. This lets us use a GList pointer as an iterator instead of something more complicated. Signed-off-by: Anna Schumaker --- core/playlists/generic.c | 30 ++++++--------- core/playlists/library.c | 10 ++--- core/playlists/system.c | 4 +- gui/treeview.c | 6 +-- include/core/playlists/iterator.h | 63 ++++++++++++++++--------------- include/core/playlists/playlist.h | 3 +- include/core/queue.h | 6 --- tests/core/playlist.c | 45 ++++++++++------------ 8 files changed, 75 insertions(+), 92 deletions(-) diff --git a/core/playlists/generic.c b/core/playlists/generic.c index accca7c8..935df560 100644 --- a/core/playlists/generic.c +++ b/core/playlists/generic.c @@ -41,9 +41,8 @@ void playlist_generic_set_callbacks(struct playlist_callbacks *cb) void playlist_generic_init(struct playlist *playlist, struct queue_ops *ops) { queue_init(&playlist->pl_queue, ops, playlist); - playlist->pl_sort = NULL; - playlist->pl_current.it_iter = NULL; - playlist->pl_current.it_pos = UINT_MAX; + playlist->pl_sort = NULL; + playlist->pl_current = NULL; playlist_generic_sort(playlist, COMPARE_ARTIST); playlist_generic_sort(playlist, COMPARE_YEAR); playlist_generic_sort(playlist, COMPARE_TRACK); @@ -63,7 +62,7 @@ void playlist_generic_deinit(struct playlist *playlist) void playlist_generic_save(struct playlist *playlist, struct file *file, unsigned int flags) { - struct queue_iter it; + playlist_iter it; GSList *sort; int field; @@ -71,7 +70,7 @@ void playlist_generic_save(struct playlist *playlist, struct file *file, return; if (flags & PL_SAVE_ITER) - file_writef(file, "%u ", playlist->pl_current.it_pos); + file_writef(file, "%u ", playlist_current_index(playlist)); if (flags & PL_SAVE_FLAGS) { sort = playlist->pl_sort; @@ -87,9 +86,9 @@ void playlist_generic_save(struct playlist *playlist, struct file *file, if (flags & PL_SAVE_TRACKS) { file_writef(file, "%u", playlist_size(playlist)); - playlist_for_each(playlist, &it) + playlist_for_each(playlist, it) file_writef(file, " %u", - track_index(playlist_iter_track(&it))); + track_index(playlist_iter_track(it))); file_writef(file, "\n"); } } @@ -152,9 +151,8 @@ void playlist_generic_clear(struct playlist *playlist) n = playlist_size(playlist); g_queue_clear(&playlist->pl_queue.q_tracks); - playlist->pl_length = 0; - playlist->pl_current.it_iter = NULL; - playlist->pl_current.it_pos = UINT_MAX; + playlist->pl_length = 0; + playlist->pl_current = NULL; if (callbacks) callbacks->pl_cb_removed(playlist, NULL, n); } @@ -168,9 +166,6 @@ bool playlist_generic_add(struct playlist *playlist, struct track *track) if (playlist->pl_sort) { g_queue_insert_sorted(&playlist->pl_queue.q_tracks, track, __playlist_generic_less_than, playlist); - playlist->pl_current.it_pos = g_queue_link_index( - &playlist->pl_queue.q_tracks, - playlist->pl_current.it_iter); } else g_queue_push_tail(&playlist->pl_queue.q_tracks, track); @@ -202,10 +197,7 @@ bool playlist_generic_remove(struct playlist *playlist, struct track *track) playlist_current_previous(playlist); count = g_queue_remove_all(&playlist->pl_queue.q_tracks, track); - playlist->pl_length -= (count * track->tr_length); - playlist->pl_current.it_pos = g_queue_link_index( - &playlist->pl_queue.q_tracks, - playlist->pl_current.it_iter); + playlist->pl_length -= (count * track->tr_length); if (callbacks) callbacks->pl_cb_removed(playlist, track, count); @@ -255,8 +247,8 @@ struct track *playlist_generic_next(struct playlist *playlist) if (size == 0) return NULL; else if (playlist->pl_random) { - pos = g_random_int_range(1, size); - pos += playlist->pl_current.it_pos; + pos = playlist_current_index(playlist); + pos += g_random_int_range(1, size); playlist_current_set(playlist, pos % size); } else if (!playlist_current_next(playlist)) playlist_current_set(playlist, 0); diff --git a/core/playlists/library.c b/core/playlists/library.c index 50907e1a..0457e2c0 100644 --- a/core/playlists/library.c +++ b/core/playlists/library.c @@ -172,15 +172,15 @@ static bool __lib_pl_update(void *data) static bool pl_library_delete(struct playlist *playlist) { struct library *library = library_lookup(playlist->pl_name); - struct queue_iter it; + playlist_iter it; if (!library) return false; - playlist_for_each(playlist, &it) { - pl_system_delete_track(playlist_iter_track(&it)); - pl_artist_delete_track(playlist_iter_track(&it)); - pl_user_delete_track(playlist_iter_track(&it)); + playlist_for_each(playlist, it) { + pl_system_delete_track(playlist_iter_track(it)); + pl_artist_delete_track(playlist_iter_track(it)); + pl_user_delete_track(playlist_iter_track(it)); } __lib_pl_free(playlist); diff --git a/core/playlists/system.c b/core/playlists/system.c index 37dfdd5e..67fcf74f 100644 --- a/core/playlists/system.c +++ b/core/playlists/system.c @@ -390,8 +390,6 @@ void pl_system_init(struct queue_ops *ops) struct playlist *playlist; unsigned int i; - idle_schedule(IDLE_SYNC, __sys_pl_load_new, NULL); - for (i = 0; i < SYS_PL_NUM_PLAYLISTS; i++) { playlist = pl_system_get(i); @@ -411,6 +409,8 @@ void pl_system_init(struct queue_ops *ops) break; } } + + idle_schedule(IDLE_SYNC, __sys_pl_load_new, NULL); } void pl_system_deinit() diff --git a/gui/treeview.c b/gui/treeview.c index 179d015e..9df4e4cf 100644 --- a/gui/treeview.c +++ b/gui/treeview.c @@ -201,11 +201,9 @@ void gui_treeview_set_playlist(struct playlist *playlist) void gui_treeview_scroll() { - struct playlist *playlist = gui_model_get_playlist(); - GtkTreePath *path; - int pos; + int pos = playlist_current_index(gui_model_get_playlist()); + GtkTreePath *path; - pos = playlist ? playlist->pl_current.it_pos : -1; if (!can_scroll || pos < 0) return; diff --git a/include/core/playlists/iterator.h b/include/core/playlists/iterator.h index e7e533e8..df1e8af4 100644 --- a/include/core/playlists/iterator.h +++ b/include/core/playlists/iterator.h @@ -1,5 +1,7 @@ /* * Copyright 2016 (c) Anna Schumaker. + * + * NOTE: The playlist_iter type is defined in include/core/playlists/playlist.h */ #ifndef OCARINA_CORE_PLAYLISTS_ITERATOR_H #define OCARINA_CORE_PLAYLISTS_ITERATOR_H @@ -7,70 +9,71 @@ #include /* Called to set the playlist iterator to a specific position. */ -static inline bool playlist_iter_set(struct playlist *playlist, - struct queue_iter *iter, - unsigned int n) +static inline playlist_iter playlist_iter_get(struct playlist *playlist, + unsigned int n) { - if (!playlist || !iter) - return false; - iter->it_iter = g_queue_peek_nth_link(&playlist->pl_queue.q_tracks, n); - iter->it_pos = n; - return iter->it_iter != NULL; + return playlist ? g_queue_peek_nth_link(&playlist->pl_queue.q_tracks, n) : NULL; } /* Called to advance the requested playlist iterator. */ -static inline bool playlist_iter_next(struct queue_iter *iter) +static inline playlist_iter playlist_iter_next(playlist_iter iter) { - if (!iter) - return false; - iter->it_iter = g_list_next(iter->it_iter); - iter->it_pos++; - return iter->it_iter != NULL; + return g_list_next(iter); } /* Called to get a pointer to the track at the requested iterator. */ -static inline struct track *playlist_iter_track(struct queue_iter *iter) +static inline struct track *playlist_iter_track(playlist_iter iter) { - return (iter && iter->it_iter) ? iter->it_iter->data : NULL; + return iter ? iter->data : NULL; +} + +/* Called to find the playlist index of the requested iterator. */ +static inline int playlist_iter_index(struct playlist *playlist, + playlist_iter iter) +{ + return (playlist && iter) ? g_queue_link_index(&playlist->pl_queue.q_tracks, iter) : -1; } /* Called to iterate over the entire playlist. */ #define playlist_for_each(playlist, it) \ - for (playlist_iter_set(playlist, it, 0); (it)->it_iter; \ - playlist_iter_next(it)) + for (it = playlist_iter_get(playlist, 0); it; it = playlist_iter_next(it)) /* Called to set the index of the current track. */ static inline bool playlist_current_set(struct playlist *playlist, unsigned int n) { - if (!playlist) - return false; - return playlist_iter_set(playlist, &playlist->pl_current, n); + if (playlist) + playlist->pl_current = playlist_iter_get(playlist, n); + return playlist && playlist->pl_current; } /* Called to advance the current track. */ static inline bool playlist_current_next(struct playlist *playlist) { - if (!playlist) - return false; - return playlist_iter_next(&playlist->pl_current); + if (playlist) + playlist->pl_current = playlist_iter_next(playlist->pl_current); + return playlist && playlist->pl_current; } /* Called to rewind the current track. */ static inline bool playlist_current_previous(struct playlist *playlist) { - if (!playlist) - return false; - playlist->pl_current.it_iter = g_list_previous(playlist->pl_current.it_iter); - playlist->pl_current.it_pos--; - return playlist->pl_current.it_iter != NULL; + if (playlist) + playlist->pl_current = g_list_previous(playlist->pl_current); + return playlist && playlist->pl_current; } /* Called to get a pointer to the current track. */ static inline struct track *playlist_current_track(struct playlist *playlist) { - return playlist ? playlist_iter_track(&playlist->pl_current) : NULL; + return playlist ? playlist_iter_track(playlist->pl_current) : NULL; +} + +/* Called to get the playlist index of the current track. */ +static inline int playlist_current_index(struct playlist *playlist) +{ + return playlist ? playlist_iter_index(playlist, playlist->pl_current) : -1; } #endif /* OCARINA_CORE_PLAYLISTS_ITERATOR_H */ diff --git a/include/core/playlists/playlist.h b/include/core/playlists/playlist.h index 5aea3d48..771e6de9 100644 --- a/include/core/playlists/playlist.h +++ b/include/core/playlists/playlist.h @@ -7,6 +7,7 @@ #include #include +typedef GList * playlist_iter; struct playlist; enum playlist_type_t { @@ -49,7 +50,7 @@ struct playlist { unsigned int pl_length; /* This playlist's length, in seconds. */ bool pl_random; /* This playlist's random setting. */ - struct queue_iter pl_current; /* This playlist's current track. */ + playlist_iter pl_current; /* This playlist's current track. */ GSList *pl_sort; /* This playlist's sort order. */ void *pl_private; /* This playlist's private data. */ struct queue pl_queue; /* This playlist's queue of tracks. */ diff --git a/include/core/queue.h b/include/core/queue.h index db33da2c..6d1c6638 100644 --- a/include/core/queue.h +++ b/include/core/queue.h @@ -23,12 +23,6 @@ struct queue_ops { }; -struct queue_iter { - GList *it_iter; /* The current link in the GQueue. */ - guint it_pos; /* The current index into the queue. */ -}; - - struct queue { GQueue q_tracks; /* The queue's list of tracks. */ void *q_private; /* The queue's private data. */ diff --git a/tests/core/playlist.c b/tests/core/playlist.c index 4fcf094f..f9bc5df6 100644 --- a/tests/core/playlist.c +++ b/tests/core/playlist.c @@ -78,22 +78,24 @@ static void test_null() playlist_generic_save(NULL, NULL, PL_SAVE_ALL); playlist_generic_load(NULL, NULL, PL_SAVE_ALL); - g_assert_false(playlist_iter_set(NULL, NULL, 0)); + g_assert_null(playlist_iter_get(NULL, 0)); g_assert_false(playlist_iter_next(NULL)); g_assert_null(playlist_iter_track(NULL)); + g_assert_cmpint(playlist_iter_index(NULL, NULL), ==, -1); g_assert_false(playlist_current_set(NULL, 0)); g_assert_false(playlist_current_next(NULL)); g_assert_false(playlist_current_previous(NULL)); g_assert_null(playlist_current_track(NULL)); + g_assert_cmpint(playlist_current_index(NULL), ==, -1); } static void test_playlist() { struct playlist p = DEFINE_PLAYLIST(PL_MAX_TYPE, "Test", 0, &test_ops); unsigned int ex_length = 0; - struct queue_iter it; - unsigned int i; + playlist_iter it; + int i; g_assert_cmpuint(playlist_size(&p), ==, 0); playlist_generic_init(&p, NULL); @@ -113,11 +115,11 @@ static void test_playlist() /* Trigger an update for each track. */ i = 13; - playlist_for_each(&p, &it) { - g_assert_cmpuint(playlist_iter_track(&it)->tr_track, ==, i--); - playlist_generic_update(&p, playlist_iter_track(&it)); + playlist_for_each(&p, it) { + g_assert_cmpuint(playlist_iter_track(it)->tr_track, ==, i--); + playlist_generic_update(&p, playlist_iter_track(it)); g_assert(cb_playlist == &p); - g_assert(cb_track == playlist_iter_track(&it)); + g_assert(cb_track == playlist_iter_track(it)); } playlist_generic_update(&p, NULL); g_assert(cb_playlist == &p); @@ -132,18 +134,17 @@ static void test_playlist() g_assert(cb_playlist == &p); g_assert(cb_track == track_get(i)); g_assert_cmpuint(p.pl_length, ==, ex_length); - g_assert_cmpuint(p.pl_current.it_pos, ==, (11 - i)); + g_assert_cmpint(playlist_current_index(&p), ==, (11 - i)); g_assert_cmpuint(playlist_size(&p), ==, (12 - i)); if (i < 12) - g_assert_nonnull(p.pl_current.it_iter); + g_assert_nonnull(p.pl_current); } g_assert_false(playlist_generic_remove(&p, NULL)); g_assert_false(playlist_has(&p, track_get(i))); g_assert(cb_playlist == &p); g_assert(cb_track == track_get(12)); g_assert_cmpuint(p.pl_length, ==, ex_length); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); - g_assert_null(p.pl_current.it_iter); + g_assert_null(p.pl_current); /* Re-add the tracks! */ for (i = 0; i < 13; i++) { @@ -164,15 +165,13 @@ static void test_playlist() playlist_generic_clear(&p); g_assert_cmpuint(playlist_size(&p), ==, 0); g_assert_cmpuint(p.pl_length, ==, 0); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); - g_assert_null(p.pl_current.it_iter); + g_assert_null(p.pl_current); /* Deinitialize the playlist. */ playlist_generic_deinit(&p); g_assert_cmpuint(playlist_size(&p), ==, 0); g_assert_cmpuint(p.pl_length, ==, 0); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); - g_assert_null(p.pl_current.it_iter); + g_assert_null(p.pl_current); g_assert_null(p.pl_sort); } @@ -255,8 +254,7 @@ static void test_sorting() playlist_generic_deinit(&p); g_assert_cmpuint(playlist_size(&p), ==, 0); g_assert_cmpuint(p.pl_length, ==, 0); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); - g_assert_null(p.pl_current.it_iter); + g_assert_null(p.pl_current); g_assert_null(p.pl_sort); } @@ -274,7 +272,6 @@ static void test_next() g_assert_true(playlist_select(&p)); g_assert(playlist_current() == &p); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); /* Test playlist_next() with wraparound, but no random. */ for (i = 0; i < (13 * 2); i++) { @@ -301,8 +298,7 @@ static void test_next() playlist_generic_deinit(&p); g_assert_cmpuint(playlist_size(&p), ==, 0); g_assert_cmpuint(p.pl_length, ==, 0); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); - g_assert_null(p.pl_current.it_iter); + g_assert_null(p.pl_current); g_assert_null(p.pl_sort); } @@ -318,6 +314,7 @@ static void test_save_load() for (i = 0; i < 13; i++) { playlist_generic_add(&p, track_get(i)); playlist_generic_add(&q, track_get(i)); + playlist_generic_add(&r, track_get(i)); } playlist_set_random(&p, true); playlist_clear_sort(&p); @@ -337,13 +334,12 @@ static void test_save_load() file_close(&f); g_assert_true(r.pl_random); - g_assert_cmpuint(r.pl_current.it_pos, ==, 3); + g_assert_cmpuint(playlist_current_index(&r), ==, 3); g_assert_cmpuint(g_slist_length(r.pl_sort), ==, 1); g_assert_cmpuint(GPOINTER_TO_UINT(r.pl_sort->data), ==, COMPARE_TRACK); - g_assert_cmpuint(g_queue_get_length(&r.pl_queue.q_tracks), ==, 0); g_assert_false(s.pl_random); - g_assert_cmpuint(s.pl_current.it_pos, ==, 4); + g_assert_cmpuint(playlist_current_index(&s), ==, 4); g_assert(playlist_current_track(&s) == queue_at(&q.pl_queue, 4)); g_assert_cmpuint(g_slist_length(s.pl_sort), ==, 0); g_assert_cmpuint(playlist_size(&s), ==, 13); @@ -352,8 +348,7 @@ static void test_save_load() playlist_generic_deinit(&p); g_assert_cmpuint(playlist_size(&p), ==, 0); g_assert_cmpuint(p.pl_length, ==, 0); - g_assert_cmpuint(p.pl_current.it_pos, ==, UINT_MAX); - g_assert_null(p.pl_current.it_iter); + g_assert_null(p.pl_current); } int main(int argc, char **argv)