Fixed long delay on main thread caused by blocking rumble writes in HIDAPI drivers
There is now a thread that handles all HIDAPI rumble requests and a lock that guarantees that we're not reading and writing the device at the same time.
diff --git a/src/joystick/hidapi/SDL_hidapi_gamecube.c b/src/joystick/hidapi/SDL_hidapi_gamecube.c
index 6634427..4800141 100644
--- a/src/joystick/hidapi/SDL_hidapi_gamecube.c
+++ b/src/joystick/hidapi/SDL_hidapi_gamecube.c
@@ -31,6 +31,7 @@
#include "SDL_gamecontroller.h"
#include "../SDL_sysjoystick.h"
#include "SDL_hidapijoystick_c.h"
+#include "SDL_hidapi_rumble.h"
#ifdef SDL_JOYSTICK_HIDAPI_GAMECUBE
@@ -285,7 +286,7 @@
/* Write rumble packet */
if (ctx->rumbleUpdate) {
- hid_write(device->dev, ctx->rumble, sizeof(ctx->rumble));
+ SDL_HIDAPI_SendRumble(device, ctx->rumble, sizeof(ctx->rumble));
ctx->rumbleUpdate = SDL_FALSE;
}
@@ -343,7 +344,7 @@
/* Stop rumble activity */
if (ctx->rumbleUpdate) {
- hid_write(device->dev, ctx->rumble, sizeof(ctx->rumble));
+ SDL_HIDAPI_SendRumble(device, ctx->rumble, sizeof(ctx->rumble));
ctx->rumbleUpdate = SDL_FALSE;
}
}
diff --git a/src/joystick/hidapi/SDL_hidapi_ps4.c b/src/joystick/hidapi/SDL_hidapi_ps4.c
index d79121a..c2e590a 100644
--- a/src/joystick/hidapi/SDL_hidapi_ps4.c
+++ b/src/joystick/hidapi/SDL_hidapi_ps4.c
@@ -33,6 +33,7 @@
#include "SDL_gamecontroller.h"
#include "../SDL_sysjoystick.h"
#include "SDL_hidapijoystick_c.h"
+#include "SDL_hidapi_rumble.h"
#ifdef SDL_JOYSTICK_HIDAPI_PS4
@@ -305,7 +306,7 @@
SDL_memcpy(&data[report_size - sizeof(unCRC)], &unCRC, sizeof(unCRC));
}
- if (hid_write(device->dev, data, report_size) != report_size) {
+ if (SDL_HIDAPI_SendRumble(device, data, report_size) != report_size) {
return SDL_SetError("Couldn't send rumble packet");
}
return 0;
diff --git a/src/joystick/hidapi/SDL_hidapi_xbox360.c b/src/joystick/hidapi/SDL_hidapi_xbox360.c
index 271af53..bc6fe95 100644
--- a/src/joystick/hidapi/SDL_hidapi_xbox360.c
+++ b/src/joystick/hidapi/SDL_hidapi_xbox360.c
@@ -30,6 +30,7 @@
#include "SDL_gamecontroller.h"
#include "../SDL_sysjoystick.h"
#include "SDL_hidapijoystick_c.h"
+#include "SDL_hidapi_rumble.h"
#ifdef SDL_JOYSTICK_HIDAPI_XBOX360
@@ -416,7 +417,7 @@
rumble_packet[4] = (high_frequency_rumble >> 8);
#endif
- if (hid_write(device->dev, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
+ if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
return SDL_SetError("Couldn't send rumble packet");
}
#endif /* __WIN32__ */
diff --git a/src/joystick/hidapi/SDL_hidapi_xbox360w.c b/src/joystick/hidapi/SDL_hidapi_xbox360w.c
index f3550f3..af22867 100644
--- a/src/joystick/hidapi/SDL_hidapi_xbox360w.c
+++ b/src/joystick/hidapi/SDL_hidapi_xbox360w.c
@@ -30,6 +30,7 @@
#include "SDL_gamecontroller.h"
#include "../SDL_sysjoystick.h"
#include "SDL_hidapijoystick_c.h"
+#include "SDL_hidapi_rumble.h"
#ifdef SDL_JOYSTICK_HIDAPI_XBOX360
@@ -151,7 +152,7 @@
rumble_packet[5] = (low_frequency_rumble >> 8);
rumble_packet[6] = (high_frequency_rumble >> 8);
- if (hid_write(device->dev, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
+ if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
return SDL_SetError("Couldn't send rumble packet");
}
return 0;
diff --git a/src/joystick/hidapi/SDL_hidapi_xboxone.c b/src/joystick/hidapi/SDL_hidapi_xboxone.c
index 4fab0af..d964351 100644
--- a/src/joystick/hidapi/SDL_hidapi_xboxone.c
+++ b/src/joystick/hidapi/SDL_hidapi_xboxone.c
@@ -30,6 +30,7 @@
#include "SDL_gamecontroller.h"
#include "../SDL_sysjoystick.h"
#include "SDL_hidapijoystick_c.h"
+#include "SDL_hidapi_rumble.h"
#ifdef SDL_JOYSTICK_HIDAPI_XBOXONE
@@ -177,7 +178,7 @@
}
static SDL_bool
-SynchronizeRumbleSequence(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
+SynchronizeRumbleSequence(SDL_HIDAPI_Device *device, SDL_DriverXboxOne_Context *ctx)
{
Uint16 vendor_id = ctx->vendor_id;
Uint16 product_id = ctx->product_id;
@@ -193,7 +194,7 @@
SDL_memcpy(init_packet, xboxone_rumble_reset, sizeof(xboxone_rumble_reset));
for (i = 0; i < 255; ++i) {
init_packet[2] = ((ctx->sequence + i) % 255);
- if (hid_write(dev, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) {
+ if (SDL_HIDAPI_SendRumble(device, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) {
SDL_SetError("Couldn't write Xbox One initialization packet");
return SDL_FALSE;
}
@@ -226,7 +227,7 @@
}
static SDL_bool
-SendControllerInit(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
+SendControllerInit(SDL_HIDAPI_Device *device, SDL_DriverXboxOne_Context *ctx)
{
Uint16 vendor_id = ctx->vendor_id;
Uint16 product_id = ctx->product_id;
@@ -258,7 +259,7 @@
if (init_packet[0] != 0x01) {
init_packet[2] = ctx->sequence++;
}
- if (hid_write(dev, init_packet, packet->size) != packet->size) {
+ if (hid_write(device->dev, init_packet, packet->size) != packet->size) {
SDL_SetError("Couldn't write Xbox One initialization packet");
return SDL_FALSE;
}
@@ -272,7 +273,7 @@
Uint8 data[USB_PACKET_LENGTH];
int size;
- while ((size = hid_read_timeout(dev, data, sizeof(data), 0)) > 0) {
+ while ((size = hid_read_timeout(device->dev, data, sizeof(data), 0)) > 0) {
#ifdef DEBUG_XBOX_PROTOCOL
DumpPacket("Xbox One INIT packet: size = %d", data, size);
#endif
@@ -288,7 +289,7 @@
}
}
- SynchronizeRumbleSequence(dev, ctx);
+ SynchronizeRumbleSequence(device, ctx);
return SDL_TRUE;
}
@@ -371,14 +372,14 @@
SDL_DriverXboxOne_Context *ctx = (SDL_DriverXboxOne_Context *)device->context;
Uint8 rumble_packet[] = { 0x09, 0x00, 0x00, 0x09, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xFF };
- SynchronizeRumbleSequence(device->dev, ctx);
+ SynchronizeRumbleSequence(device, ctx);
/* Magnitude is 1..100 so scale the 16-bit input here */
rumble_packet[2] = ctx->sequence++;
rumble_packet[8] = low_frequency_rumble / 655;
rumble_packet[9] = high_frequency_rumble / 655;
- if (hid_write(device->dev, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
+ if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
return SDL_SetError("Couldn't send rumble packet");
}
return 0;
@@ -523,7 +524,7 @@
if (!ctx->initialized &&
!ControllerSendsWaitingForInit(device->vendor_id, device->product_id)) {
if (SDL_TICKS_PASSED(SDL_GetTicks(), ctx->start_time + CONTROLLER_INIT_DELAY_MS)) {
- if (!SendControllerInit(device->dev, ctx)) {
+ if (!SendControllerInit(device, ctx)) {
HIDAPI_JoystickDisconnected(device, joystick->instance_id);
return SDL_FALSE;
}
@@ -542,7 +543,7 @@
#ifdef DEBUG_XBOX_PROTOCOL
SDL_Log("Delay after init: %ums\n", SDL_GetTicks() - ctx->start_time);
#endif
- if (!SendControllerInit(device->dev, ctx)) {
+ if (!SendControllerInit(device, ctx)) {
HIDAPI_JoystickDisconnected(device, joystick->instance_id);
return SDL_FALSE;
}
diff --git a/src/joystick/hidapi/SDL_hidapijoystick.c b/src/joystick/hidapi/SDL_hidapijoystick.c
index 1da537b..a2e91a3 100644
--- a/src/joystick/hidapi/SDL_hidapijoystick.c
+++ b/src/joystick/hidapi/SDL_hidapijoystick.c
@@ -32,6 +32,7 @@
#include "SDL_joystick.h"
#include "../SDL_sysjoystick.h"
#include "SDL_hidapijoystick_c.h"
+#include "SDL_hidapi_rumble.h"
#include "../../SDL_hints_c.h"
#if defined(__WIN32__)
@@ -682,6 +683,7 @@
device->guid.data[14] = 'h';
device->guid.data[15] = 0;
}
+ device->dev_lock = SDL_CreateMutex();
/* Need the device name before getting the driver to know whether to ignore this device */
if (!device->name) {
@@ -768,6 +770,7 @@
HIDAPI_CleanupDeviceDriver(device);
+ SDL_DestroyMutex(device->dev_lock);
SDL_free(device->name);
SDL_free(device->path);
SDL_free(device);
@@ -904,7 +907,10 @@
device = SDL_HIDAPI_devices;
while (device) {
if (device->driver) {
- device->driver->UpdateDevice(device);
+ if (SDL_TryLockMutex(device->dev_lock) == 0) {
+ device->driver->UpdateDevice(device);
+ SDL_UnlockMutex(device->dev_lock);
+ }
}
device = device->next;
}
@@ -1029,6 +1035,11 @@
if (joystick->hwdata) {
SDL_HIDAPI_Device *device = joystick->hwdata->device;
+ /* Wait for pending rumble to complete */
+ while (SDL_AtomicGet(&device->rumble_pending) > 0) {
+ SDL_Delay(10);
+ }
+
device->driver->CloseJoystick(device, joystick);
SDL_free(joystick->hwdata);
@@ -1048,6 +1059,9 @@
while (SDL_HIDAPI_devices) {
HIDAPI_DelDevice(SDL_HIDAPI_devices);
}
+
+ SDL_HIDAPI_QuitRumble();
+
for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) {
SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i];
SDL_DelHintCallback(driver->hint, SDL_HIDAPIDriverHintChanged, NULL);
diff --git a/src/joystick/hidapi/SDL_hidapijoystick_c.h b/src/joystick/hidapi/SDL_hidapijoystick_c.h
index ae0326c..562e8fa 100644
--- a/src/joystick/hidapi/SDL_hidapijoystick_c.h
+++ b/src/joystick/hidapi/SDL_hidapijoystick_c.h
@@ -23,6 +23,10 @@
#ifndef SDL_JOYSTICK_HIDAPI_H
#define SDL_JOYSTICK_HIDAPI_H
+#include "SDL_atomic.h"
+#include "SDL_mutex.h"
+#include "SDL_joystick.h"
+#include "SDL_gamecontroller.h"
#include "../../hidapi/hidapi/hidapi.h"
#include "../usb_ids.h"
@@ -50,6 +54,9 @@
#define SDL_JOYSTICK_HIDAPI_STEAM
#endif
+/* The maximum size of a USB packet for HID devices */
+#define USB_PACKET_LENGTH 64
+
/* Forward declaration */
struct _SDL_HIDAPI_DeviceDriver;
@@ -70,7 +77,9 @@
struct _SDL_HIDAPI_DeviceDriver *driver;
void *context;
+ SDL_mutex *dev_lock;
hid_device *dev;
+ SDL_atomic_t rumble_pending;
int num_joysticks;
SDL_JoystickID *joysticks;
@@ -98,9 +107,6 @@
} SDL_HIDAPI_DeviceDriver;
-/* The maximum size of a USB packet for HID devices */
-#define USB_PACKET_LENGTH 64
-
/* HIDAPI device support */
extern SDL_HIDAPI_DeviceDriver SDL_HIDAPI_DriverPS4;
extern SDL_HIDAPI_DeviceDriver SDL_HIDAPI_DriverSteam;
