[pulseaudio-discuss,7/8] rtpoll: Separate out before/after/work callback userdata

Submitted by Arun Raghavan on Feb. 29, 2016, 10:16 a.m.

Details

Message ID 1456740996-13908-8-git-send-email-arun@accosted.net
State New
Headers show
Series "Add GStreamer-based RTP support" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Arun Raghavan Feb. 29, 2016, 10:16 a.m.
From: Arun Raghavan <git@arunraghavan.net>

It is possible that we might want to have a separate userdata to be used
for these callbacks, so let's split them out.

This is particularly needed when using an pa_rtpoll_item around pa_fdsem
since that uses its own before/after callback but will essentially have
whatever is using the fdsem set up the work callback appropriately (and
thus at least the work callback's userdata needs to be separated from
the before/after callback -- we might as well then just separate all
three).
---
 src/modules/alsa/alsa-mixer.c     |  3 +-
 src/modules/rtp/module-rtp-recv.c |  3 +-
 src/pulsecore/rtpoll.c            | 61 +++++++++++++++++++--------------------
 src/pulsecore/rtpoll.h            |  7 ++---
 src/tests/rtpoll-test.c           | 10 +++----
 5 files changed, 39 insertions(+), 45 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c
index 1fe2a02..9f4ffb6 100644
--- a/src/modules/alsa/alsa-mixer.c
+++ b/src/modules/alsa/alsa-mixer.c
@@ -547,8 +547,7 @@  int pa_alsa_set_mixer_rtpoll(struct pa_alsa_mixer_pdata *pd, snd_mixer_t *mixer,
     pd->poll_item = i;
     pd->mixer = mixer;
 
-    pa_rtpoll_item_set_userdata(i, pd);
-    pa_rtpoll_item_set_work_callback(i, rtpoll_work_cb);
+    pa_rtpoll_item_set_work_callback(i, rtpoll_work_cb, pd);
 
     return 0;
 }
diff --git a/src/modules/rtp/module-rtp-recv.c b/src/modules/rtp/module-rtp-recv.c
index fd25e08..1ee9c91 100644
--- a/src/modules/rtp/module-rtp-recv.c
+++ b/src/modules/rtp/module-rtp-recv.c
@@ -385,8 +385,7 @@  static void sink_input_attach(pa_sink_input *i) {
     pa_assert(!s->rtpoll_item);
     s->rtpoll_item = pa_rtp_context_get_rtpoll_item(s->rtp_context, i->sink->thread_info.rtpoll);
 
-    pa_rtpoll_item_set_work_callback(s->rtpoll_item, rtpoll_work_cb);
-    pa_rtpoll_item_set_userdata(s->rtpoll_item, s);
+    pa_rtpoll_item_set_work_callback(s->rtpoll_item, rtpoll_work_cb, s);
 }
 
 /* Called from I/O thread context */
diff --git a/src/pulsecore/rtpoll.c b/src/pulsecore/rtpoll.c
index 98cf88f..515d881 100644
--- a/src/pulsecore/rtpoll.c
+++ b/src/pulsecore/rtpoll.c
@@ -77,7 +77,9 @@  struct pa_rtpoll_item {
     int (*work_cb)(pa_rtpoll_item *i);
     int (*before_cb)(pa_rtpoll_item *i);
     void (*after_cb)(pa_rtpoll_item *i);
-    void *userdata;
+    void *work_userdata;
+    void *before_userdata;
+    void *after_userdata;
 
     PA_LLIST_FIELDS(pa_rtpoll_item);
 };
