[pulseaudio-discuss] [PATCH] null-source: fix multiple bugs

[Georg Chini]

The current null-source implementation has several bugs:

1) The latency reported is the negative of the correct latency.
2) The memchunk passed to pa_source_post() is not initialized
with silence.
3) In PA_SOURCE_MESSAGE_SET_STATE the timestamp is always set
when the source transitions to RUNNING state. This should only
happen when the source transitions from SUSPENDED to RUNNING
but also if it changes from SUSPENDED to IDLE.
4) The timing of the thread function is incorrect. It always
uses u->latency_time, regardless of the specified source
latency.
5) The latency_time argument seems pointless because the source
is defined with dynamic latency.

This patch fixes the issues by
1) inverting the sign of the reported latency,
2) initializing the memchunk with silence,
3) changing the logic in PA_SOURCE_MESSAGE_SET_STATE so that
the timestamp is set when needed,
4) using u->block_usec instead of u->latency_time for setting
the rtpoll timer and checking if the timer has elapsed,
5) removing the latency_time option.

Thanks to Raman Shishniou for his reviews and contribution.
---
 src/modules/module-null-source.c | 40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/src/modules/module-null-source.c b/src/modules/module-null-source.c
index 41f17bd9..c28304ca 100644
--- a/src/modules/module-null-source.c
+++ b/src/modules/module-null-source.c
@@ -51,12 +51,11 @@ PA_MODULE_USAGE(
         "rate=<sample rate> "
         "source_name=<name of source> "
         "channel_map=<channel map> "
-        "description=<description for the source> "
-        "latency_time=<latency time in ms>");
+        "description=<description for the source> ");
 
 #define DEFAULT_SOURCE_NAME "source.null"
-#define DEFAULT_LATENCY_TIME 20
 #define MAX_LATENCY_USEC (PA_USEC_PER_SEC * 2)
+#define MIN_LATENCY_USEC (500)
 
 struct userdata {
     pa_core *core;
@@ -71,7 +70,6 @@ struct userdata {
 
     pa_usec_t block_usec;
     pa_usec_t timestamp;
-    pa_usec_t latency_time;
 };
 
 static const char* const valid_modargs[] = {
@@ -81,7 +79,6 @@ static const char* const valid_modargs[] = {
     "source_name",
     "channel_map",
     "description",
-    "latency_time",
     NULL
 };
 
@@ -91,8 +88,10 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
     switch (code) {
         case PA_SOURCE_MESSAGE_SET_STATE:
 
-            if (PA_PTR_TO_UINT(data) == PA_SOURCE_RUNNING)
-                u->timestamp = pa_rtclock_now();
+            if (u->source->thread_info.state == PA_SOURCE_SUSPENDED || u->source->thread_info.state == PA_SOURCE_INIT) {
+                if (PA_SOURCE_IS_OPENED(PA_PTR_TO_UINT(data)))
+                    u->timestamp = pa_rtclock_now();
+            }
 
             break;
 
@@ -100,7 +99,7 @@ static int source_process_msg(pa_msgobject *o, int code, void *data, int64_t off
             pa_usec_t now;
 
             now = pa_rtclock_now();
-            *((int64_t*) data) = (int64_t)u->timestamp - (int64_t)now;
+            *((int64_t*) data) = (int64_t)now - (int64_t)u->timestamp;
 
             return 0;
         }
@@ -117,10 +116,13 @@ static void source_update_requested_latency_cb(pa_source *s) {
     pa_assert(u);
 
     u->block_usec = pa_source_get_requested_latency_within_thread(s);
+    if (u->block_usec == (pa_usec_t)-1)
+        u->block_usec = u->source->thread_info.max_latency;
 }
 
 static void thread_func(void *userdata) {
     struct userdata *u = userdata;
+    bool timer_elapsed = false;
 
     pa_assert(u);
 
@@ -140,17 +142,19 @@ static void thread_func(void *userdata) {
 
             now = pa_rtclock_now();
 
-            if ((chunk.length = pa_usec_to_bytes(now - u->timestamp, &u->source->sample_spec)) > 0) {
+            if (timer_elapsed && (chunk.length = pa_usec_to_bytes(now - u->timestamp, &u->source->sample_spec)) > 0) {
 
-                chunk.memblock = pa_memblock_new(u->core->mempool, (size_t) -1); /* or chunk.length? */
+                chunk.length = PA_MIN(pa_mempool_block_size_max(u->core->mempool), chunk.length);
+                chunk.memblock = pa_memblock_new(u->core->mempool, chunk.length);
                 chunk.index = 0;
+                pa_silence_memchunk(&chunk, &u->source->sample_spec);
                 pa_source_post(u->source, &chunk);
                 pa_memblock_unref(chunk.memblock);
 
-                u->timestamp = now;
+                u->timestamp += pa_bytes_to_usec(chunk.length, &u->source->sample_spec);
             }
 
-            pa_rtpoll_set_timer_absolute(u->rtpoll, u->timestamp + u->latency_time * PA_USEC_PER_MSEC);
+            pa_rtpoll_set_timer_absolute(u->rtpoll, u->timestamp + u->block_usec);
         } else
             pa_rtpoll_set_timer_disabled(u->rtpoll);
 
@@ -158,6 +162,8 @@ static void thread_func(void *userdata) {
         if ((ret = pa_rtpoll_run(u->rtpoll)) < 0)
             goto fail;
 
+        timer_elapsed = pa_rtpoll_timer_elapsed(u->rtpoll);
+
         if (ret == 0)
             goto finish;
     }
@@ -178,7 +184,6 @@ int pa__init(pa_module*m) {
     pa_channel_map map;
     pa_modargs *ma = NULL;
     pa_source_new_data data;
-    uint32_t latency_time = DEFAULT_LATENCY_TIME;
 
     pa_assert(m);
 
@@ -221,13 +226,6 @@ int pa__init(pa_module*m) {
         goto fail;
     }
 
-    u->latency_time = DEFAULT_LATENCY_TIME;
-    if (pa_modargs_get_value_u32(ma, "latency_time", &latency_time) < 0) {
-        pa_log("Failed to parse latency_time value.");
-        goto fail;
-    }
-    u->latency_time = latency_time;
-
     u->source->parent.process_msg = source_process_msg;
     u->source->update_requested_latency = source_update_requested_latency_cb;
     u->source->userdata = u;
@@ -235,7 +233,7 @@ int pa__init(pa_module*m) {
     pa_source_set_asyncmsgq(u->source, u->thread_mq.inq);
     pa_source_set_rtpoll(u->source, u->rtpoll);
 
-    pa_source_set_latency_range(u->source, 0, MAX_LATENCY_USEC);
+    pa_source_set_latency_range(u->source, MIN_LATENCY_USEC, MAX_LATENCY_USEC);
     u->block_usec = u->source->thread_info.max_latency;
 
     u->source->thread_info.max_rewind =
-- 
2.14.1