On Mon, 2018-07-16 at 20:25 -0400, Yclept Nemo wrote:
> Call 'pa_sink_put' or 'pa_source_put' after the connection is
> authorized. For the new tunnel modules this involves sending a
> module-specific message from the IO thread to the main thread. On
> receipt the sink/source message handler calls 'pa_*_put' if connected
> (this doesn't need to by guarded by a conditional; the message should
> only be sent once). For the old tunnel module this involves moving
> 'pa_*_put' down the call chain of the main thread: 'pa__init' ->
> 'on_connection' -> 'setup_complete_callback'.
> ---
> src/modules/module-tunnel-sink-new.c | 14 +++++++++++++-
> src/modules/module-tunnel-source-new.c | 14 +++++++++++++-
> src/modules/module-tunnel.c | 12 ++++++------
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/src/modules/module-tunnel-sink-new.c b/src/modules/module-tunnel-sink-new.c
> index 802e6a59..b301f999 100644
> --- a/src/modules/module-tunnel-sink-new.c
> +++ b/src/modules/module-tunnel-sink-new.c
> @@ -60,6 +60,10 @@ PA_MODULE_USAGE(
> #define MAX_LATENCY_USEC (200 * PA_USEC_PER_MSEC)
> #define TUNNEL_THREAD_FAILED_MAINLOOP 1
>
> +enum {
> + SINK_MESSAGE_PUT = PA_SINK_MESSAGE_MAX,
> +};
> +
> static void stream_state_cb(pa_stream *stream, void *userdata);
> static void stream_changed_buffer_attr_cb(pa_stream *stream, void *userdata);
> static void stream_set_buffer_attr_cb(pa_stream *stream, int success, void *userdata);
> @@ -345,6 +349,7 @@ static void context_state_cb(pa_context *c, void *userdata) {
> u->thread_mainloop_api->quit(u->thread_mainloop_api, TUNNEL_THREAD_FAILED_MAINLOOP);
> }
> u->connected = true;
> + pa_asyncmsgq_send(u->thread_mq->outq, PA_MSGOBJECT(u->sink), SINK_MESSAGE_PUT, NULL, 0, NULL);
pa_asyncmsgq_send() is synchronous, and it shouldn't be used from the
IO thread, because that can cause deadlocks. I think an asynchronous
message would work fine in this case (i.e. pa_asyncmsgq_post()).
> break;
> }
> case PA_CONTEXT_FAILED:
> @@ -402,6 +407,7 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
> struct userdata *u = PA_SINK(o)->userdata;
>
> switch (code) {
> + /* Delivered from the main thread, handled in the IO thread. */
> case PA_SINK_MESSAGE_GET_LATENCY: {
> int negative;
> pa_usec_t remote_latency;
> @@ -429,6 +435,13 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
> *((int64_t*) data) = remote_latency;
> return 0;
> }
> +
> + /* Delivered from the IO thread, handled in the main thread. */
> + case SINK_MESSAGE_PUT: {
> + if (u->connected)
> + pa_sink_put(u->sink);
u->connected is supposed to be used only in the IO thread. In what
situation would you want to not put the sink? I suppose at least during
module unloading it could happen that the SINK_MESSAGE_PUT message is
processed, but we don't want to call pa_sink_put().
A separate flag could be set in the beginning of pa__done() to indicate
that the module is being unloaded, but I think it would make sense to
have that flag in the core pa_module struct instead, and set it in
pa_module_free(). There's already pa_module.unload_requested, which is
kind of what we want, but it's currently only set if
pa_module_unload_request() is called. I think
pa_module.unload_requested could be set also from pa_module_free(), and
then it would be a reliable flag for checking whether the module is
being unloaded.
--
Tanu
https://www.patreon.com/tanuk
https://liberapay.com/tanuk
