[pulseaudio-discuss] [PATCH] tunnel-new: cork the stream when sink/source gets suspended and uncork it when resumed

[Tanu Kaskinen]

On Mon, 2013-09-16 at 14:10 +0200, Alexander Couzens wrote:
> ---
>  src/modules/module-tunnel-sink-new.c   | 33 ++++++++++++++++++++++++++++++
>  src/modules/module-tunnel-source-new.c | 37 ++++++++++++++++++++++++++++++++--
>  2 files changed, 68 insertions(+), 2 deletions(-)

Thanks for the patch! There are some issues pointed out below.

> diff --git a/src/modules/module-tunnel-sink-new.c b/src/modules/module-tunnel-sink-new.c
> index d9fe0e6..2d732bf 100644
> --- a/src/modules/module-tunnel-sink-new.c
> +++ b/src/modules/module-tunnel-sink-new.c
> @@ -102,6 +102,17 @@ static const char* const valid_modargs[] = {
>      NULL,
>  };
>  
> +static void cork_stream(struct userdata *u, int cork) {

bool would be a better type for the cork argument.

> +    pa_operation *operation;
> +
> +    pa_assert(u);
> +    pa_assert(u->stream);
> +
> +    if ((operation = pa_stream_cork(u->stream, cork, NULL, NULL))) {
> +        pa_operation_unref(operation);
> +    }

Unnecessary braces.

> +}
> +
>  static void reset_bufferattr(pa_buffer_attr *bufferattr) {
>      pa_assert(bufferattr);
>      bufferattr->fragsize = (uint32_t) -1;
> @@ -244,6 +255,9 @@ static void stream_state_cb(pa_stream *stream, void *userdata) {
>              pa_log_debug("Stream terminated.");
>              break;
>          case PA_STREAM_READY:
> +            if (PA_SINK_IS_OPENED(u->sink->thread_info.state))
> +                cork_stream(u, 0);
> +
>              /* Only call our requested_latency_cb when requested_latency
>               * changed between PA_STREAM_CREATING -> PA_STREAM_READY, because
>               * we don't want to override the initial tlength set by the server
> @@ -413,6 +427,25 @@ static int sink_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t of
>              *((pa_usec_t*) data) = remote_latency;
>              return 0;
>          }
> +        case PA_SINK_MESSAGE_SET_STATE:
> +            if (!u->stream || !PA_STREAM_IS_GOOD(pa_stream_get_state(u->stream)))

I think the condition here should be state != PA_STREAM_READY instead
of !PA_STREAM_IS_GOOD(state). You don't want to mess with the cork state
while the state is PA_STREAM_CREATING.

> +                break;
> +
> +            switch ((pa_sink_state_t) PA_PTR_TO_UINT(data)) {
> +                case PA_SINK_SUSPENDED: {
> +                    cork_stream(u, 1);
> +                    break;
> +                }
> +                case PA_SINK_IDLE:
> +                case PA_SINK_RUNNING: {
> +                    cork_stream(u, 0);
> +                    break;
> +                }
> +                case PA_SINK_INVALID_STATE:
> +                case PA_SINK_INIT:
> +                case PA_SINK_UNLINKED:
> +                    break;
> +            }
>      }
>      return pa_sink_process_msg(o, code, data, offset, chunk);
>  }
> diff --git a/src/modules/module-tunnel-source-new.c b/src/modules/module-tunnel-source-new.c
> index d2e2d6c..317007f 100644
> --- a/src/modules/module-tunnel-source-new.c
> +++ b/src/modules/module-tunnel-source-new.c
> @@ -101,6 +101,17 @@ static const char* const valid_modargs[] = {
>      NULL,
>  };
>  
> +static void cork_stream(struct userdata *u, int cork) {
> +    pa_operation *operation;
> +
> +    pa_assert(u);
> +    pa_assert(u->stream);
> +
> +    if ((operation = pa_stream_cork(u->stream, cork, NULL, NULL))) {
> +        pa_operation_unref(operation);
> +    }
> +}
> +
>  static void reset_bufferattr(pa_buffer_attr *bufferattr) {
>      pa_assert(bufferattr);
>      bufferattr->fragsize = (uint32_t) -1;
> @@ -262,6 +273,9 @@ static void stream_state_cb(pa_stream *stream, void *userdata) {
>              pa_log_debug("Stream terminated.");
>              break;
>          case PA_STREAM_READY:
> +            if (PA_SOURCE_IS_OPENED(u->source->thread_info.state))
> +                cork_stream(u, 0);
> +
>              /* Only call our requested_latency_cb when requested_latency
>               * changed between PA_STREAM_CREATING -> PA_STREAM_READY, because
>               * we don't want to override the initial fragsize set by the server
> @@ -325,7 +339,7 @@ static void context_state_cb(pa_context *c, void *userdata) {
>              if (pa_stream_connect_record(u->stream,
>                                           u->remote_source_name,
>                                           &bufferattr,
> -                                         PA_STREAM_INTERPOLATE_TIMING|PA_STREAM_DONT_MOVE|PA_STREAM_AUTO_TIMING_UPDATE) < 0) {
> +                                         PA_STREAM_INTERPOLATE_TIMING|PA_STREAM_DONT_MOVE|PA_STREAM_AUTO_TIMING_UPDATE|PA_STREAM_START_CORKED) < 0) {
>                  pa_log_debug("Could not create stream: %s", pa_strerror(pa_context_errno(u->context)));
>                  u->thread_mainloop_api->quit(u->thread_mainloop_api, TUNNEL_THREAD_FAILED_MAINLOOP);
>              }
> @@ -417,7 +431,26 @@ static int source_process_msg_cb(pa_msgobject *o, int code, void *data, int64_t
>  
>              return 0;
>          }
> -    }
> +        case PA_SOURCE_MESSAGE_SET_STATE:
> +            if (!u->stream || !PA_STREAM_IS_GOOD(pa_stream_get_state(u->stream)))
> +                break;
> +
> +            switch ((pa_source_state_t) PA_PTR_TO_UINT(data)) {
> +                case PA_SOURCE_SUSPENDED: {
> +                    cork_stream(u, 1);
> +                    break;
> +                }
> +                case PA_SOURCE_IDLE:
> +                case PA_SOURCE_RUNNING: {
> +                    cork_stream(u, 0);
> +                    break;
> +                }
> +                case PA_SOURCE_INVALID_STATE:
> +                case PA_SOURCE_INIT:
> +                case PA_SOURCE_UNLINKED:
> +                    break;
> +            }
> +        }

Misaligned closing brace.

In module-tunnel-sink-new, there is this stuff in thread_func() that I
believe should be deleted:

            /* TODO: Cork the stream when the sink is suspended. */

            if (pa_stream_is_corked(u->stream)) {
                pa_operation *operation;
                if ((operation = pa_stream_cork(u->stream, 0, NULL, NULL)))
                    pa_operation_unref(operation);
            }

-- 
Tanu