List refactor and fixes

 - Fix lack of 64-bit int for argument of seek().
 - Fix add() behavior when list is idle but non-empty.

Signed-off-by: Andrew Opalach <andrew@akon.city>

2 files changed, 97 insertions, 71 deletions

diff --git a/src/liana/list.c b/src/liana/list.c
index 42fcb91..2193360 100644
--- a/src/liana/list.c
+++ b/src/liana/list.c
@@ -44,27 +44,43 @@ void lia_list_init(struct lia_list *list, str *name)
     list->cmd = NULL;
 }
 
-#define META_OPAQUE(op) ((u8[]){ op })
+// These small functions may seem excessive but their purpose is an attempt
+// to reduce noise in parts that are harder to understand.
 
-static void signal_meta(struct lia_list *list, struct lia_list_entry *entry, u8 meta)
+static inline void list_signal_meta(struct lia_list *list, struct lia_list_entry *entry, u8 meta)
 {
-    list->callback(list->userdata, LIANA_LIST_META, entry, META_OPAQUE(meta));
+    list->callback(list->userdata, LIANA_LIST_META, entry, (u8[]){ meta });
 }
 
-static void entry_unload(struct lia_list *list, struct lia_list_entry *entry)
+static inline void list_add_entry(struct lia_list *list, struct lia_list_entry *entry)
+{
+    al_array_push(list->entries, entry);
+    list_signal_meta(list, entry, LIANA_META_ADDED_ENTRY);
+}
+
+static inline void entry_unload(struct lia_list *list, struct lia_list_entry *entry)
 {
     list->callback(list->userdata, LIANA_UNLOAD_ENTRY, entry, NULL);
 }
 
+static inline void entry_free(struct lia_list_entry *entry)
+{
+    al_str_free(&entry->brief);
+    al_free(entry);
+}
+
 static bool entry_load_and_get_duration(struct lia_list *list, struct lia_list_entry *entry, s32 sequence, bool *error)
 {
     u8 status;
     list->callback(list->userdata, LIANA_LOAD_ENTRY, entry, &status);
     if (status == LIANA_ENTRY_ERRORED) {
-        // If sequence is <0 that must mean entry is not contained in list->entries.
+        // If sequence is <0 that must mean entry is not yet added to list->entries.
         if (sequence >= 0) {
             al_array_remove_at(list->entries, (u32)sequence);
             if (list->current > sequence) {
+                // @TODO: Consider this when implementing list_remove().
+                //  In the case of remove sequence 0, causing a skip to sequence 1, and sequence 1
+                //  fails to load, do we properly move to -1 and an idle list.
                 list->current--;
             }
             struct lia_list_cmd *cmd = list->cmd;
@@ -79,9 +95,10 @@ static bool entry_load_and_get_duration(struct lia_list *list, struct lia_list_e
             }
         }
         *error = true;
-        signal_meta(list, entry, LIANA_META_ENTRY_ERRORED);
+        list_signal_meta(list, entry, LIANA_META_ENTRY_ERRORED);
         al_assert(!al_array_contains(list->entries, entry));
         entry_unload(list, entry);
+        entry_free(entry);
         return false;
     }
     *error = false;
@@ -92,12 +109,12 @@ static bool entry_load_and_get_duration(struct lia_list *list, struct lia_list_e
     return false;
 }
 
-static void entry_ref(struct lia_list *list, struct lia_list_entry *entry)
+static inline void entry_ref(struct lia_list *list, struct lia_list_entry *entry)
 {
     list->callback(list->userdata, LIANA_REF_ENTRY, entry, NULL);
 }
 
-static void entry_unref(struct lia_list *list, struct lia_list_entry *entry)
+static inline void entry_unref(struct lia_list *list, struct lia_list_entry *entry)
 {
     list->callback(list->userdata, LIANA_UNREF_ENTRY, entry, NULL);
 }
@@ -112,39 +129,40 @@ static void unref_all_entries(struct lia_list *list)
     }
 }
 
