On Fri, 2015-01-02 at 15:04 +0200, Tanu Kaskinen wrote: > The new tunnel sink and source use libpulse to talk to the remote > server, and libpulse requires a pa_mainloop_api implementation for > interacting with the event loop. The tunnel sink and source have so > far been using pa_mainloop, but there are some modules that assume > that all sinks and sources use pa_rtpoll in their IO threads, and > trying to use those modules together with the pa_mainloop based > tunnel sinks and sources will result in crashes (see [1]). > > I will switch the event loop implementation in the tunnel modules > to pa_rtpoll, but that requires a pa_mainloop_api implementation in > pa_rtpoll first - that's implemented here. > > pa_rtpoll_run() is changed so that it processes all defer events > first, and then all expired time events. The rest of pa_rtpoll_run() > works as before, except the poll timeout calculation now has to also > take the time events into account. IO events use the existing > pa_rtpoll_item interface. > > The time events are handled separately from the old timer > functionality, which is somewhat ugly. It might be a good idea to > remove the old timer functionality and only use the time events. > I didn't attempt to do that at this time, because I feared that > adapting the pa_rtpoll users to the new system would be difficult. > > [1] https://bugs.freedesktop.org/show_bug.cgi?id=73429 > --- As a first comment, I'd like to ask that there be a test for this. It should be quite easy to refactor mainloop-test to run the same set of tests on different mainloop implementations. > Â src/pulsecore/rtpoll.c | 504 > ++++++++++++++++++++++++++++++++++++++++++++++++- > Â src/pulsecore/rtpoll.h |Â Â Â 4 + > Â 2 files changed, 500 insertions(+), 8 deletions(-) > > diff --git a/src/pulsecore/rtpoll.c b/src/pulsecore/rtpoll.c > index 5f3ca8b..2e1907c 100644 > --- a/src/pulsecore/rtpoll.c > +++ b/src/pulsecore/rtpoll.c > @@ -32,6 +32,7 @@ > Â #include <pulse/xmalloc.h> > Â #include <pulse/timeval.h> > Â > +#include <pulsecore/dynarray.h> > Â #include <pulsecore/poll.h> > Â #include <pulsecore/core-error.h> > Â #include <pulsecore/core-rtclock.h> > @@ -65,6 +66,18 @@ struct pa_rtpoll { > Â #endif > Â > Â Â Â Â Â PA_LLIST_HEAD(pa_rtpoll_item, items); > + > +Â Â Â Â pa_mainloop_api mainloop_api; > + > +Â Â Â Â pa_dynarray *io_events; > + > +Â Â Â Â pa_dynarray *time_events; > +Â Â Â Â pa_dynarray *enabled_time_events; > +Â Â Â Â pa_dynarray *expired_time_events; > +Â Â Â Â pa_time_event *cached_next_time_event; > + > +Â Â Â Â pa_dynarray *defer_events; > +Â Â Â Â pa_dynarray *enabled_defer_events; > Â }; > Â > Â struct pa_rtpoll_item { > @@ -84,8 +97,321 @@ struct pa_rtpoll_item { > Â Â Â Â Â PA_LLIST_FIELDS(pa_rtpoll_item); > Â }; > Â > +struct pa_io_event { > +Â Â Â Â pa_rtpoll *rtpoll; > +Â Â Â Â pa_rtpoll_item *rtpoll_item; > +Â Â Â Â pa_io_event_flags_t events; > +Â Â Â Â pa_io_event_cb_t callback; > +Â Â Â Â pa_io_event_destroy_cb_t destroy_callback; > +Â Â Â Â void *userdata; > +}; > + > +static void io_event_enable(pa_io_event *event, pa_io_event_flags_t > events); > + > +struct pa_time_event { > +Â Â Â Â pa_rtpoll *rtpoll; > +Â Â Â Â pa_usec_t time; > +Â Â Â Â bool use_rtclock; > +Â Â Â Â bool enabled; > +Â Â Â Â pa_time_event_cb_t callback; > +Â Â Â Â pa_time_event_destroy_cb_t destroy_callback; > +Â Â Â Â void *userdata; > +}; > + > +static void time_event_restart(pa_time_event *event, const struct > timeval *tv); > + > +struct pa_defer_event { > +Â Â Â Â pa_rtpoll *rtpoll; > +Â Â Â Â bool enabled; > +Â Â Â Â pa_defer_event_cb_t callback; > +Â Â Â Â pa_defer_event_destroy_cb_t destroy_callback; > +Â Â Â Â void *userdata; > +}; > + > +static void defer_event_enable(pa_defer_event *event, int enable); > + > Â PA_STATIC_FLIST_DECLARE(items, 0, pa_xfree); > Â > +static short map_flags_to_libc(pa_io_event_flags_t flags) { > +Â Â Â Â return (short) > +Â Â Â Â Â Â Â Â ((flags & PA_IO_EVENT_INPUT ? POLLIN : 0) | > +Â Â Â Â Â Â Â Â Â (flags & PA_IO_EVENT_OUTPUT ? POLLOUT : 0) | > +Â Â Â Â Â Â Â Â Â (flags & PA_IO_EVENT_ERROR ? POLLERR : 0) | > +Â Â Â Â Â Â Â Â Â (flags & PA_IO_EVENT_HANGUP ? POLLHUP : 0)); > +} > + > +static pa_io_event_flags_t map_flags_from_libc(short flags) { > +Â Â Â Â return > +Â Â Â Â Â Â Â Â (flags & POLLIN ? PA_IO_EVENT_INPUT : 0) | > +Â Â Â Â Â Â Â Â (flags & POLLOUT ? PA_IO_EVENT_OUTPUT : 0) | > +Â Â Â Â Â Â Â Â (flags & POLLERR ? PA_IO_EVENT_ERROR : 0) | > +Â Â Â Â Â Â Â Â (flags & POLLHUP ? PA_IO_EVENT_HANGUP : 0); > +} Can we dump these two in a util.c and have this reused with mainloop.c? > +static int io_event_work_cb(pa_rtpoll_item *item) { > +Â Â Â Â pa_io_event *event; > +Â Â Â Â struct pollfd *pollfd; > + > +Â Â Â Â pa_assert(item); > + > +Â Â Â Â event = pa_rtpoll_item_get_userdata(item); > +Â Â Â Â pollfd = pa_rtpoll_item_get_pollfd(item, NULL); > +Â Â Â Â event->callback(&event->rtpoll->mainloop_api, event, pollfd->fd, > map_flags_from_libc(pollfd->revents), event->userdata); > + > +Â Â Â Â return 0; > +} > + > +static pa_io_event* io_event_new(pa_mainloop_api *api, int fd, > pa_io_event_flags_t events, pa_io_event_cb_t callback, > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â void *userdata) { > +Â Â Â Â pa_rtpoll *rtpoll; > +Â Â Â Â pa_io_event *event; > +Â Â Â Â struct pollfd *pollfd; > + > +Â Â Â Â pa_assert(api); > +Â Â Â Â pa_assert(api->userdata); > +Â Â Â Â pa_assert(fd >= 0); > +Â Â Â Â pa_assert(callback); > + > +Â Â Â Â rtpoll = api->userdata; > +Â Â Â Â pa_assert(api == &rtpoll->mainloop_api); > + > +Â Â Â Â event = pa_xnew0(pa_io_event, 1); > +Â Â Â Â event->rtpoll = rtpoll; > +Â Â Â Â event->rtpoll_item = pa_rtpoll_item_new(rtpoll, > PA_RTPOLL_NORMAL, 1); > +Â Â Â Â pa_rtpoll_item_set_work_callback(event->rtpoll_item, > io_event_work_cb); > +Â Â Â Â pa_rtpoll_item_set_userdata(event->rtpoll_item, event); > +Â Â Â Â pollfd = pa_rtpoll_item_get_pollfd(event->rtpoll_item, NULL); > +Â Â Â Â pollfd->fd = fd; > +Â Â Â Â event->callback = callback; > +Â Â Â Â event->userdata = userdata; > + > +Â Â Â Â pa_dynarray_append(rtpoll->io_events, event); > +Â Â Â Â io_event_enable(event, events); > + > +Â Â Â Â return event; > +} > + > +static void io_event_free(pa_io_event *event) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â pa_dynarray_remove_by_data(event->rtpoll->io_events, event); > + > +Â Â Â Â if (event->destroy_callback) > +Â Â Â Â Â Â Â Â event->destroy_callback(&event->rtpoll->mainloop_api, event, > event->userdata); > + > +Â Â Â Â if (event->rtpoll_item) > +Â Â Â Â Â Â Â Â pa_rtpoll_item_free(event->rtpoll_item); > + > +Â Â Â Â pa_xfree(event); > +} > + > +static void io_event_enable(pa_io_event *event, pa_io_event_flags_t > events) { > +Â Â Â Â struct pollfd *pollfd; > + > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â if (events == event->events) > +Â Â Â Â Â Â Â Â return; > + > +Â Â Â Â event->events = events; > + > +Â Â Â Â pollfd = pa_rtpoll_item_get_pollfd(event->rtpoll_item, NULL); > +Â Â Â Â pollfd->events = map_flags_to_libc(events); > +} > + > +static void io_event_set_destroy(pa_io_event *event, > pa_io_event_destroy_cb_t callback) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â event->destroy_callback = callback; > +} > + > +static pa_usec_t make_rt(const struct timeval *tv, bool > *use_rtclock) { > +Â Â Â Â struct timeval ttv; > + > +Â Â Â Â if (!tv) { > +Â Â Â Â Â Â Â Â *use_rtclock = false; > +Â Â Â Â Â Â Â Â return PA_USEC_INVALID; > +Â Â Â Â } > + > +Â Â Â Â ttv = *tv; > +Â Â Â Â *use_rtclock = !!(ttv.tv_usec & PA_TIMEVAL_RTCLOCK); > + > +Â Â Â Â if (*use_rtclock) > +Â Â Â Â Â Â Â Â ttv.tv_usec &= ~PA_TIMEVAL_RTCLOCK; > +Â Â Â Â else > +Â Â Â Â Â Â Â Â pa_rtclock_from_wallclock(&ttv); > + > +Â Â Â Â return pa_timeval_load(&ttv); > +} Same comment for code reuse about this. > +static pa_time_event* time_event_new(pa_mainloop_api *api, const > struct timeval *tv, pa_time_event_cb_t callback, > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â void *userdata) { > +Â Â Â Â pa_rtpoll *rtpoll; > +Â Â Â Â pa_time_event *event; > + > +Â Â Â Â pa_assert(api); > +Â Â Â Â pa_assert(api->userdata); > +Â Â Â Â pa_assert(callback); > + > +Â Â Â Â rtpoll = api->userdata; > +Â Â Â Â pa_assert(api == &rtpoll->mainloop_api); > + > +Â Â Â Â event = pa_xnew0(pa_time_event, 1); > +Â Â Â Â event->rtpoll = rtpoll; > +Â Â Â Â event->time = PA_USEC_INVALID; > +Â Â Â Â event->callback = callback; > +Â Â Â Â event->userdata = userdata; > + > +Â Â Â Â pa_dynarray_append(rtpoll->time_events, event); > +Â Â Â Â time_event_restart(event, tv); > + > +Â Â Â Â return event; > +} > + > +static void time_event_free(pa_time_event *event) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â time_event_restart(event, NULL); > +Â Â Â Â pa_dynarray_remove_by_data(event->rtpoll->time_events, event); > + > +Â Â Â Â if (event->destroy_callback) > +Â Â Â Â Â Â Â Â event->destroy_callback(&event->rtpoll->mainloop_api, event, > event->userdata); > + > +Â Â Â Â pa_xfree(event); > +} > + > +static void time_event_restart(pa_time_event *event, const struct > timeval *tv) { > +Â Â Â Â pa_usec_t t; > +Â Â Â Â bool use_rtclock; > +Â Â Â Â bool enabled; > +Â Â Â Â bool old_enabled; > + > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â t = make_rt(tv, &use_rtclock); > +Â Â Â Â enabled = (t != PA_USEC_INVALID); > +Â Â Â Â old_enabled = event->enabled; > + > +Â Â Â Â /* We return early only if the event stays disabled. If the > event stays > +Â Â Â Â Â * enabled, we can't return early, because the event time may > change. */ > +Â Â Â Â if (!enabled && !old_enabled) > +Â Â Â Â Â Â Â Â return; > + > +Â Â Â Â event->enabled = enabled; > +Â Â Â Â event->time = t; > +Â Â Â Â event->use_rtclock = use_rtclock; > + > +Â Â Â Â if (enabled && !old_enabled) > +Â Â Â Â Â Â Â Â pa_dynarray_append(event->rtpoll->enabled_time_events, > event); > +Â Â Â Â else if (!enabled) { > +Â Â Â Â Â Â Â Â pa_dynarray_remove_by_data(event->rtpoll- > >enabled_time_events, event); > +Â Â Â Â Â Â Â Â pa_dynarray_remove_by_data(event->rtpoll- > >expired_time_events, event); > +Â Â Â Â } > + > +Â Â Â Â if (event->rtpoll->cached_next_time_event == event) > +Â Â Â Â Â Â Â Â event->rtpoll->cached_next_time_event = NULL; > + > +Â Â Â Â if (event->rtpoll->cached_next_time_event && enabled) { > +Â Â Â Â Â Â Â Â pa_assert(event->rtpoll->cached_next_time_event->enabled); > + > +Â Â Â Â Â Â Â Â if (t < event->rtpoll->cached_next_time_event->time) > +Â Â Â Â Â Â Â Â Â Â Â Â event->rtpoll->cached_next_time_event = event; > +Â Â Â Â } > +} > + > +static void time_event_set_destroy(pa_time_event *event, > pa_time_event_destroy_cb_t callback) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â event->destroy_callback = callback; > +} > + > +static pa_defer_event* defer_event_new(pa_mainloop_api *api, > pa_defer_event_cb_t callback, void *userdata) { > +Â Â Â Â pa_rtpoll *rtpoll; > +Â Â Â Â pa_defer_event *event; > + > +Â Â Â Â pa_assert(api); > +Â Â Â Â pa_assert(api->userdata); > +Â Â Â Â pa_assert(callback); > + > +Â Â Â Â rtpoll = api->userdata; > +Â Â Â Â pa_assert(api == &rtpoll->mainloop_api); > + > +Â Â Â Â event = pa_xnew0(pa_defer_event, 1); > +Â Â Â Â event->rtpoll = rtpoll; > +Â Â Â Â event->callback = callback; > +Â Â Â Â event->userdata = userdata; > + > +Â Â Â Â pa_dynarray_append(rtpoll->defer_events, event); > +Â Â Â Â defer_event_enable(event, true); > + > +Â Â Â Â return event; > +} > + > +static void defer_event_free(pa_defer_event *event) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â defer_event_enable(event, false); > +Â Â Â Â pa_dynarray_remove_by_data(event->rtpoll->defer_events, event); > + > +Â Â Â Â if (event->destroy_callback) > +Â Â Â Â Â Â Â Â event->destroy_callback(&event->rtpoll->mainloop_api, event, > event->userdata); > + > +Â Â Â Â pa_xfree(event); > +} > + > +static void defer_event_enable(pa_defer_event *event, int enable) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â if (enable == event->enabled) > +Â Â Â Â Â Â Â Â return; > + > +Â Â Â Â event->enabled = enable; > + > +Â Â Â Â if (enable) > +Â Â Â Â Â Â Â Â pa_dynarray_append(event->rtpoll->enabled_defer_events, > event); > +Â Â Â Â else > +Â Â Â Â Â Â Â Â pa_dynarray_remove_by_data(event->rtpoll- > >enabled_defer_events, event); > +} > + > +static void defer_event_set_destroy(pa_defer_event *event, > pa_defer_event_destroy_cb_t callback) { > +Â Â Â Â pa_assert(event); > + > +Â Â Â Â event->destroy_callback = callback; > +} > + > +static void mainloop_api_quit(pa_mainloop_api *api, int retval) { > +Â Â Â Â pa_rtpoll *rtpoll; > + > +Â Â Â Â pa_assert(api); > +Â Â Â Â pa_assert(api->userdata); > + > +Â Â Â Â rtpoll = api->userdata; > +Â Â Â Â pa_assert(api == &rtpoll->mainloop_api); > + > +Â Â Â Â pa_rtpoll_quit(rtpoll); > +} > + > +static const pa_mainloop_api vtable = { > +Â Â Â Â .userdata = NULL, > + > +Â Â Â Â .io_new = io_event_new, > +Â Â Â Â .io_enable = io_event_enable, > +Â Â Â Â .io_free = io_event_free, > +Â Â Â Â .io_set_destroy = io_event_set_destroy, > + > +Â Â Â Â .time_new = time_event_new, > +Â Â Â Â .time_restart = time_event_restart, > +Â Â Â Â .time_free = time_event_free, > +Â Â Â Â .time_set_destroy = time_event_set_destroy, > + > +Â Â Â Â .defer_new = defer_event_new, > +Â Â Â Â .defer_enable = defer_event_enable, > +Â Â Â Â .defer_free = defer_event_free, > +Â Â Â Â .defer_set_destroy = defer_event_set_destroy, > + > +Â Â Â Â .quit = mainloop_api_quit, > +}; > + > Â pa_rtpoll *pa_rtpoll_new(void) { > Â Â Â Â Â pa_rtpoll *p; > Â > @@ -99,6 +425,15 @@ pa_rtpoll *pa_rtpoll_new(void) { > Â Â Â Â Â p->timestamp = pa_rtclock_now(); > Â #endif > Â > +Â Â Â Â p->mainloop_api = vtable; > +Â Â Â Â p->mainloop_api.userdata = p; > +Â Â Â Â p->io_events = pa_dynarray_new(NULL); > +Â Â Â Â p->time_events = pa_dynarray_new(NULL); > +Â Â Â Â p->enabled_time_events = pa_dynarray_new(NULL); > +Â Â Â Â p->expired_time_events = pa_dynarray_new(NULL); > +Â Â Â Â p->defer_events = pa_dynarray_new(NULL); > +Â Â Â Â p->enabled_defer_events = pa_dynarray_new(NULL); > + > Â Â Â Â Â return p; > Â } > Â > @@ -167,15 +502,108 @@ static void rtpoll_item_destroy(pa_rtpoll_item > *i) { > Â void pa_rtpoll_free(pa_rtpoll *p) { > Â Â Â Â Â pa_assert(p); > Â > +Â Â Â Â if (p->defer_events) { > +Â Â Â Â Â Â Â Â pa_defer_event *event; > + > +Â Â Â Â Â Â Â Â while ((event = pa_dynarray_last(p->defer_events))) > +Â Â Â Â Â Â Â Â Â Â Â Â defer_event_free(event); > +Â Â Â Â } > + > +Â Â Â Â if (p->time_events) { > +Â Â Â Â Â Â Â Â pa_time_event *event; > + > +Â Â Â Â Â Â Â Â while ((event = pa_dynarray_last(p->time_events))) > +Â Â Â Â Â Â Â Â Â Â Â Â time_event_free(event); > +Â Â Â Â } > + > +Â Â Â Â if (p->io_events) { > +Â Â Â Â Â Â Â Â pa_io_event *event; > + > +Â Â Â Â Â Â Â Â while ((event = pa_dynarray_last(p->io_events))) > +Â Â Â Â Â Â Â Â Â Â Â Â io_event_free(event); > +Â Â Â Â } > + > Â Â Â Â Â while (p->items) > Â Â Â Â Â Â Â Â Â rtpoll_item_destroy(p->items); > Â > +Â Â Â Â if (p->enabled_defer_events) { > +Â Â Â Â Â Â Â Â pa_assert(pa_dynarray_size(p->enabled_defer_events) == 0); These asserts seem kind of useless given that we have a loop cleaning up the dynarray prior to this. > +Â Â Â Â Â Â Â Â pa_dynarray_free(p->enabled_defer_events); > +Â Â Â Â } > + > +Â Â Â Â if (p->defer_events) { > +Â Â Â Â Â Â Â Â pa_assert(pa_dynarray_size(p->defer_events) == 0); > +Â Â Â Â Â Â Â Â pa_dynarray_free(p->defer_events); > +Â Â Â Â } > + > +Â Â Â Â if (p->expired_time_events) { > +Â Â Â Â Â Â Â Â pa_assert(pa_dynarray_size(p->expired_time_events) == 0); > +Â Â Â Â Â Â Â Â pa_dynarray_free(p->expired_time_events); > +Â Â Â Â } > + > +Â Â Â Â if (p->enabled_time_events) { > +Â Â Â Â Â Â Â Â pa_assert(pa_dynarray_size(p->enabled_time_events) == 0); > +Â Â Â Â Â Â Â Â pa_dynarray_free(p->enabled_time_events); > +Â Â Â Â } > + > +Â Â Â Â if (p->time_events) { > +Â Â Â Â Â Â Â Â pa_assert(pa_dynarray_size(p->time_events) == 0); > +Â Â Â Â Â Â Â Â pa_dynarray_free(p->time_events); > +Â Â Â Â } > + > +Â Â Â Â if (p->io_events) { > +Â Â Â Â Â Â Â Â pa_assert(pa_dynarray_size(p->io_events) == 0); > +Â Â Â Â Â Â Â Â pa_dynarray_free(p->io_events); > +Â Â Â Â } > + > Â Â Â Â Â pa_xfree(p->pollfd); > Â Â Â Â Â pa_xfree(p->pollfd2); > Â > Â Â Â Â Â pa_xfree(p); > Â } > Â > +pa_mainloop_api *pa_rtpoll_get_mainloop_api(pa_rtpoll *rtpoll) { > +Â Â Â Â pa_assert(rtpoll); > + > +Â Â Â Â return &rtpoll->mainloop_api; Is there any value to having each rtpoll have its own copy of this, rather than just returning the vtable directly here? > +} > + > +static void find_expired_time_events(pa_rtpoll *rtpoll) { > +Â Â Â Â pa_usec_t now; > +Â Â Â Â pa_time_event *event; > +Â Â Â Â unsigned idx; > + > +Â Â Â Â pa_assert(rtpoll); > +Â Â Â Â pa_assert(pa_dynarray_size(rtpoll->expired_time_events) == 0); > + > +Â Â Â Â now = pa_rtclock_now(); > + > +Â Â Â Â PA_DYNARRAY_FOREACH(event, rtpoll->enabled_time_events, idx) { > +Â Â Â Â Â Â Â Â if (event->time <= now) > +Â Â Â Â Â Â Â Â Â Â Â Â pa_dynarray_append(rtpoll->expired_time_events, event); > +Â Â Â Â } > +} > + > +static pa_time_event *find_next_time_event(pa_rtpoll *rtpoll) { > +Â Â Â Â pa_time_event *event; > +Â Â Â Â pa_time_event *result = NULL; > +Â Â Â Â unsigned idx; > + > +Â Â Â Â pa_assert(rtpoll); > + > +Â Â Â Â if (rtpoll->cached_next_time_event) > +Â Â Â Â Â Â Â Â return rtpoll->cached_next_time_event; > + > +Â Â Â Â PA_DYNARRAY_FOREACH(event, rtpoll->enabled_time_events, idx) { > +Â Â Â Â Â Â Â Â if (!result || event->time < result->time) > +Â Â Â Â Â Â Â Â Â Â Â Â result = event; > +Â Â Â Â } > + > +Â Â Â Â rtpoll->cached_next_time_event = result; > + > +Â Â Â Â return result; > +} > + > Â static void reset_revents(pa_rtpoll_item *i) { > Â Â Â Â Â struct pollfd *f; > Â Â Â Â Â unsigned n; > @@ -204,9 +632,14 @@ static void reset_all_revents(pa_rtpoll *p) { > Â } > Â > Â int pa_rtpoll_run(pa_rtpoll *p) { > +Â Â Â Â pa_defer_event *defer_event; > +Â Â Â Â pa_time_event *time_event; > Â Â Â Â Â pa_rtpoll_item *i; > Â Â Â Â Â int r = 0; > Â Â Â Â Â struct timeval timeout; > +Â Â Â Â pa_time_event *next_time_event; > +Â Â Â Â struct timeval next_time_event_elapse; > +Â Â Â Â bool timer_enabled; > Â > Â Â Â Â Â pa_assert(p); > Â Â Â Â Â pa_assert(!p->running); > @@ -218,7 +651,28 @@ int pa_rtpoll_run(pa_rtpoll *p) { > Â Â Â Â Â p->running = true; > Â Â Â Â Â p->timer_elapsed = false; > Â > -Â Â Â Â /* First, let's do some work */ > +Â Â Â Â /* Dispatch all enabled defer events. */ > +Â Â Â Â while ((defer_event = pa_dynarray_last(p- > >enabled_defer_events))) { > +Â Â Â Â Â Â Â Â if (p->quit) > +Â Â Â Â Â Â Â Â Â Â Â Â break; > + > +Â Â Â Â Â Â Â Â defer_event->callback(&p->mainloop_api, defer_event, > defer_event->userdata); > +Â Â Â Â } Am I missing something, or is this an infinite loop if there are any enabled defer events? > + > +Â Â Â Â /* Dispatch all expired time events. */ > +Â Â Â Â find_expired_time_events(p); > +Â Â Â Â while ((time_event = pa_dynarray_last(p->expired_time_events))) > { > +Â Â Â Â Â Â Â Â struct timeval tv; > + > +Â Â Â Â Â Â Â Â if (p->quit) > +Â Â Â Â Â Â Â Â Â Â Â Â break; > + > +Â Â Â Â Â Â Â Â time_event_restart(time_event, NULL); > +Â Â Â Â Â Â Â Â time_event->callback(&p->mainloop_api, time_event, > pa_timeval_rtstore(&tv, time_event->time, time_event->use_rtclock), > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â time_event->userdata); > +Â Â Â Â } > + > +Â Â Â Â /* Let's do some work */ > Â Â Â Â Â for (i = p->items; i && i->priority < PA_RTPOLL_NEVER; i = i- > >next) { > Â Â Â Â Â Â Â Â Â int k; > Â > @@ -282,15 +736,40 @@ int pa_rtpoll_run(pa_rtpoll *p) { > Â Â Â Â Â if (p->rebuild_needed) > Â Â Â Â Â Â Â Â Â rtpoll_rebuild(p); > Â > +Â Â Â Â /* Calculate timeout */ > + > Â Â Â Â Â pa_zero(timeout); > Â > -Â Â Â Â /* Calculate timeout */ > -Â Â Â Â if (!p->quit && p->timer_enabled) { > +Â Â Â Â next_time_event = find_next_time_event(p); > +Â Â Â Â if (next_time_event) > +Â Â Â Â Â Â Â Â pa_timeval_rtstore(&next_time_event_elapse, next_time_event- > >time, next_time_event->use_rtclock); > + > +Â Â Â Â /* p->timer_enabled and p->next_elapse are controlled by the > rtpoll owner, > +Â Â Â Â Â * while the time events can be created by anyone through > pa_mainloop_api. > +Â Â Â Â Â * It might be a good idea to merge p->timer_enabled and p- > >next_elapse > +Â Â Â Â Â * with the time events so that we wouldn't need to handle them > separately > +Â Â Â Â Â * here. The reason why they are currently separate is that the > +Â Â Â Â Â * pa_mainloop_api interface was bolted on pa_rtpoll as an > afterthought. */ > +Â Â Â Â timer_enabled = p->timer_enabled || next_time_event; > + > +Â Â Â Â if (!p->quit && timer_enabled) { > +Â Â Â Â Â Â Â Â struct timeval *next_elapse; > Â Â Â Â Â Â Â Â Â struct timeval now; > + > +Â Â Â Â Â Â Â Â if (p->timer_enabled && next_time_event) { > +Â Â Â Â Â Â Â Â Â Â Â Â if (pa_timeval_cmp(&p->next_elapse, > &next_time_event_elapse) > 0) > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â next_elapse = &next_time_event_elapse; > +Â Â Â Â Â Â Â Â Â Â Â Â else > +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â next_elapse = &p->next_elapse; > +Â Â Â Â Â Â Â Â } else if (p->timer_enabled) > +Â Â Â Â Â Â Â Â Â Â Â Â next_elapse = &p->next_elapse; > +Â Â Â Â Â Â Â Â else > +Â Â Â Â Â Â Â Â Â Â Â Â next_elapse = &next_time_event_elapse; > + > Â Â Â Â Â Â Â Â Â pa_rtclock_get(&now); > Â > -Â Â Â Â Â Â Â Â if (pa_timeval_cmp(&p->next_elapse, &now) > 0) > -Â Â Â Â Â Â Â Â Â Â Â Â pa_timeval_add(&timeout, pa_timeval_diff(&p- > >next_elapse, &now)); > +Â Â Â Â Â Â Â Â if (pa_timeval_cmp(next_elapse, &now) > 0) > +Â Â Â Â Â Â Â Â Â Â Â Â pa_timeval_add(&timeout, pa_timeval_diff(next_elapse, > &now)); > Â Â Â Â Â } > Â > Â #ifdef DEBUG_TIMING > @@ -298,7 +777,7 @@ int pa_rtpoll_run(pa_rtpoll *p) { > Â Â Â Â Â Â Â Â Â pa_usec_t now = pa_rtclock_now(); > Â Â Â Â Â Â Â Â Â p->awake = now - p->timestamp; > Â Â Â Â Â Â Â Â Â p->timestamp = now; > -Â Â Â Â Â Â Â Â if (!p->quit && p->timer_enabled) > +Â Â Â Â Â Â Â Â if (!p->quit && timer_enabled) > Â Â Â Â Â Â Â Â Â Â Â Â Â pa_log("poll timeout: %d ms ",(int) > ((timeout.tv_sec*1000) + (timeout.tv_usec / 1000))); > Â Â Â Â Â Â Â Â Â else if (q->quit) > Â Â Â Â Â Â Â Â Â Â Â Â Â pa_log("poll timeout is ZERO"); > @@ -313,12 +792,21 @@ int pa_rtpoll_run(pa_rtpoll *p) { > Â Â Â Â Â Â Â Â Â struct timespec ts; > Â Â Â Â Â Â Â Â Â ts.tv_sec = timeout.tv_sec; > Â Â Â Â Â Â Â Â Â ts.tv_nsec = timeout.tv_usec * 1000; > -Â Â Â Â Â Â Â Â r = ppoll(p->pollfd, p->n_pollfd_used, (p->quit || p- > >timer_enabled) ? &ts : NULL, NULL); > +Â Â Â Â Â Â Â Â r = ppoll(p->pollfd, p->n_pollfd_used, (p->quit || > timer_enabled) ? &ts : NULL, NULL); > Â Â Â Â Â } > Â #else > -Â Â Â Â r = pa_poll(p->pollfd, p->n_pollfd_used, (p->quit || p- > >timer_enabled) ? (int) ((timeout.tv_sec*1000) + (timeout.tv_usec / > 1000)) : -1); > +Â Â Â Â r = pa_poll(p->pollfd, p->n_pollfd_used, (p->quit || > timer_enabled) ? (int) ((timeout.tv_sec*1000) + (timeout.tv_usec / > 1000)) : -1); > Â #endif > Â > +Â Â Â Â /* FIXME: We don't know whether the pa_rtpoll owner's timer > elapsed or one > +Â Â Â Â Â * of the time events created by others through pa_mainloop_api. > The alsa > +Â Â Â Â Â * sink and source use pa_rtpoll_timer_elapsed() to check > whether *their* > +Â Â Â Â Â * timer elapsed, so this ambiguity is a problem for them in > theory. > +Â Â Â Â Â * However, currently the pa_rtpoll objects of the alsa sink and > source are > +Â Â Â Â Â * not being used through pa_mainloop_api, so in practice > there's no > +Â Â Â Â Â * ambiguity. We could use pa_rtclock_now() to check whether p- > >next_elapse > +Â Â Â Â Â * is in the past, but we don't do that currently, because > pa_rtclock_now() > +Â Â Â Â Â * is somewhat expensive and this ambiguity isn't currently a > big issue. */ > Â Â Â Â Â p->timer_elapsed = r == 0; > Â > Â #ifdef DEBUG_TIMING > diff --git a/src/pulsecore/rtpoll.h b/src/pulsecore/rtpoll.h > index c0a4dda..e2aee73 100644 > --- a/src/pulsecore/rtpoll.h > +++ b/src/pulsecore/rtpoll.h > @@ -25,7 +25,9 @@ > Â #include <sys/types.h> > Â #include <limits.h> > Â > +#include <pulse/mainloop-api.h> > Â #include <pulse/sample.h> > + > Â #include <pulsecore/asyncmsgq.h> > Â #include <pulsecore/fdsem.h> > Â #include <pulsecore/macro.h> > @@ -56,6 +58,8 @@ typedef enum pa_rtpoll_priority { > Â pa_rtpoll *pa_rtpoll_new(void); > Â void pa_rtpoll_free(pa_rtpoll *p); > Â > +pa_mainloop_api *pa_rtpoll_get_mainloop_api(pa_rtpoll *rtpoll); > + > Â /* Sleep on the rtpoll until the time event, or any of the fd events > Â * is triggered. Returns negative on error, positive if the loop > Â * should continue to run, 0 when the loop should be terminated Modulo looking at the test and making sure everything works as expected (and we can also make sure it is valgrind clean, manually). This looks okay to me. -- Arun