[pulseaudio-discuss] [PATCH 1/2] thread_mq: supports a mainloop as thread/consumer

[Tanu Kaskinen]

On Tue, 2013-08-06 at 05:29 +0200, Alexander Couzens wrote:
> Signed-off-by: Alexander Couzens <lynxis at fe80.eu>
> ---
>  src/pulsecore/thread-mq.c | 74 ++++++++++++++++++++++++++++++++++++++++-------
>  src/pulsecore/thread-mq.h |  7 +++--
>  2 files changed, 68 insertions(+), 13 deletions(-)

There were still some issues (pointed out below), but since they were
trivial to fix, I fixed them. Thanks for the patch, it's now applied.

> diff --git a/src/pulsecore/thread-mq.c b/src/pulsecore/thread-mq.c
> index dd84c9a..12e6d4e 100644
> --- a/src/pulsecore/thread-mq.c
> +++ b/src/pulsecore/thread-mq.c
> @@ -30,18 +30,27 @@
>  #include <pulsecore/semaphore.h>
>  #include <pulsecore/macro.h>
>  
> +#include <pulse/mainloop-api.h>
> +
>  #include "thread-mq.h"
>  
>  PA_STATIC_TLS_DECLARE_NO_FREE(thread_mq);
>  
> -static void asyncmsgq_read_cb(pa_mainloop_api*api, pa_io_event* e, int fd, pa_io_event_flags_t events, void *userdata) {
> +static void asyncmsgq_read_cb(pa_mainloop_api *api, pa_io_event *e, int fd, pa_io_event_flags_t events, void *userdata) {
>      pa_thread_mq *q = userdata;
>      pa_asyncmsgq *aq;
>  
> -    pa_assert(pa_asyncmsgq_read_fd(q->outq) == fd);
>      pa_assert(events == PA_IO_EVENT_INPUT);
>  
> -    pa_asyncmsgq_ref(aq = q->outq);
> +    if (pa_asyncmsgq_read_fd(q->outq) == fd)
> +        pa_asyncmsgq_ref(aq = q->outq);
> +    else if (pa_asyncmsgq_read_fd(q->inq))

I believe the intention was to compare the pa_asyncmsgq_read_fd() return
value to fd.

> +        pa_asyncmsgq_ref(aq = q->inq);
> +    else {
> +        pa_assert(false);
> +        return;

pa_assert_not_reached() can be used here.

> +void pa_thread_mq_init_thread_mainloop(pa_thread_mq *q, pa_mainloop_api *main_mainloop, pa_mainloop_api *thread_mainloop) {
> +    pa_assert(q);
> +    pa_assert(main_mainloop);
> +    pa_assert(thread_mainloop);
> +
> +    pa_assert_se(q->inq = pa_asyncmsgq_new(0));
> +    pa_assert_se(q->outq = pa_asyncmsgq_new(0));
> +
> +    q->main_mainloop = main_mainloop;

q->thread_mainloop initialization is missing.

> @@ -106,15 +152,21 @@ void pa_thread_mq_done(pa_thread_mq *q) {
>      if (!pa_asyncmsgq_dispatching(q->outq))
>          pa_asyncmsgq_flush(q->outq, true);
>  
> -    q->mainloop->io_free(q->read_event);
> -    q->mainloop->io_free(q->write_event);
> -    q->read_event = q->write_event = NULL;
> +    q->main_mainloop->io_free(q->read_main_event);
> +    q->main_mainloop->io_free(q->write_main_event);
> +    q->read_main_event = q->write_main_event = NULL;
> +
> +    if (q->thread_mainloop) {
> +        q->thread_mainloop->io_free(q->read_thread_event);
> +        q->thread_mainloop->io_free(q->write_thread_event);
> +        q->read_thread_event = q->write_thread_event = NULL;
> +    }
>  
>      pa_asyncmsgq_unref(q->inq);
>      pa_asyncmsgq_unref(q->outq);
>      q->inq = q->outq = NULL;
>  
> -    q->mainloop = NULL;
> +    q->main_mainloop = NULL;

I suppose q->thread_mainloop should be set to NULL too.

-- 
Tanu