Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Sensor API may send duplicate events with the same timestamp, confusing downstream consumers #9688

Open
antheas opened this issue May 4, 2024 · 0 comments
Assignees
Milestone

Comments

@antheas
Copy link

antheas commented May 4, 2024

Under certain circumstances, the sensor polling drivers may send an update without having new sensor data, which will contain a duplicate sensor timestamp. If the sensor timestamp is assumed to be unique by library users, this can cause a division by zero, which destabilizes their Gyro implementation.

Steam, at least, has this problem. It uses the sensor timestamp for both gyroscope and touchpad events. Any duplicate timestamp will cause a large camera jitter in game that makes it unusable (Under the Gyro to Mouse mode, or if the touchpad has enabled acceleration).

Dualsense Driver

The hidapi Dualsense driver will always send sensor events, even if the timestamp is the same (and perhaps overflow itself).

if (ctx->report_sensors) {
Uint64 sensor_timestamp;
float data[3];
if (ctx->use_alternate_report) {
/* 16-bit timestamp */
Uint32 delta;
Uint16 tick = LOAD16(packet->rgucSensorTimestamp[0],
packet->rgucSensorTimestamp[1]);
if (ctx->last_tick < tick) {
delta = (tick - ctx->last_tick);
} else {
delta = (SDL_MAX_UINT16 - ctx->last_tick + tick + 1);
}
ctx->last_tick = tick;
ctx->sensor_ticks += delta;
/* Sensor timestamp is in 1us units */
sensor_timestamp = SDL_US_TO_NS(ctx->sensor_ticks);
} else {
/* 32-bit timestamp */
Uint32 delta;
Uint32 tick = LOAD32(packet->rgucSensorTimestamp[0],
packet->rgucSensorTimestamp[1],
packet->rgucSensorTimestamp[2],
packet->rgucSensorTimestamp[3]);
if (ctx->last_tick < tick) {
delta = (tick - ctx->last_tick);
} else {
delta = (SDL_MAX_UINT32 - ctx->last_tick + tick + 1);
}
ctx->last_tick = tick;
ctx->sensor_ticks += delta;
/* Sensor timestamp is in 0.33us units */
sensor_timestamp = (ctx->sensor_ticks * SDL_NS_PER_US) / 3;
}
data[0] = HIDAPI_DriverPS5_ApplyCalibrationData(ctx, 0, LOAD16(packet->rgucGyroX[0], packet->rgucGyroX[1]));
data[1] = HIDAPI_DriverPS5_ApplyCalibrationData(ctx, 1, LOAD16(packet->rgucGyroY[0], packet->rgucGyroY[1]));
data[2] = HIDAPI_DriverPS5_ApplyCalibrationData(ctx, 2, LOAD16(packet->rgucGyroZ[0], packet->rgucGyroZ[1]));
SDL_SendJoystickSensor(timestamp, joystick, SDL_SENSOR_GYRO, sensor_timestamp, data, 3);
data[0] = HIDAPI_DriverPS5_ApplyCalibrationData(ctx, 3, LOAD16(packet->rgucAccelX[0], packet->rgucAccelX[1]));
data[1] = HIDAPI_DriverPS5_ApplyCalibrationData(ctx, 4, LOAD16(packet->rgucAccelY[0], packet->rgucAccelY[1]));
data[2] = HIDAPI_DriverPS5_ApplyCalibrationData(ctx, 5, LOAD16(packet->rgucAccelZ[0], packet->rgucAccelZ[1]));
SDL_SendJoystickSensor(timestamp, joystick, SDL_SENSOR_ACCEL, sensor_timestamp, data, 3);
}

This is not a problem with real dualsense controllers, as they always send unique timestamps (?), but may be a problem with third party controllers. It is certainly a problem with controller emulators such as those based on ViGEm on Windows (e.g., Handheld Companion, potentially Sunshine), or in the linux side on a project I'm involved in, Handheld Daemon.

The problem here arises from the fact that it's common for game controllers to have 200-500hz polling rates, whereas the onboard IMU of devices is around 100hz (e.g., Legion Go is 500hz, its controller gyros are 125hz, Bosch windows driver is 100hz).
On Linux we can go higher, up to 1600hz.

A potential solution to this is syncing the controller output to the gyro. This is done by Handheld Daemon and works perfectly well. However, this is not an ideal solution, as it lowers the polling rate of the controllers (e.g., Legion Go becomes 125hz).

Linux Evdev Driver

Currently, I am working on a reference evdev implementation for IMU in userspace in Handheld Daemon. It works quite well, both in Steam and in emulators.

However, when playing Spiderman Remastered (by emulating a JoyCon pair for gyro, paddles) once every 1-2 minutes I get a large gyro jitter while gaming in Steam. Here, the solution is out of my hands, as the duplicate timestamp is produced by SDL (I suspect).

In the following code, if SDL does not poll the sensor fast enough, it will receive a SYN_DROPPED event.

case SYN_DROPPED:
#ifdef DEBUG_INPUT_EVENTS
SDL_Log("Event SYN_DROPPED detected\n");
#endif
joystick->hwdata->recovering_from_dropped_sensor = SDL_TRUE;
break;
case SYN_REPORT:
if (joystick->hwdata->recovering_from_dropped_sensor) {
joystick->hwdata->recovering_from_dropped_sensor = SDL_FALSE;
PollAllSensors(SDL_GetTicksNS(), joystick); /* try to sync up to current state now */
} else {
Uint64 timestamp = SDL_EVDEV_GetEventTimestamp(event);
SDL_SendJoystickSensor(timestamp, joystick, SDL_SENSOR_GYRO,
SDL_US_TO_NS(joystick->hwdata->sensor_tick),
joystick->hwdata->gyro_data, 3);
SDL_SendJoystickSensor(timestamp, joystick, SDL_SENSOR_ACCEL,
SDL_US_TO_NS(joystick->hwdata->sensor_tick),
joystick->hwdata->accel_data, 3);

When that happens, the function PollAllSensors is called, which emits an event without updating the sensor_tick variable. This always results in a duplicate timestamp.

static void PollAllSensors(Uint64 timestamp, SDL_Joystick *joystick)
{
struct input_absinfo absinfo;
int i;
SDL_AssertJoysticksLocked();
SDL_assert(joystick->hwdata->fd_sensor >= 0);
if (joystick->hwdata->has_gyro) {
float data[3] = {0.0f, 0.0f, 0.0f};
for (i = 0; i < 3; i++) {
if (ioctl(joystick->hwdata->fd_sensor, EVIOCGABS(ABS_RX + i), &absinfo) >= 0) {
data[i] = absinfo.value * (SDL_PI_F / 180.f) / joystick->hwdata->gyro_scale[i];
#ifdef DEBUG_INPUT_EVENTS
SDL_Log("Joystick : Re-read Gyro (axis %d) val= %f\n", i, data[i]);
#endif
}
}
SDL_SendJoystickSensor(timestamp, joystick, SDL_SENSOR_GYRO, SDL_US_TO_NS(joystick->hwdata->sensor_tick), data, 3);
}
if (joystick->hwdata->has_accelerometer) {
float data[3] = {0.0f, 0.0f, 0.0f};
for (i = 0; i < 3; i++) {
if (ioctl(joystick->hwdata->fd_sensor, EVIOCGABS(ABS_X + i), &absinfo) >= 0) {
data[i] = absinfo.value * SDL_STANDARD_GRAVITY / joystick->hwdata->accelerometer_scale[i];
#ifdef DEBUG_INPUT_EVENTS
SDL_Log("Joystick : Re-read Accelerometer (axis %d) val= %f\n", i, data[i]);
#endif
}
}
SDL_SendJoystickSensor(timestamp, joystick, SDL_SENSOR_ACCEL, SDL_US_TO_NS(joystick->hwdata->sensor_tick), data, 3);
}
}

I made sure that as far as Handheld Daemon is concerned, it will only send a syn event after a timestamp has been sent. However, it is out of my hands whether SDL will receive a SYN_DROPPED event.

Perhaps sensor_tick should not be assumed to be unique. Then, this would be a problem for clients to deal with (e.g., Steam), and the function comment should be updated to reflect that. See below:
https://wiki.libsdl.org/SDL3/SDL_GamepadSensorEvent

If sensor_tick should be assumed to be unique, then SDL would have to be updated to ensure it always sends a unique sensor timestamp. For that there could be two potential solutions:

  1. Skip Sending Events with 0 Delta: For Dualsense, this could be done by checking the delta and if it is 0, skipping updating the sensors. For evdev, if a SYN_DROPPED happens, PollAllSensors could be modified to not send an event in favor of waiting for the next event.
    • This would have the minor issue of lowering the polling rate slightly, and breaking non-compliant controllers that do not have a hw timestamp.
  2. Send Synthetic Delta: If a SYN_DROPPED happens, or the Dualsense controller sends a duplicate timestamp, a synthetic delta based on SDL_GetTicksNS could be used.
    • This would maintain the polling rate of the controllers, and restore functionality with devices that do not meet spec (e.g., controller emulators without a hw timestamp). However, it would interfere with motion vectoring (e.g., touchpad acceleration), if the downstream consumer expects that.
    • In fact, when testing using a synthetic timestamp version in earlier versions of Handheld Daemon, I noticed that gyro to camera would experience slight stutters, so perhaps this is not a good solution. I attributed that to inconsistent wake-up times, but it could have been something else.
    • The current version always uses the sensor hardware timestamp, and behavior is excellent, other than the evdev occasional stutter (which is part of and blocks the release of the next version).

Would love to hear your thoughts. If in the end, this is a docs issue and a problem on Steam's side, I'd also appreciate help with forwarding the issue.

Sidenote: as part of Handheld Daemon, I have implemented Linux support for all Windows handhelds in the market right now (see here) including RGB and touchpads in Steam Input and I would be interested in defining a spec for handheld controllers, so we can get away from hacks such as emulating Dualsense or Joycon controllers, which have Glyph issues and non-matching capabilities.

Perhaps starting by making the appropriate extensions to the evdev API, which can be the foundation to a hid API and kernel driver that is implemented in firmware by device manufacturers. Current devices are a hodgepodge of different vendor specific windows devices, so a tool like Handheld Daemon w/ root access is needed to fuse them together in a cohesive manner. This should not be the case with future devices, and the sooner we start the better.

@slouken slouken added this to the 3.2.0 milestone May 4, 2024
@slouken slouken self-assigned this May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants