[PATCH v2 libevdev] Drain all events before synchronizing after SYN_DROPPED
Peter Hutterer
peter.hutterer at who-t.net
Wed Apr 9 17:57:04 PDT 2014
The kernel ring buffer drops all events on SYN_DROPPED, but then continues to
fill up again. So by the time we read the events, the kernel's client buffer is
essentially like this:
SYN_DROPPED, ev1, ev2, ev3, ...., evN
The kernel's device state represents the device after evN, and that is what
the ioctls return. So we can't actually sync while there are events on the
wire because the events represent an earlier state. So simply discard all
events in the kernel buffer, synchronize, and then start processing again. We
lose some granularity but at least the events are correct.
Note that the above is true for EV_ABS events, for EV_KEY, EV_SND, EV_LED and
EV_SW the kernel removes potential duplicates from the client buffer anyway.
Really, this should be fixed in the kernel for EV_ABS, but until then we need
this solution.
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
Changes to v1:
- assign the right slot value
- add test for changing the opposite tracking ID direction, so now we test
going from N to -1, and from -1 to N.
libevdev/libevdev.c | 58 ++++++++----
libevdev/libevdev.h | 45 +++++++++
test/test-libevdev-events.c | 219 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 302 insertions(+), 20 deletions(-)
diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
index 7e8d3db..a316831 100644
--- a/libevdev/libevdev.c
+++ b/libevdev/libevdev.c
@@ -726,32 +726,50 @@ read_more_events(struct libevdev *dev)
return 0;
}
+static inline void
+drain_events(struct libevdev *dev)
+{
+ int rc;
+ size_t nelem;
+ int iterations = 0;
+ const int max_iterations = 8; /* EVDEV_BUF_PACKETS in
+ kernel/drivers/input/evedev.c */
+
+ queue_shift_multiple(dev, queue_num_elements(dev), NULL);
+
+ do {
+ rc = read_more_events(dev);
+ if (rc == -EAGAIN)
+ return;
+
+ if (rc < 0) {
+ log_error("Failed to drain events before sync.\n");
+ return;
+ }
+
+ nelem = queue_num_elements(dev);
+ queue_shift_multiple(dev, nelem, NULL);
+ } while (iterations++ < max_iterations && nelem >= queue_size(dev));
+
+ /* Our buffer should be roughly the same or bigger than the kernel
+ buffer in most cases, so we usually don't expect to recurse. If
+ we do, make sure we stop after max_iterations and proceed with
+ what we have. This could happen if events queue up faster than
+ we can drain them.
+ */
+ if (iterations >= max_iterations)
+ log_info("Unable to drain events, buffer size mismatch.\n");
+}
+
static int
sync_state(struct libevdev *dev)
{
- int i;
int rc = 0;
struct input_event *ev;
- /* FIXME: if we have events in the queue after the SYN_DROPPED (which was
- queue[0]) we need to shift this backwards. Except that chances are that the
- queue may be either full or too full to prepend all the events needed for
- SYNC_IN_PROGRESS.
-
- so we search for the last sync event in the queue and drop everything before
- including that event and rely on the kernel to tell us the right value for that
- bitfield during the sync process.
- */
-
- for (i = queue_num_elements(dev) - 1; i >= 0; i--) {
- struct input_event e = {{0,0}, 0, 0, 0};
- queue_peek(dev, i, &e);
- if (e.type == EV_SYN)
- break;
- }
-
- if (i > 0)
- queue_shift_multiple(dev, i + 1, NULL);
+ /* see section "Discarding events before synchronizing" in
+ * libevdev/libevdev.h */
+ drain_events(dev);
if (libevdev_has_event_type(dev, EV_KEY))
rc = sync_key_state(dev);
diff --git a/libevdev/libevdev.h b/libevdev/libevdev.h
index d79b191..95f1e19 100644
--- a/libevdev/libevdev.h
+++ b/libevdev/libevdev.h
@@ -367,6 +367,51 @@ extern "C" {
* The axis events do not reflect the position of a current touch point, a
* client must take care not to generate a new touch point based on those
* updates.
+ *
+ * Discarding events before synchronizing
+ * =====================================
+ *
+ * The kernel implements the client buffer as a ring buffer. SYN_DROPPED
+ * events are handled when the buffer is full and a new event is received
+ * from a device. All existing events are discarded, a SYN_DROPPED is added
+ * to the buffer followed by the actual device event. Further events will be
+ * appended to the buffer until it is either read by the client, or filled
+ * again, at which point the sequence repeats.
+ *
+ * When the client reads the buffer, the buffer will thus always consist of
+ * exactly one SYN_DROPPED event followed by an unspecified number of real
+ * events. The data the ioctls return is the current state of the device,
+ * i.e. the state after all these events have been processed. For example,
+ * assume the buffer contains the following sequence:
+ *
+ * @code
+ EV_SYN SYN_DROPPED
+ EV_ABS ABS_X 1
+ EV_SYN SYN_REPORT 0
+ EV_ABS ABS_X 2
+ EV_SYN SYN_REPORT 0
+ EV_ABS ABS_X 3
+ EV_SYN SYN_REPORT 0
+ EV_ABS ABS_X 4
+ EV_SYN SYN_REPORT 0
+ EV_ABS ABS_X 5
+ EV_SYN SYN_REPORT 0
+ EV_ABS ABS_X 6
+ EV_SYN SYN_REPORT 0
+ * @endcode
+ * An ioctl at any time in this sequence will return a value of 6 for ABS_X.
+ *
+ * libevdev discards all events after a SYN_DROPPED to ensure the events
+ * during @ref LIBEVDEV_READ_FLAG_SYNC represent the last known state of the
+ * device. This loses some granularity of the events especially as the time
+ * between the SYN_DROPPED and the sync process increases. It does however
+ * avoid spurious cursor movements. In the above example, the event sequence
+ * by libevdev is:
+ * @code
+ EV_SYN SYN_DROPPED
+ EV_ABS ABS_X 6
+ EV_SYN SYN_REPORT 0
+ @endcode
*/
/**
diff --git a/test/test-libevdev-events.c b/test/test-libevdev-events.c
index dc412a7..ddc509d 100644
--- a/test/test-libevdev-events.c
+++ b/test/test-libevdev-events.c
@@ -927,6 +927,224 @@ START_TEST(test_syn_delta_tracking_ids)
}
END_TEST
+START_TEST(test_syn_delta_late_sync)
+{
+ struct uinput_device* uidev;
+ struct libevdev *dev;
+ int rc;
+ struct input_event ev;
+ struct input_absinfo abs[6];
+ int i, slot;
+
+ memset(abs, 0, sizeof(abs));
+ abs[0].value = ABS_X;
+ abs[0].maximum = 1000;
+ abs[1].value = ABS_MT_POSITION_X;
+ abs[1].maximum = 1000;
+
+ abs[2].value = ABS_Y;
+ abs[2].maximum = 1000;
+ abs[3].value = ABS_MT_POSITION_Y;
+ abs[3].maximum = 1000;
+
+ abs[4].value = ABS_MT_SLOT;
+ abs[4].maximum = 1;
+
+ abs[5].minimum = -1;
+ abs[5].maximum = 255;
+ abs[5].value = ABS_MT_TRACKING_ID;
+
+ test_create_abs_device(&uidev, &dev,
+ 6, abs,
+ EV_SYN, SYN_REPORT,
+ -1);
+
+ /* emulate a touch down, make sure libevdev sees it */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 1);
+ uinput_device_event(uidev, EV_ABS, ABS_X, 100);
+ uinput_device_event(uidev, EV_ABS, ABS_Y, 500);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+ do {
+ rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+ ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC);
+ } while (rc >= 0);
+
+ /* force enough events to trigger a SYN_DROPPED */
+ for (i = 0; i < 100; i++) {
+ uinput_device_event(uidev, EV_ABS, ABS_X, 100 + i);
+ uinput_device_event(uidev, EV_ABS, ABS_Y, 500 + i);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100 + i);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500 + i);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+ }
+
+ rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC);
+
+ /* trigger the tracking ID change after getting the SYN_DROPPED */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ uinput_device_event(uidev, EV_ABS, ABS_X, 200);
+ uinput_device_event(uidev, EV_ABS, ABS_Y, 600);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 200);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 600);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ slot = 0;
+
+ /* Now sync the device, expect the data to be equal to the last event*/
+ while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev)) != -EAGAIN) {
+ if (ev.type == EV_SYN)
+ continue;
+
+ ck_assert_int_eq(ev.type, EV_ABS);
+ switch(ev.code) {
+ case ABS_MT_SLOT:
+ slot = ev.value;
+ break;
+ case ABS_MT_TRACKING_ID:
+ if (slot == 0)
+ ck_assert_int_eq(ev.value, -1);
+ break;
+ case ABS_X:
+ case ABS_MT_POSITION_X:
+ ck_assert_int_eq(ev.value, 200);
+ break;
+ case ABS_Y:
+ case ABS_MT_POSITION_Y:
+ ck_assert_int_eq(ev.value, 600);
+ break;
+ }
+ }
+
+ /* And a new tracking ID */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 2);
+ uinput_device_event(uidev, EV_ABS, ABS_X, 201);
+ uinput_device_event(uidev, EV_ABS, ABS_Y, 601);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 201);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 601);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev)) != -EAGAIN) {
+ ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC);
+
+ if (ev.type == EV_SYN)
+ continue;
+
+ ck_assert_int_eq(ev.type, EV_ABS);
+
+ switch(ev.code) {
+ case ABS_MT_SLOT:
+ ck_assert_int_eq(ev.value, 0);
+ break;
+ case ABS_MT_TRACKING_ID:
+ ck_assert_int_eq(ev.value, 2);
+ break;
+ case ABS_X:
+ case ABS_MT_POSITION_X:
+ ck_assert_int_eq(ev.value, 201);
+ break;
+ case ABS_Y:
+ case ABS_MT_POSITION_Y:
+ ck_assert_int_eq(ev.value, 601);
+ break;
+ }
+ }
+
+
+ /* Now we basically re-do the exact same test, just with the
+ tracking ID order inverted */
+
+ /* drop the tracking ID, make sure libevdev sees it */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+ do {
+ rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+ ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC);
+ } while (rc >= 0);
+
+ /* force enough events to trigger a SYN_DROPPED */
+ for (i = 0; i < 100; i++) {
+ uinput_device_event(uidev, EV_ABS, ABS_X, 100 + i);
+ uinput_device_event(uidev, EV_ABS, ABS_Y, 500 + i);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 100 + i);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 500 + i);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+ }
+
+ rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+ ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SYNC);
+
+ /* trigger the new tracking ID after getting the SYN_DROPPED */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 5);
+ uinput_device_event(uidev, EV_ABS, ABS_X, 200);
+ uinput_device_event(uidev, EV_ABS, ABS_Y, 600);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_X, 200);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_POSITION_Y, 600);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ slot = 0;
+
+ /* Now sync the device, expect the data to be equal to the last event*/
+ while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_SYNC, &ev)) != -EAGAIN) {
+ if (ev.type == EV_SYN)
+ continue;
+
+ ck_assert_int_eq(ev.type, EV_ABS);
+ switch(ev.code) {
+ case ABS_MT_SLOT:
+ slot = ev.value;
+ break;
+ case ABS_MT_TRACKING_ID:
+ if (slot == 0)
+ ck_assert_int_eq(ev.value, 5);
+ break;
+ case ABS_X:
+ case ABS_MT_POSITION_X:
+ ck_assert_int_eq(ev.value, 200);
+ break;
+ case ABS_Y:
+ case ABS_MT_POSITION_Y:
+ ck_assert_int_eq(ev.value, 600);
+ break;
+ }
+ }
+
+ /* Drop the tracking ID */
+ uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 0);
+ uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+ uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+ while ((rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev)) != -EAGAIN) {
+ ck_assert_int_ne(rc, LIBEVDEV_READ_STATUS_SYNC);
+
+ if (ev.type == EV_SYN)
+ continue;
+
+ ck_assert_int_eq(ev.type, EV_ABS);
+
+ switch(ev.code) {
+ case ABS_MT_SLOT:
+ ck_assert_int_eq(ev.value, 0);
+ break;
+ case ABS_MT_TRACKING_ID:
+ ck_assert_int_eq(ev.value, -1);
+ break;
+ }
+ }
+
+
+ uinput_device_free(uidev);
+ libevdev_free(dev);
+}
+END_TEST
+
START_TEST(test_syn_delta_fake_mt)
{
struct uinput_device* uidev;
@@ -1863,6 +2081,7 @@ libevdev_events(void)
tcase_add_test(tc, test_syn_delta_sw);
tcase_add_test(tc, test_syn_delta_fake_mt);
tcase_add_test(tc, test_syn_delta_tracking_ids);
+ tcase_add_test(tc, test_syn_delta_late_sync);
suite_add_tcase(s, tc);
tc = tcase_create("skipped syncs");
--
1.9.0
More information about the Input-tools
mailing list