On Thu, 2016-03-03 at 18:02 +0200, Tanu Kaskinen wrote: > On Tue, 2016-02-09 at 12:42 +0200, Tanu Kaskinen wrote: > > > > On Tue, 2016-02-09 at 15:47 +0530, Arun Raghavan wrote: > > > > > > On Fri, 2015-01-02 at 15:04 +0200, Tanu Kaskinen wrote: > > > > > > > > +} > > > > + > > > > +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? > > As discussed in IRC, I did this like this, because pa_mainloop_run() in > > practice behaves the same way. However, mainloop-api.h says that each > > defer event runs only once per mainloop iteration, so I'll have to > > change this. > I started looking into this, and realized that the "once per mainloop > iteration" clause makes no difference to pa_mainloop_api users. There > is no pa_rtpoll_iterate(), and pa_rtpoll_run() will behave the same way > in any case, so is it really worth the effort to artificially refactor > the code to have "iterations"? But there is a difference of order, right? The mainloop API idea of an iteration is not just about pa_mainloop_iterate(), but also about something like:  1. Run all I/O events  2. Run all expired time events  3. Run all defer events  4. Goto 1 The order is not something we commit to, but the fact that we do one run of each is. Your code runs a loop in (3) until the defer event basically goes away. >From what I can tell, this should not required a radical restructuring. Instead of always picking pa_dynarray_last(), you could just iterate over all the enabled events once. -- Arun