[pulseaudio-discuss] [PATCH] module-tunnel-sink-new: add a rewrite of module-tunnel using libpulse

[Tanu Kaskinen]

On Wed, 2013-08-21 at 13:58 +0200, Alexander Couzens wrote:
> Old module-tunnel shares duplicated functionality with libpulse because
> it is implementing pulse protocol again. It is also not
> as much tested as libpulse it.
> 
> Signed-off-by: Alexander Couzens <lynxis at fe80.eu>
> ---
>  src/Makefile.am                      |   6 +
>  src/modules/module-tunnel-sink-new.c | 546 +++++++++++++++++++++++++++++++++++
>  2 files changed, 552 insertions(+)
>  create mode 100644 src/modules/module-tunnel-sink-new.c

Still some small issues, but they are trivial enough for me to fix them
myself. Patch applied, thank you!

> +static void thread_func(void *userdata) {
> +    struct userdata *u = userdata;
> +    pa_proplist *proplist;
> +    pa_assert(u);
> +
> +    pa_log_debug("Thread starting up");
> +    pa_thread_mq_install(&u->thread_mq);
> +
> +    proplist = tunnel_new_proplist(u);
> +    u->context = pa_context_new_with_proplist(u->thread_mainloop_api,
> +                                              "PulseAudio",
> +                                              proplist);
> +    pa_proplist_free(proplist);
> +
> +    if (!u->context) {
> +        pa_log("Failed to create libpulse context");
> +        goto fail;
> +    }
> +
> +    pa_context_set_state_callback(u->context, context_state_cb, u);
> +    if (pa_context_connect(u->context,
> +                           u->remote_server,
> +                           PA_CONTEXT_NOAUTOSPAWN,
> +                           NULL) < 0) {
> +        pa_log("Failed to connect libpulse context");
> +        goto fail;
> +    }
> +
> +    for (;;) {
> +        int ret;
> +        const void *p;
> +        pa_memchunk memchunk;
> +
> +        size_t writable = 0;
> +
> +        if (pa_mainloop_iterate(u->thread_mainloop, 1, &ret) < 0) {
> +            if (ret == 0)
> +                goto finish;
> +            else
> +                goto fail;
> +        }
> +
> +        if (PA_UNLIKELY(u->sink->thread_info.rewind_requested))
> +            pa_sink_process_rewind(u->sink, 0);
> +
> +        if (u->connected &&
> +                pa_stream_get_state(u->stream) == PA_STREAM_READY &&
> +                PA_SINK_IS_LINKED(u->sink->thread_info.state)) {
> +            /* TODO: use IS_RUNNING and cork the stream when sink is not running */

The stream should be corked only if the sink is suspended. The sink can
be idle or running, and in both cases the stream should stay uncorked.

> +static void stream_state_cb(pa_stream *stream, void *userdata) {
> +    struct userdata *u = userdata;
> +
> +    pa_assert(u);
> +
> +    switch (pa_stream_get_state(stream)) {
> +        case PA_STREAM_FAILED:
> +            pa_log_error("Stream failed.");
> +            u->connected = false;
> +            u->thread_mainloop_api->quit(u->thread_mainloop_api, TUNNEL_THREAD_FAILED_MAINLOOP);
> +            break;
> +        case PA_STREAM_TERMINATED:
> +            pa_log_debug("Stream terminated.");
> +            break;
> +        case PA_STREAM_READY:
> +            /* only call our requested_latency_cb when request_latency changed between PA_CONNECTING -> PA_STREAM_READY
> +             * otherwhise we would deny servers response to bufferattr (which defines latency) */

Comments should be wrapped at 80 characters.

> +            if (u->update_stream_bufferattr_after_connect)
> +                sink_update_requested_latency_cb(u->sink);

There should be an else section that updates the sink max_request to
match the tlength that the server assigned.

> +static void sink_update_requested_latency_cb(pa_sink *s) {
> +    struct userdata *u;
> +    pa_operation *operation;
> +    size_t nbytes;
> +    pa_usec_t block_usec;
> +    pa_buffer_attr bufferattr;
> +
> +    pa_sink_assert_ref(s);
> +    pa_assert_se(u = s->userdata);
> +
> +    block_usec = pa_sink_get_requested_latency_within_thread(s);
> +    if (block_usec == (pa_usec_t) -1)
> +        block_usec = s->thread_info.max_latency;
> +
> +    nbytes = pa_usec_to_bytes(block_usec, &s->sample_spec);
> +    pa_sink_set_max_request_within_thread(s, nbytes);
> +
> +    if (u->stream) {
> +        switch (pa_stream_get_state(u->stream)) {
> +        case PA_STREAM_READY:

One level of indentation is missing.

-- 
Tanu