[PATCH v9] pipe-source: [DRAFT] implement autosuspend behavior

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 22.02.2018 17:15, Raman Shyshniou wrote:
> Currently the pipe-source will remain running even if no
> writer is connected and therefore no data is produced.
> This patch prevets this by auto-suspending source
> when all writers are disconnected.
> ---
>   src/modules/module-pipe-source.c | 190 ++++++++++++++++++++++++++++++++-------
>   1 file changed, 156 insertions(+), 34 deletions(-)
>
> diff --git a/src/modules/module-pipe-source.c b/src/modules/module-pipe-source.c
> index f8284c1..6b13e28 100644
> --- a/src/modules/module-pipe-source.c
> +++ b/src/modules/module-pipe-source.c
> @@ -33,6 +33,7 @@
>   #include <sys/filio.h>
>   #endif
>   
> +#include <pulse/rtclock.h>
>   #include <pulse/xmalloc.h>
>   
>   #include <pulsecore/core-error.h>
> @@ -62,6 +63,18 @@ PA_MODULE_USAGE(
>   #define DEFAULT_FILE_NAME "/tmp/music.input"
>   #define DEFAULT_SOURCE_NAME "fifo_input"
>   
> +struct pipe_source_msg {
> +    pa_msgobject parent;
> +};
> +
> +typedef struct pipe_source_msg pipe_source_msg;
> +PA_DEFINE_PRIVATE_CLASS(pipe_source_msg, pa_msgobject);
> +
> +enum {
> +    PIPE_SOURCE_SUSPEND,
> +    PIPE_SOURCE_RESUME
> +};
> +
>   struct userdata {
>       pa_core *core;
>       pa_module *module;
> @@ -71,7 +84,15 @@ struct userdata {
>       pa_thread_mq thread_mq;
>       pa_rtpoll *rtpoll;
>   
> +    pipe_source_msg *msg;
> +    pa_usec_t timestamp;
> +    size_t pipe_size;
> +
> +    bool writer_connected;
> +    bool suspended_by_user;
> +
>       char *filename;
> +    int corkfd;
>       int fd;
>   
>       pa_memchunk memchunk;
> @@ -90,6 +111,37 @@ static const char* const valid_modargs[] = {
>       NULL
>   };
>   
> +/* Called from main context */
> +static int pipe_source_process_msg(
> +        pa_msgobject *o,
> +        int code,
> +        void *data,
> +        int64_t offset,
> +        pa_memchunk *chunk) {
> +
> +    struct userdata *u = (struct userdata *) data;
> +
> +    pa_assert(u);
> +
> +    switch (code) {
> +        case PIPE_SOURCE_SUSPEND:
> +            pa_log_debug("Suspending source %s because no writers left", u->source->name);
> +            pa_source_suspend(u->source, true, PA_SUSPEND_UNAVAILABLE);
> +            break;
> +
> +        case PIPE_SOURCE_RESUME:
> +            pa_log_debug("Resuming source %s", u->source->name);
> +            pa_source_suspend(u->source, false, PA_SUSPEND_UNAVAILABLE);
> +            break;
> +
> +        default:
> +            pa_assert_not_reached();
> +    }
> +
> +    return 0;
> +}
> +
> +/* Called from thread context */
>   static int source_process_msg(
>           pa_msgobject *o,
>           int code,
> @@ -101,17 +153,28 @@ static int source_process_msg(
>   
>       switch (code) {
>   
> -        case PA_SOURCE_MESSAGE_GET_LATENCY: {
> -            size_t n = 0;
> +        case PA_SOURCE_MESSAGE_SET_STATE: {
> +            pa_source_state_t new_state = ((pa_source_message_set_state *) data)->state;
> +            pa_suspend_cause_t suspend_cause = ((pa_source_message_set_state *) data)->suspend_cause;
>   
> -#ifdef FIONREAD
> -            int l;
> +            if (u->source->thread_info.state == PA_SOURCE_SUSPENDED || u->source->thread_info.state == PA_SOURCE_INIT)
> +                if (PA_SOURCE_IS_OPENED(new_state)) {
> +                    u->timestamp = pa_rtclock_now();
> +                    u->suspended_by_user = false;
> +                }
>   
> -            if (ioctl(u->fd, FIONREAD, &l) >= 0 && l > 0)
> -                n = (size_t) l;
> -#endif
> +            if (new_state == PA_SOURCE_SUSPENDED && suspend_cause & ~PA_SUSPEND_UNAVAILABLE)
> +                u->suspended_by_user = true;
> +
> +            break;
> +        }
> +
> +        case PA_SOURCE_MESSAGE_GET_LATENCY: {
> +            pa_usec_t now;
> +
> +            now = pa_rtclock_now();
> +            *((int64_t*) data) = (int64_t)now - (int64_t)u->timestamp;
>   
> -            *((int64_t*) data) = pa_bytes_to_usec(n, &u->source->sample_spec);
>               return 0;
>           }
>       }
> @@ -121,6 +184,7 @@ static int source_process_msg(
>   
>   static void thread_func(void *userdata) {
>       struct userdata *u = userdata;
> +    struct pollfd *pollfd;
>       int read_type = 0;
>   
>       pa_assert(u);
> @@ -129,30 +193,30 @@ static void thread_func(void *userdata) {
>   
>       pa_thread_mq_install(&u->thread_mq);
>   
> +    u->memchunk.memblock = pa_memblock_new(u->core->mempool, u->pipe_size);
> +
> +    u->timestamp = pa_rtclock_now();
> +
> +    /* Close our writer here to suspend source if no writers left */
> +    pa_assert_se(pa_close(u->corkfd) == 0);
> +    u->corkfd = -1;
> +
> +    pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL);
> +
>       for (;;) {
>           int ret;
> -        struct pollfd *pollfd;
> -
> -        pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL);
>   
> -        /* Try to read some data and pass it on to the source driver */
> -        if (u->source->thread_info.state == PA_SOURCE_RUNNING && pollfd->revents) {
> +        /* Try to read some data if there are any events */
> +        if (pollfd->revents) {
>               ssize_t l;
>               void *p;
>   
> -            if (!u->memchunk.memblock) {
> -                u->memchunk.memblock = pa_memblock_new(u->core->mempool, pa_pipe_buf(u->fd));
> -                u->memchunk.index = u->memchunk.length = 0;
> -            }
> -
> -            pa_assert(pa_memblock_get_length(u->memchunk.memblock) > u->memchunk.index);
> +            pa_assert(u->memchunk.length == 0);
>   
>               p = pa_memblock_acquire(u->memchunk.memblock);
> -            l = pa_read(u->fd, (uint8_t*) p + u->memchunk.index, pa_memblock_get_length(u->memchunk.memblock) - u->memchunk.index, &read_type);
> +            l = pa_read(u->fd, p, pa_memblock_get_length(u->memchunk.memblock), &read_type);
>               pa_memblock_release(u->memchunk.memblock);
>   
> -            pa_assert(l != 0); /* EOF cannot happen, since we opened the fifo for both reading and writing */
> -
>               if (l < 0) {
>   
>                   if (errno == EINTR)
> @@ -162,23 +226,52 @@ static void thread_func(void *userdata) {
>                       goto fail;
>                   }
>   
> +            } else if (l == 0) {
> +
> +                if (u->writer_connected) {
> +                    pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(u->msg), PIPE_SOURCE_SUSPEND, u, 0, NULL, NULL);
> +
> +                    if ((u->corkfd = pa_open_cloexec(u->filename, O_WRONLY, 0)) < 0) {
> +                        pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno));
> +                        goto fail;
> +                    }
> +
> +                    u->writer_connected = false;
> +                }
> +
>               } else {
>   
>                   u->memchunk.length = (size_t) l;
> -                pa_source_post(u->source, &u->memchunk);
> -                u->memchunk.index += (size_t) l;
> +                u->timestamp = pa_rtclock_now();
>   
> -                if (u->memchunk.index >= pa_memblock_get_length(u->memchunk.memblock)) {
> -                    pa_memblock_unref(u->memchunk.memblock);
> -                    pa_memchunk_reset(&u->memchunk);
> +                if (!u->writer_connected) {
> +                    pa_asyncmsgq_post(pa_thread_mq_get()->outq, PA_MSGOBJECT(u->msg), PIPE_SOURCE_RESUME, u, 0, NULL, NULL);
> +                    u->writer_connected = true;
>                   }
>   
> -                pollfd->revents = 0;
>               }
>           }
>   
> -        /* Hmm, nothing to do. Let's sleep */
> -        pollfd->events = (short) (u->source->thread_info.state == PA_SOURCE_RUNNING ? POLLIN : 0);
> +        /* Post data to source, discard data or wait for state transition to be complete */
> +        if (PA_SOURCE_IS_OPENED(u->source->thread_info.state)) {
> +
> +            if (u->writer_connected && u->corkfd >= 0) {
> +                pa_assert_se(pa_close(u->corkfd) == 0);
> +                u->corkfd = -1;
> +            }
> +
> +            if (u->memchunk.length > 0) {
> +                pa_source_post(u->source, &u->memchunk);
> +                pa_memblock_unref(u->memchunk.memblock);
> +                u->memchunk.memblock = pa_memblock_new(u->core->mempool, u->pipe_size);
> +                u->memchunk.length = 0;
> +            }
> +
> +        } else if (u->suspended_by_user)

You have to open the corkfd here as well to avoid POLLHUP spam if the writer
disconnects during user suspend.

> +            u->memchunk.length = 0;
> +
> +
> +        pollfd->events = u->memchunk.length ? 0 : POLLIN;

How do you wake up from autosuspend? When the writer disconnects,
memchunk.length will be 0 and so you do no longer listen for POLLIN.
I guess you need to know the current suspend cause here:
user suspended -> events = 0
(only auto suspended + no writer connected) or running -> events = POLLIN
only auto suspended + writer connected (=wake up transition) -> events = 0

>   
>           if ((ret = pa_rtpoll_run(u->rtpoll)) < 0)
>               goto fail;
> @@ -188,8 +281,13 @@ static void thread_func(void *userdata) {
>   
>           pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL);
>   
> -        if (pollfd->revents & ~POLLIN) {
> -            pa_log("FIFO shutdown.");
> +        if (pollfd->revents & ~(POLLIN|POLLHUP)) {
> +            pa_log("FD error: %s%s%s%s",
> +                   pollfd->revents & POLLOUT ? "POLLOUT " : "",
> +                   pollfd->revents & POLLERR ? "POLLERR " : "",
> +                   pollfd->revents & POLLPRI ? "POLLPRI " : "",
> +                   pollfd->revents & POLLNVAL ? "POLLNVAL " : "");
> +
>               goto fail;
>           }
>       }
> @@ -238,13 +336,26 @@ int pa__init(pa_module *m) {
>           goto fail;
>       }
>   
> +    if (!(u->msg = pa_msgobject_new(pipe_source_msg)))
> +        goto fail;
> +
> +    u->msg->parent.process_msg = pipe_source_process_msg;
> +
>       u->filename = pa_runtime_path(pa_modargs_get_value(ma, "file", DEFAULT_FILE_NAME));
>   
>       if (mkfifo(u->filename, 0666) < 0) {
>           pa_log("mkfifo('%s'): %s", u->filename, pa_cstrerror(errno));
>           goto fail;
>       }
> -    if ((u->fd = pa_open_cloexec(u->filename, O_RDWR, 0)) < 0) {
> +
> +    if ((u->corkfd = pa_open_cloexec(u->filename, O_RDWR, 0)) < 0) {
> +        pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno));
> +        goto fail;
> +    }
> +
> +    u->writer_connected = true;
> +
> +    if ((u->fd = pa_open_cloexec(u->filename, O_RDONLY, 0)) < 0) {
>           pa_log("open('%s'): %s", u->filename, pa_cstrerror(errno));
>           goto fail;
>       }
> @@ -289,13 +400,18 @@ int pa__init(pa_module *m) {
>   
>       pa_source_set_asyncmsgq(u->source, u->thread_mq.inq);
>       pa_source_set_rtpoll(u->source, u->rtpoll);
> -    pa_source_set_fixed_latency(u->source, pa_bytes_to_usec(pa_pipe_buf(u->fd), &u->source->sample_spec));
> +
> +    u->pipe_size = pa_pipe_buf(u->fd);
> +    u->pipe_size = PA_MIN(pa_mempool_block_size_max(m->core->mempool), u->pipe_size);
> +    pa_source_set_fixed_latency(u->source, pa_bytes_to_usec(u->pipe_size, &u->source->sample_spec));
>   
>       u->rtpoll_item = pa_rtpoll_item_new(u->rtpoll, PA_RTPOLL_NEVER, 1);
>       pollfd = pa_rtpoll_item_get_pollfd(u->rtpoll_item, NULL);
>       pollfd->fd = u->fd;
>       pollfd->events = pollfd->revents = 0;
>   
> +    u->suspended_by_user = false;
> +
>       if (!(u->thread = pa_thread_new("pipe-source", thread_func, u))) {
>           pa_log("Failed to create thread.");
>           goto fail;
> @@ -360,6 +476,12 @@ void pa__done(pa_module *m) {
>           pa_xfree(u->filename);
>       }
>   
> +    if (u->msg)
> +        pa_xfree(u->msg);
> +
> +    if (u->corkfd >= 0)
> +        pa_assert_se(pa_close(u->corkfd) == 0);
> +
>       if (u->fd >= 0)
>           pa_assert_se(pa_close(u->fd) == 0);
>   

I believe you need some user_unsuspended and/or auto_unsuspended
flag that is set in the SET_STATE handler to handle the first data read
after suspend correctly.

When you unsuspend from auto suspend, the first chunk you already read
from the pipe should be posted - as far as I can see this happens.

However, when you unsuspend from user suspend, the data in the pipe may
be old - you did not read anything during suspend. Therefore the first chunk
you read should be discarded. That does not happen as far as I can tell. Or
am I overlooking something (again)?

The general approach of the patch looks OK now.



[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux