[PATCH libevdev 1/3] Count the number of events needed for a full sync

[Peter Hutterer]

On Fri, Jan 17, 2014 at 01:15:02PM +0100, David Herrmann wrote:
> Hi
> 
> On Fri, Jan 17, 2014 at 4:31 AM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > Make sure we have a queue that is at least large enough to do a full sync
> > after a SYN_DROPPED, plus store a few extra events in case some came in after
> > the sync.
> 
> EVIOCGKEY() and friends flush the input queue, so there's no need to
> account for extra events. Obviously, we don't flush EV_REL or EV_ABS,
> as they don't suffer from synchronization-issues. So only these need
> extra space.
> 
> On the other hand, there might be incoming events between our
> sync-ioctl()s, so we can never be sure there're no new events.. So
> maybe I should just shut up..

there's one use-case where we need the ability to store all key events: when
a client runs LIBEVDEV_READ_FLAG_FORCE_SYNC, which you will have to do (in
most cases) after libevdev_change_fd(). for example in the X driver, if the
device is disabled and re-enabled, the fd changes. to make sure the library
has the right view of the device, you need FORCE_SYNC. otherwise, libevdev
may think a key is still down even though it was released ages ago.

> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  libevdev/libevdev.c | 34 ++++++++++++++++++++++++++++++----
> >  1 file changed, 30 insertions(+), 4 deletions(-)
> >
> > diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
> > index 8a37204..59625a4 100644
> > --- a/libevdev/libevdev.c
> > +++ b/libevdev/libevdev.c
> > @@ -41,12 +41,38 @@ static int sync_mt_state(struct libevdev *dev, int create_events);
> >  static int
> >  init_event_queue(struct libevdev *dev)
> >  {
> > -       /* FIXME: count the number of axes, keys, etc. to get a better idea at how many events per
> > -          EV_SYN we could possibly get. Then multiply that by the actual buffer size we care about */
> > +       const int MIN_QUEUE_SIZE = 256;
> > +       int nevents = 1; /* terminating SYN_REPORT */
> 
> You do "nevents * 2" below, but you account for a single report here?
> Seems overkill, but ok, doesn't hurt.
> 
> > +       int nslots;
> > +       unsigned int type, code;
> >
> > -       const int QUEUE_SIZE = 256;
> > +       /* count the number of axes, keys, etc. to get a better idea at how
> > +          many events per EV_SYN we could possibly get. That's the max we
> > +          may get during SYN_DROPPED too. Use double that, just so we have
> > +          room for events while syncing an event.
> > +        */
> > +       for (type = EV_KEY; type < EV_MAX; type++) {
> > +               int max = libevdev_event_type_get_max(type);
> > +               for (code = 0; max > 0 && code < (unsigned int) max; code++) {
> > +                       if (libevdev_has_event_code(dev, type, code))
> > +                               nevents++;
> > +               }
> > +       }
> >
> > -       return queue_alloc(dev, QUEUE_SIZE);
> > +       nslots = libevdev_get_num_slots(dev);
> > +       if (nslots > 1) {
> > +               int num_mt_axes = 0;
> > +
> > +               for (code = ABS_MT_SLOT; code < ABS_MAX; code++) {
> > +                       if (libevdev_has_event_code(dev, EV_ABS, code))
> > +                               num_mt_axes++;
> > +               }
> > +
> > +               /* We already counted the first slot in the initial count */
> > +               nevents += num_mt_axes * (nslots - 1);
> 
> Again a bit overkill to drop that one axis, but ok. Patch looks good to me:
> Reviewed-by: David Herrmann <dh.herrmann at gmail.com>

fwiw, just for correctness this is one slot, not just one axis, so
potentially 16 axes.
anyway, I admit that it seems overkill, I only added the * 2 late in the
patch. The reasoning is that we want the maximum number of possible events
plus a little bit. but "a little bit" is hard to define, so * 2 was the
easiest solution :)

Cheers,
   Peter

> > +       }
> > +
> > +       return queue_alloc(dev, min(MIN_QUEUE_SIZE, nevents * 2));
> >  }
> >
> >  static void
> > --
> > 1.8.4.2
> >
> > _______________________________________________
> > Input-tools mailing list
> > Input-tools at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/input-tools