-static void sink_set(struct lia_list_sink *sink, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
+static inline void sink_set_entry(struct lia_list_sink *sink, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
 {
     sink->set = sequence;
     sink->callback(sink->userdata, LIANA_SINK_SET, entry, sequence, time);
 }
 
-static void sink_seek(struct lia_list_sink *sink, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
+static inline void sink_seek_entry(struct lia_list_sink *sink, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
 {
     sink->callback(sink->userdata, LIANA_SINK_SEEK, entry, sequence, time);
 }
 
-static void sink_toggle_pause(struct lia_list_sink *sink, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
+static inline void sink_pause_entry(struct lia_list_sink *sink, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
 {
     sink->callback(sink->userdata, LIANA_SINK_PAUSE, entry, sequence, time);
 }
 
-static void sink_unset(struct lia_list_sink *sink)
+static inline void sink_unset_entry(struct lia_list_sink *sink)
 {
     sink->callback(sink->userdata, LIANA_SINK_UNSET, NULL, -1, NULL);
 }
 
-static void set_current(struct lia_list *list, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
+static void list_set_current(struct lia_list *list, struct lia_list_entry *entry, s32 sequence, struct lia_timing *time)
 {
     entry_ref(list, entry);
     list->current = sequence;
     list->idle = false;
-    // @TODO: Skipping back and forth between 2 entries will unload everything else.
+    // @TODO: This is incorrect and unfinished.
+    //  Skipping back and forth between 2 entries will unload everything else.
     unref_all_entries(list);
     struct lia_list_sink *sink;
     al_array_foreach(list->sinks, i, sink) {
-        sink_set(sink, entry, sequence, time);
+        sink_set_entry(sink, entry, sequence, time);
     }
-    signal_meta(list, entry, LIANA_META_CURRENT_CHANGED);
+    list_signal_meta(list, entry, LIANA_META_CURRENT_CHANGED);
 }
 
 static void pump_queue(struct lia_list *list);
@@ -175,7 +193,7 @@ static bool handle_add_sink(struct lia_list *list, struct lia_list_sink *sink)
             .pause = pause,
             .ended = current->ended
         };
-        sink_set(sink, current, list->current, &time);
+        sink_set_entry(sink, current, list->current, &time);
     }
     al_array_push(list->sinks, sink);
     return true;
@@ -208,31 +226,6 @@ static void handle_remove_sink(struct lia_list *list, void *userdata)
     }
 }
 
-static bool handle_add(struct lia_list *list, struct lia_list_entry *entry)
-{
-    if (list->idle) {
-        bool error;
-        if (!entry_load_and_get_duration(list, entry, -1, &error)) {
-            return error;
-        }
-        entry->start = nn_get_timestamp() + LIANA_BASE_DELAY;
-    } else {
-        entry->start = LIANA_TIMESTAMP_INVALID;
-    }
-    al_array_push(list->entries, entry);
-    signal_meta(list, entry, LIANA_META_ADDED_ENTRY);
-    if (list->idle) {
-        struct lia_timing time = {
-            .at = entry->start,
-            .seek_pos = entry->offset,
-            .pause = LIANA_PAUSE_RESUME,
-            .ended = false
-        };
-        set_current(list, entry, list->current + 1, &time);
-    }
-    return true;
-}
-
 static struct lia_list_entry *get_entry_from_sequence(struct lia_list *list, s32 sequence)
 {
     s32 size = (s32)list->entries.count;
@@ -287,10 +280,7 @@ static bool handle_skipto(struct lia_list *list, s32 sequence, s32 index)
         // Target was never started before.
         target->start = at;
     } else {
-        // @TODO: I've observed this tripping when target->ended is true.
-        //  Possibly related to clients having network latency and/or high video processing load.
-        //  Maybe also exclusive to SINK_LOCAL mode.
-        // Target is paused or static.
+        // Target is static or paused. This is strict but should hold true.
         al_assert(target->duration == 0 || target->paused_at != LIANA_TIMESTAMP_INVALID);
     }
 
@@ -335,8 +325,45 @@ static bool handle_skipto(struct lia_list *list, s32 sequence, s32 index)
         .ended = target->ended
     };
 
-    set_current(list, target, index, &time);
+    list_set_current(list, target, index, &time);
+
+    return true;
+}
 
+static bool handle_add(struct lia_list *list, struct lia_list_entry *entry)
+{
+    // @TODO: This is an easy spot to preload an entry.
+    //  Just fire an entry_load_and_get_duration() but ignore the immediate result.
+    if (list->idle) {
+        bool error;
+        if (!entry_load_and_get_duration(list, entry, -1, &error)) {
+            // We passed a sequence of -1 so, on error, do not touch list->entries.
+            return error;
+        }
+    }
+    list_add_entry(list, entry);
+    if (list->idle) {
+        if (list->current == -1) { // Start the list.
+            entry->start = nn_get_timestamp() + LIANA_BASE_DELAY;
+            struct lia_timing time = {
+                .at = entry->start,
+                .seek_pos = entry->offset,
+                .pause = LIANA_PAUSE_RESUME,
+                .ended = false
+            };
+            list_set_current(list, entry, list->current + 1, &time);
+        } else { // Immediately skip to the added entry.
+            // Processing this through a SKIPTO is extremely important for consistency.
+            // We expect current is ended but it still must be held before moving to this entry.
+            // -2 cause we just added this entry above.
+            al_assert((u32)list->current == (list->entries.count - 2));
+            struct lia_list_cmd *cmd = list->cmd;
+            cmd->op = SKIPTO;
+            cmd->sequence = list->current;
+            cmd->arg0.i = list->current + 1;
+            return handle_skipto(list, cmd->sequence, cmd->arg0.i);
+        }
+    }
     return true;
 }
 
@@ -394,10 +421,10 @@ static void handle_toggle_pause(struct lia_list *list, s32 sequence, f64 pts)
 
     struct lia_list_sink *sink;
     al_array_foreach(list->sinks, i, sink) {
-        sink_toggle_pause(sink, entry, sequence, &time);
+        sink_pause_entry(sink, entry, sequence, &time);
     }
 
-    signal_meta(list, entry, LIANA_META_ENTRY_PAUSED);
+    list_signal_meta(list, entry, LIANA_META_ENTRY_PAUSED);
 }
 
 static void handle_seek(struct lia_list *list, s32 sequence, u32 id, u64 pos)