@@ -411,7 +413,9 @@  pa_rtpoll_item *pa_rtpoll_item_new(pa_rtpoll *p, pa_rtpoll_priority_t prio, unsi
     i->pollfd = NULL;
     i->priority = prio;
 
-    i->userdata = NULL;
+    i->work_userdata = NULL;
+    i->before_userdata = NULL;
+    i->work_userdata = NULL;
     i->before_cb = NULL;
     i->after_cb = NULL;
     i->work_cb = NULL;
@@ -458,42 +462,39 @@  struct pollfd *pa_rtpoll_item_get_pollfd(pa_rtpoll_item *i, unsigned *n_fds) {
     return i->pollfd;
 }
 
-void pa_rtpoll_item_set_before_callback(pa_rtpoll_item *i, int (*before_cb)(pa_rtpoll_item *i)) {
+void pa_rtpoll_item_set_before_callback(pa_rtpoll_item *i, int (*before_cb)(pa_rtpoll_item *i), void *userdata) {
     pa_assert(i);
     pa_assert(i->priority < PA_RTPOLL_NEVER);
 
     i->before_cb = before_cb;
+    i->before_userdata = userdata;
 }
 
-void pa_rtpoll_item_set_after_callback(pa_rtpoll_item *i, void (*after_cb)(pa_rtpoll_item *i)) {
+void pa_rtpoll_item_set_after_callback(pa_rtpoll_item *i, void (*after_cb)(pa_rtpoll_item *i), void *userdata) {
     pa_assert(i);
     pa_assert(i->priority < PA_RTPOLL_NEVER);
 
     i->after_cb = after_cb;
+    i->after_userdata = userdata;
 }
 
-void pa_rtpoll_item_set_work_callback(pa_rtpoll_item *i, int (*work_cb)(pa_rtpoll_item *i)) {
+void pa_rtpoll_item_set_work_callback(pa_rtpoll_item *i, int (*work_cb)(pa_rtpoll_item *i), void *userdata) {
     pa_assert(i);
     pa_assert(i->priority < PA_RTPOLL_NEVER);
 
     i->work_cb = work_cb;
-}
-
-void pa_rtpoll_item_set_userdata(pa_rtpoll_item *i, void *userdata) {
-    pa_assert(i);
-
-    i->userdata = userdata;
+    i->work_userdata = userdata;
 }
 
 void* pa_rtpoll_item_get_userdata(pa_rtpoll_item *i) {
     pa_assert(i);
 
-    return i->userdata;
+    return i->work_userdata;
 }
 
 static int fdsem_before(pa_rtpoll_item *i) {
 
-    if (pa_fdsem_before_poll(i->userdata) < 0)
+    if (pa_fdsem_before_poll(i->before_userdata) < 0)
         return 1; /* 1 means immediate restart of the loop */
 
     return 0;
@@ -503,7 +504,7 @@  static void fdsem_after(pa_rtpoll_item *i) {
     pa_assert(i);
 
     pa_assert((i->pollfd[0].revents & ~POLLIN) == 0);
-    pa_fdsem_after_poll(i->userdata);
+    pa_fdsem_after_poll(i->after_userdata);
 }
 
 pa_rtpoll_item *pa_rtpoll_item_new_fdsem(pa_rtpoll *p, pa_rtpoll_priority_t prio, pa_fdsem *f) {
@@ -520,9 +521,8 @@  pa_rtpoll_item *pa_rtpoll_item_new_fdsem(pa_rtpoll *p, pa_rtpoll_priority_t prio
     pollfd->fd = pa_fdsem_get(f);
     pollfd->events = POLLIN;
 
-    i->before_cb = fdsem_before;
-    i->after_cb = fdsem_after;
-    i->userdata = f;
+    pa_rtpoll_item_set_before_callback(i, fdsem_before, f);
+    pa_rtpoll_item_set_after_callback(i, fdsem_after, f);
 
     return i;
 }
@@ -530,7 +530,7 @@  pa_rtpoll_item *pa_rtpoll_item_new_fdsem(pa_rtpoll *p, pa_rtpoll_priority_t prio
 static int asyncmsgq_read_before(pa_rtpoll_item *i) {
     pa_assert(i);
 
-    if (pa_asyncmsgq_read_before_poll(i->userdata) < 0)
+    if (pa_asyncmsgq_read_before_poll(i->work_userdata) < 0)
         return 1; /* 1 means immediate restart of the loop */
 
     return 0;
@@ -540,7 +540,7 @@  static void asyncmsgq_read_after(pa_rtpoll_item *i) {
     pa_assert(i);
 
     pa_assert((i->pollfd[0].revents & ~POLLIN) == 0);
-    pa_asyncmsgq_read_after_poll(i->userdata);
+    pa_asyncmsgq_read_after_poll(i->work_userdata);
 }
 
 static int asyncmsgq_read_work(pa_rtpoll_item *i) {
@@ -552,11 +552,11 @@  static int asyncmsgq_read_work(pa_rtpoll_item *i) {
 
     pa_assert(i);
 
-    if (pa_asyncmsgq_get(i->userdata, &object, &code, &data, &offset, &chunk, 0) == 0) {
+    if (pa_asyncmsgq_get(i->work_userdata, &object, &code, &data, &offset, &chunk, 0) == 0) {
         int ret;
 
         if (!object && code == PA_MESSAGE_SHUTDOWN) {
-            pa_asyncmsgq_done(i->userdata, 0);
+            pa_asyncmsgq_done(i->work_userdata, 0);
             /* Requests the loop to exit. Will cause the next iteration of
              * pa_rtpoll_run() to return 0 */
             i->rtpoll->quit = true;
@@ -564,7 +564,7 @@  static int asyncmsgq_read_work(pa_rtpoll_item *i) {
         }
 
         ret = pa_asyncmsgq_dispatch(object, code, data, offset, &chunk);
-        pa_asyncmsgq_done(i->userdata, ret);
+        pa_asyncmsgq_done(i->work_userdata, ret);
         return 1;
     }
 
@@ -584,10 +584,9 @@  pa_rtpoll_item *pa_rtpoll_item_new_asyncmsgq_read(pa_rtpoll *p, pa_rtpoll_priori
     pollfd->fd = pa_asyncmsgq_read_fd(q);
     pollfd->events = POLLIN;
 
-    i->before_cb = asyncmsgq_read_before;
-    i->after_cb = asyncmsgq_read_after;
-    i->work_cb = asyncmsgq_read_work;
-    i->userdata = q;
+    pa_rtpoll_item_set_before_callback(i, asyncmsgq_read_before, q);
+    pa_rtpoll_item_set_after_callback(i, asyncmsgq_read_after, q);
+    pa_rtpoll_item_set_work_callback(i, asyncmsgq_read_work, q);
 
     return i;
 }
@@ -595,7 +594,7 @@  pa_rtpoll_item *pa_rtpoll_item_new_asyncmsgq_read(pa_rtpoll *p, pa_rtpoll_priori
 static int asyncmsgq_write_before(pa_rtpoll_item *i) {
     pa_assert(i);
 
-    pa_asyncmsgq_write_before_poll(i->userdata);
+    pa_asyncmsgq_write_before_poll(i->before_userdata);
     return 0;
 }
 
@@ -603,7 +602,7 @@  static void asyncmsgq_write_after(pa_rtpoll_item *i) {
     pa_assert(i);
 
     pa_assert((i->pollfd[0].revents & ~POLLIN) == 0);
-    pa_asyncmsgq_write_after_poll(i->userdata);
+    pa_asyncmsgq_write_after_poll(i->after_userdata);
 }
 
 pa_rtpoll_item *pa_rtpoll_item_new_asyncmsgq_write(pa_rtpoll *p, pa_rtpoll_priority_t prio, pa_asyncmsgq *q) {
@@ -619,10 +618,8 @@  pa_rtpoll_item *pa_rtpoll_item_new_asyncmsgq_write(pa_rtpoll *p, pa_rtpoll_prior
     pollfd->fd = pa_asyncmsgq_write_fd(q);
     pollfd->events = POLLIN;
 
-    i->before_cb = asyncmsgq_write_before;
-    i->after_cb = asyncmsgq_write_after;
-    i->work_cb = NULL;
-    i->userdata = q;
+    pa_rtpoll_item_set_before_callback(i, asyncmsgq_write_before, q);
+    pa_rtpoll_item_set_after_callback(i, asyncmsgq_write_after, q);
 
     return i;
 }
diff --git a/src/pulsecore/rtpoll.h b/src/pulsecore/rtpoll.h
index 8f0715a..4200503 100644
--- a/src/pulsecore/rtpoll.h
+++ b/src/pulsecore/rtpoll.h
@@ -80,18 +80,17 @@  struct pollfd *pa_rtpoll_item_get_pollfd(pa_rtpoll_item *i, unsigned *n_fds);
 /* Set the callback that shall be called when there's time to do some work: If the
  * callback returns a value > 0, the poll is skipped and the next
  * iteration of the loop will start immediately. */
-void pa_rtpoll_item_set_work_callback(pa_rtpoll_item *i, int (*work_cb)(pa_rtpoll_item *i));
+void pa_rtpoll_item_set_work_callback(pa_rtpoll_item *i, int (*work_cb)(pa_rtpoll_item *i), void *userdata);
 
 /* Set the callback that shall be called immediately before entering
  * the sleeping poll: If the callback returns a value > 0, the poll is
  * skipped and the next iteration of the loop will start immediately. */
-void pa_rtpoll_item_set_before_callback(pa_rtpoll_item *i, int (*before_cb)(pa_rtpoll_item *i));
+void pa_rtpoll_item_set_before_callback(pa_rtpoll_item *i, int (*before_cb)(pa_rtpoll_item *i), void *userdata);
 
 /* Set the callback that shall be called immediately after having
  * entered the sleeping poll */
-void pa_rtpoll_item_set_after_callback(pa_rtpoll_item *i, void (*after_cb)(pa_rtpoll_item *i));
+void pa_rtpoll_item_set_after_callback(pa_rtpoll_item *i, void (*after_cb)(pa_rtpoll_item *i), void *userdata);
 
-void pa_rtpoll_item_set_userdata(pa_rtpoll_item *i, void *userdata);
 void* pa_rtpoll_item_get_userdata(pa_rtpoll_item *i);
 
 pa_rtpoll_item *pa_rtpoll_item_new_fdsem(pa_rtpoll *p, pa_rtpoll_priority_t prio, pa_fdsem *s);
diff --git a/src/tests/rtpoll-test.c b/src/tests/rtpoll-test.c
index 7db7177..5ab5543 100644
--- a/src/tests/rtpoll-test.c
+++ b/src/tests/rtpoll-test.c
@@ -48,15 +48,15 @@  START_TEST (rtpoll_test) {
     p = pa_rtpoll_new();
 
     i = pa_rtpoll_item_new(p, PA_RTPOLL_EARLY, 1);
-    pa_rtpoll_item_set_before_callback(i, before);
-    pa_rtpoll_item_set_after_callback(i, after);
+    pa_rtpoll_item_set_before_callback(i, before, NULL);
+    pa_rtpoll_item_set_after_callback(i, after, NULL);
 
     pollfd = pa_rtpoll_item_get_pollfd(i, NULL);
     pollfd->fd = 0;
     pollfd->events = POLLIN;
 
     w = pa_rtpoll_item_new(p, PA_RTPOLL_NORMAL, 0);
-    pa_rtpoll_item_set_before_callback(w, worker);
+    pa_rtpoll_item_set_before_callback(w, worker, NULL);
 
     pa_rtpoll_set_timer_relative(p, 10000000); /* 10 s */
 
@@ -65,8 +65,8 @@  START_TEST (rtpoll_test) {
     pa_rtpoll_item_free(i);
 
     i = pa_rtpoll_item_new(p, PA_RTPOLL_EARLY, 1);
-    pa_rtpoll_item_set_before_callback(i, before);
-    pa_rtpoll_item_set_after_callback(i, after);
+    pa_rtpoll_item_set_before_callback(i, before, NULL);
+    pa_rtpoll_item_set_after_callback(i, after, NULL);
 
     pollfd = pa_rtpoll_item_get_pollfd(i, NULL);
     pollfd->fd = 0;

Comments

On Mon, 2016-02-29 at 15:46 +0530, arun@accosted.net wrote:
>  void* pa_rtpoll_item_get_userdata(pa_rtpoll_item *i) {
>      pa_assert(i);
>  
> -    return i->userdata;
> +    return i->work_userdata;
>  }

I think it would make sense to rename the function to
pa_rtpoll_item_get_work_userdata().

>  static int asyncmsgq_read_before(pa_rtpoll_item *i) {
>      pa_assert(i);
>  
> -    if (pa_asyncmsgq_read_before_poll(i->userdata) < 0)
> +    if (pa_asyncmsgq_read_before_poll(i->work_userdata) < 0)

In the asyncmsgq read before and after callbacks you use work_userdata,
but in the write before and after callbacks you use
before/after_userdata. All of these userdatas are the same, so it
doesn't affect the behaviour, but it would be good to be consistent.