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.
Sam Lantinga 2020-02-04 15:26:56 -08:00
parent 6efebf1768
commit 1684606fdf
7 changed files with 44 additions and 19 deletions

View File

@ -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 @@ HIDAPI_DriverGameCube_UpdateDevice(SDL_HIDAPI_Device *device)
/* 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 @@ HIDAPI_DriverGameCube_CloseJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joy
/* 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;
}
}

View File

@ -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 @@ HIDAPI_DriverPS4_RumbleJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joystic
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;

View File

@ -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 @@ HIDAPI_DriverXbox360_RumbleJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joy
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__ */

View File

@ -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 @@ HIDAPI_DriverXbox360W_RumbleJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *jo
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;

View File

@ -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 @@ ControllerNeedsRumbleSequenceSynchronized(Uint16 vendor_id, Uint16 product_id)
}
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 @@ SynchronizeRumbleSequence(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
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 @@ ControllerSendsWaitingForInit(Uint16 vendor_id, Uint16 product_id)
}
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 @@ SendControllerInit(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
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 @@ SendControllerInit(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
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 @@ SendControllerInit(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
}
}
SynchronizeRumbleSequence(dev, ctx);
SynchronizeRumbleSequence(device, ctx);
return SDL_TRUE;
}
@ -371,14 +372,14 @@ HIDAPI_DriverXboxOne_RumbleJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joy
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 @@ HIDAPI_DriverXboxOne_UpdateDevice(SDL_HIDAPI_Device *device)
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 @@ HIDAPI_DriverXboxOne_UpdateDevice(SDL_HIDAPI_Device *device)
#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;
}

View File

@ -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 @@ HIDAPI_AddDevice(struct hid_device_info *info)
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_DelDevice(SDL_HIDAPI_Device *device)
HIDAPI_CleanupDeviceDriver(device);
SDL_DestroyMutex(device->dev_lock);
SDL_free(device->name);
SDL_free(device->path);
SDL_free(device);
@ -904,7 +907,10 @@ HIDAPI_UpdateDevices(void)
device = SDL_HIDAPI_devices;
while (device) {
if (device->driver) {
if (SDL_TryLockMutex(device->dev_lock) == 0) {
device->driver->UpdateDevice(device);
SDL_UnlockMutex(device->dev_lock);
}
}
device = device->next;
}
@ -1029,6 +1035,11 @@ HIDAPI_JoystickClose(SDL_Joystick * joystick)
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 @@ HIDAPI_JoystickQuit(void)
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);

View File

@ -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 @@ typedef struct _SDL_HIDAPI_Device
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 @@ typedef struct _SDL_HIDAPI_DeviceDriver
} 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;