@@ -443,12 +470,13 @@ static void handle_seek(struct lia_list *list, s32 sequence, u32 id, u64 pos)
     struct lia_list_sink *sink;
     al_array_foreach(list->sinks, i, sink) {
         if (sequence == list->current && sink->set != sequence) {
-            sink_set(sink, entry, sequence, &time);
+            // Entry might not be set if the sink was added after it ended.
+            sink_set_entry(sink, entry, sequence, &time);
         }
-        sink_seek(sink, entry, sequence, &time);
+        sink_seek_entry(sink, entry, sequence, &time);
     }
 
-    signal_meta(list, entry, LIANA_META_ENTRY_SEEKED);
+    list_signal_meta(list, entry, LIANA_META_ENTRY_SEEKED);
 }
 
 static bool handle_end(struct lia_list *list, u32 id, u32 reset_id)
@@ -457,10 +485,10 @@ static bool handle_end(struct lia_list *list, u32 id, u32 reset_id)
     if (sequence < 0) return true;
     struct lia_list_entry *entry = get_entry_from_sequence(list, sequence);
 
-    // Looping:
-    //  - Main issue is rolling back an entry that skipped onto queued
-    //    before it's looping state was synced. Maybe we can track which
-    //    sink END comes from.
+    // @TODO: Looping.
+    //  Main issue is rolling back an entry that skipped onto queued
+    //  before it's looping state was synced. If we track which sink END
+    //  is coming from, we could probably handle it then.
 
     if (reset_id != entry->reset_id) {
         log_warn("Got end() with out of order or incorrect reset id, ignoring.");
@@ -482,8 +510,8 @@ static bool handle_end(struct lia_list *list, u32 id, u32 reset_id)
     cmd->op = SEEK;
     cmd->sequence = sequence;
     cmd->arg0.u = id;
-    cmd->arg1.u = 0;
-    handle_seek(list, cmd->sequence, cmd->arg0.u, cmd->arg1.u);
+    cmd->arg1.l = 0;
+    handle_seek(list, cmd->sequence, cmd->arg0.u, cmd->arg1.l);
     return true;
 #endif
 
@@ -499,7 +527,7 @@ static bool handle_end(struct lia_list *list, u32 id, u32 reset_id)
             }
             list->current = list->queued;
             list->queued = -1;
