Le mardi 25 septembre 2007, Lennart Poettering a ?crit?:
>
> A few comments:
>
> Are you sure you need three seperate "SocketNotifiers" in the IOEvent
> class? Can't you OR them together?
No, I can't OR them. The Qt documentation says explicitly that I need to use
one SocketNotifier per type of event spied.
>
> On POSIX POLLHUP, POLLERR, POLLNVAL are ignored when you write them to
> pollfd->events. They are only valid in ->revents. Thus I wonder about
> the validity of the third notifier you are using.
Yes but I need to tell Qt what I want to do when such events happens. That's
the job of the third QSocketNotifier.
>
> (Hmm, and did I mention that the QT event loop sucks big time? BTW:
> couldn't you just be using the glib event loop? I heard that
> internally Qt uses the glib mainloop these days anyway?)
Well I know that the Qt mainloop can be replaced be the glib one on Linux.
But I don't know about the other OS ( Windows and MacOSX ). Beside I don't
know how the Linux distribution are shipping Qt ( with the glib mainloop or
without it). So I think that so a fully Qt integration can be usefull.
>
> The implementation of IOEvent::onException() is not correct, because
> the event object might have been destructed from the callback. Hence
> the second callback might be called for an event that is no longer valid.
Ok, I have made some quick modifications, I will give a second look to be sure
there are no problemes.
>
> pa_usec_t is unsigned. The comparison < 0 is thus unlikely to
> succeed ever in TimeEvent::restart(). Use something like
> pa_timeval_cmp(pa_gettimeofday(&now), tv) < 0. Also, passing NULL as
> tv is supposed to deactivate the time event. Passing a time that
> already passed (including {0, 0}) should cause the event to be
> dispatched in the next event loop iteration. (a little bit like the
> defer event)
Well the NULL case is already correctly handled. And the other case to. When I
start a QTimer with a 0 as the delay, action associed with it will be
executed as soon as the program return in the mainloop. I have juste get ride
of the if that wasn't necessary.
>
> Otherwise this looks pretty good to me. Does the event loop
> abstraction work properly?
Well it seems to work correctly for the moment. I have been able to make an
test application that display the volume of the sinks and sources. It's get
correctly updated when I changed the volume with pavucontrol.
Nicolas