-            signal_meta(list, queued, LIANA_META_CURRENT_CHANGED);
+            list_signal_meta(list, queued, LIANA_META_CURRENT_CHANGED);
         } else if (next < size) {
             struct lia_list_cmd *cmd = list->cmd;
             cmd->op = SKIPTO;
@@ -532,7 +560,7 @@ static bool adjust_current(struct lia_list *list, struct lia_list_entry *previou
         }
     }
     al_assert(entry && cmd->op == SKIPTO);
-    signal_meta(list, previous, LIANA_META_ORDER_CHANGED);
+    list_signal_meta(list, previous, LIANA_META_ORDER_CHANGED);
     return handle_skipto(list, cmd->sequence, cmd->arg0.i);
 }
 
@@ -591,7 +619,7 @@ static void handle_unset(struct lia_list *list)
     struct lia_list_sink *sink;
     al_array_foreach(list->sinks, i, sink) {
         sink->set = -1;
-        sink_unset(sink);
+        sink_unset_entry(sink);
     }
 }
 
@@ -631,10 +659,10 @@ static void run_queue(struct lia_list *list)
         }
         break;
     case TOGGLE_PAUSE:
-        handle_toggle_pause(list, cmd->sequence, cmd->argf);
+        handle_toggle_pause(list, cmd->sequence, cmd->arg1.f);
         break;
     case SEEK:
-        handle_seek(list, cmd->sequence, cmd->arg0.u, cmd->arg1.u);
+        handle_seek(list, cmd->sequence, cmd->arg0.u, cmd->arg1.l);
         break;
     case END:
         if (!handle_end(list, cmd->arg0.u, cmd->arg1.u)) {
@@ -748,7 +776,7 @@ void lia_list_toggle_pause(struct lia_list *list, s32 sequence, f64 pts)
     struct lia_list_cmd *cmd = al_alloc_object(struct lia_list_cmd);
     cmd->op = TOGGLE_PAUSE;
     cmd->sequence = sequence;
-    cmd->argf = pts;
+    cmd->arg1.f = pts;
     al_array_push(list->command_queue, cmd);
     pump_queue(list);
 }
@@ -759,7 +787,7 @@ void lia_list_seek(struct lia_list *list, s32 sequence, u32 id, u64 pos)
     cmd->op = SEEK;
     cmd->sequence = sequence;
     cmd->arg0.u = id;
-    cmd->arg1.u = pos;
+    cmd->arg1.l = pos;
     al_array_push(list->command_queue, cmd);
     pump_queue(list);
 }
@@ -819,7 +847,7 @@ void lia_list_clear(struct lia_list *list)
 void lia_list_close(struct lia_list *list)
 {
     // @TODO: Consider sinks being in use.
-    //  Wait for list->sinks to be empty?
+    //  Delay until list->sinks is empty.
     struct lia_list_entry *entry;
     al_array_foreach(list->entries, i, entry) {
         entry_unload(list, entry);
@@ -836,8 +864,7 @@ void lia_list_free(struct lia_list *list)
     if (list->cmd) al_free(list->cmd);
     struct lia_list_entry *entry;
     al_array_foreach(list->entries, i, entry) {
-        al_str_free(&entry->brief);
-        al_free(entry);
+        entry_free(entry);
     }
     al_array_free(list->entries);
     struct lia_list_sink *sink;
diff --git a/src/liana/list.h b/src/liana/list.h
index 36d95a4..bfcd240 100644
--- a/src/liana/list.h
+++ b/src/liana/list.h
@@ -19,7 +19,7 @@
 // @TODO:
 //  To handle an entry being queued right before a skip, keep a global
 //   "max time until all sinks buffered" and used that instead of LIANA_PAUSE_DELAY (if greater).
-//  Factor in LIANA_BASE_PING
+//  Factor in LIANA_BASE_PING.
 
 enum {
     LIANA_SINK_SET = 0,
@@ -100,8 +100,7 @@ struct lia_list_cmd {
     struct lia_list_entry *entry;
     s32 sequence;
     union { s32 i; u32 u; } arg0;
-    union { s32 i; u32 u; } arg1;
-    f64 argf;
+    union { f64 f; u64 l; u32 u; } arg1;
 };
 
 struct lia_list {