Fixed long delay on main thread caused by blocking rumble writes in HIDAPI drivers
authorSam Lantinga <slouken@libsdl.org>
Tue, 04 Feb 2020 15:26:56 -0800
changeset 134812f70560af6d4
parent 13480 6a144eb5e1f1
child 13482 19e0bdbce46d
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.
src/joystick/hidapi/SDL_hidapi_gamecube.c
src/joystick/hidapi/SDL_hidapi_ps4.c
src/joystick/hidapi/SDL_hidapi_xbox360.c
src/joystick/hidapi/SDL_hidapi_xbox360w.c
src/joystick/hidapi/SDL_hidapi_xboxone.c
src/joystick/hidapi/SDL_hidapijoystick.c
src/joystick/hidapi/SDL_hidapijoystick_c.h
     1.1 --- a/src/joystick/hidapi/SDL_hidapi_gamecube.c	Tue Feb 04 12:48:53 2020 -0800
     1.2 +++ b/src/joystick/hidapi/SDL_hidapi_gamecube.c	Tue Feb 04 15:26:56 2020 -0800
     1.3 @@ -31,6 +31,7 @@
     1.4  #include "SDL_gamecontroller.h"
     1.5  #include "../SDL_sysjoystick.h"
     1.6  #include "SDL_hidapijoystick_c.h"
     1.7 +#include "SDL_hidapi_rumble.h"
     1.8  
     1.9  
    1.10  #ifdef SDL_JOYSTICK_HIDAPI_GAMECUBE
    1.11 @@ -285,7 +286,7 @@
    1.12  
    1.13      /* Write rumble packet */
    1.14      if (ctx->rumbleUpdate) {
    1.15 -        hid_write(device->dev, ctx->rumble, sizeof(ctx->rumble));
    1.16 +        SDL_HIDAPI_SendRumble(device, ctx->rumble, sizeof(ctx->rumble));
    1.17          ctx->rumbleUpdate = SDL_FALSE;
    1.18      }
    1.19  
    1.20 @@ -343,7 +344,7 @@
    1.21  
    1.22      /* Stop rumble activity */
    1.23      if (ctx->rumbleUpdate) {
    1.24 -        hid_write(device->dev, ctx->rumble, sizeof(ctx->rumble));
    1.25 +        SDL_HIDAPI_SendRumble(device, ctx->rumble, sizeof(ctx->rumble));
    1.26          ctx->rumbleUpdate = SDL_FALSE;
    1.27      }
    1.28  }
     2.1 --- a/src/joystick/hidapi/SDL_hidapi_ps4.c	Tue Feb 04 12:48:53 2020 -0800
     2.2 +++ b/src/joystick/hidapi/SDL_hidapi_ps4.c	Tue Feb 04 15:26:56 2020 -0800
     2.3 @@ -33,6 +33,7 @@
     2.4  #include "SDL_gamecontroller.h"
     2.5  #include "../SDL_sysjoystick.h"
     2.6  #include "SDL_hidapijoystick_c.h"
     2.7 +#include "SDL_hidapi_rumble.h"
     2.8  
     2.9  
    2.10  #ifdef SDL_JOYSTICK_HIDAPI_PS4
    2.11 @@ -305,7 +306,7 @@
    2.12          SDL_memcpy(&data[report_size - sizeof(unCRC)], &unCRC, sizeof(unCRC));
    2.13      }
    2.14  
    2.15 -    if (hid_write(device->dev, data, report_size) != report_size) {
    2.16 +    if (SDL_HIDAPI_SendRumble(device, data, report_size) != report_size) {
    2.17          return SDL_SetError("Couldn't send rumble packet");
    2.18      }
    2.19      return 0;
     3.1 --- a/src/joystick/hidapi/SDL_hidapi_xbox360.c	Tue Feb 04 12:48:53 2020 -0800
     3.2 +++ b/src/joystick/hidapi/SDL_hidapi_xbox360.c	Tue Feb 04 15:26:56 2020 -0800
     3.3 @@ -30,6 +30,7 @@
     3.4  #include "SDL_gamecontroller.h"
     3.5  #include "../SDL_sysjoystick.h"
     3.6  #include "SDL_hidapijoystick_c.h"
     3.7 +#include "SDL_hidapi_rumble.h"
     3.8  
     3.9  
    3.10  #ifdef SDL_JOYSTICK_HIDAPI_XBOX360
    3.11 @@ -416,7 +417,7 @@
    3.12      rumble_packet[4] = (high_frequency_rumble >> 8);
    3.13  #endif
    3.14  
    3.15 -    if (hid_write(device->dev, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    3.16 +    if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    3.17          return SDL_SetError("Couldn't send rumble packet");
    3.18      }
    3.19  #endif /* __WIN32__ */
     4.1 --- a/src/joystick/hidapi/SDL_hidapi_xbox360w.c	Tue Feb 04 12:48:53 2020 -0800
     4.2 +++ b/src/joystick/hidapi/SDL_hidapi_xbox360w.c	Tue Feb 04 15:26:56 2020 -0800
     4.3 @@ -30,6 +30,7 @@
     4.4  #include "SDL_gamecontroller.h"
     4.5  #include "../SDL_sysjoystick.h"
     4.6  #include "SDL_hidapijoystick_c.h"
     4.7 +#include "SDL_hidapi_rumble.h"
     4.8  
     4.9  
    4.10  #ifdef SDL_JOYSTICK_HIDAPI_XBOX360
    4.11 @@ -151,7 +152,7 @@
    4.12      rumble_packet[5] = (low_frequency_rumble >> 8);
    4.13      rumble_packet[6] = (high_frequency_rumble >> 8);
    4.14  
    4.15 -    if (hid_write(device->dev, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    4.16 +    if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    4.17          return SDL_SetError("Couldn't send rumble packet");
    4.18      }
    4.19      return 0;
     5.1 --- a/src/joystick/hidapi/SDL_hidapi_xboxone.c	Tue Feb 04 12:48:53 2020 -0800
     5.2 +++ b/src/joystick/hidapi/SDL_hidapi_xboxone.c	Tue Feb 04 15:26:56 2020 -0800
     5.3 @@ -30,6 +30,7 @@
     5.4  #include "SDL_gamecontroller.h"
     5.5  #include "../SDL_sysjoystick.h"
     5.6  #include "SDL_hidapijoystick_c.h"
     5.7 +#include "SDL_hidapi_rumble.h"
     5.8  
     5.9  
    5.10  #ifdef SDL_JOYSTICK_HIDAPI_XBOXONE
    5.11 @@ -177,7 +178,7 @@
    5.12  }
    5.13  
    5.14  static SDL_bool
    5.15 -SynchronizeRumbleSequence(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
    5.16 +SynchronizeRumbleSequence(SDL_HIDAPI_Device *device, SDL_DriverXboxOne_Context *ctx)
    5.17  {
    5.18      Uint16 vendor_id = ctx->vendor_id;
    5.19      Uint16 product_id = ctx->product_id;
    5.20 @@ -193,7 +194,7 @@
    5.21          SDL_memcpy(init_packet, xboxone_rumble_reset, sizeof(xboxone_rumble_reset));
    5.22          for (i = 0; i < 255; ++i) {
    5.23              init_packet[2] = ((ctx->sequence + i) % 255);
    5.24 -            if (hid_write(dev, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) {
    5.25 +            if (SDL_HIDAPI_SendRumble(device, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) {
    5.26                  SDL_SetError("Couldn't write Xbox One initialization packet");
    5.27                  return SDL_FALSE;
    5.28              }
    5.29 @@ -226,7 +227,7 @@
    5.30  }
    5.31  
    5.32  static SDL_bool
    5.33 -SendControllerInit(hid_device *dev, SDL_DriverXboxOne_Context *ctx)
    5.34 +SendControllerInit(SDL_HIDAPI_Device *device, SDL_DriverXboxOne_Context *ctx)
    5.35  {
    5.36      Uint16 vendor_id = ctx->vendor_id;
    5.37      Uint16 product_id = ctx->product_id;
    5.38 @@ -258,7 +259,7 @@
    5.39              if (init_packet[0] != 0x01) {
    5.40                  init_packet[2] = ctx->sequence++;
    5.41              }
    5.42 -            if (hid_write(dev, init_packet, packet->size) != packet->size) {
    5.43 +            if (hid_write(device->dev, init_packet, packet->size) != packet->size) {
    5.44                  SDL_SetError("Couldn't write Xbox One initialization packet");
    5.45                  return SDL_FALSE;
    5.46              }
    5.47 @@ -272,7 +273,7 @@
    5.48                      Uint8 data[USB_PACKET_LENGTH];
    5.49                      int size;
    5.50  
    5.51 -                    while ((size = hid_read_timeout(dev, data, sizeof(data), 0)) > 0) {
    5.52 +                    while ((size = hid_read_timeout(device->dev, data, sizeof(data), 0)) > 0) {
    5.53  #ifdef DEBUG_XBOX_PROTOCOL
    5.54                          DumpPacket("Xbox One INIT packet: size = %d", data, size);
    5.55  #endif
    5.56 @@ -288,7 +289,7 @@
    5.57          }
    5.58      }
    5.59  
    5.60 -    SynchronizeRumbleSequence(dev, ctx);
    5.61 +    SynchronizeRumbleSequence(device, ctx);
    5.62  
    5.63      return SDL_TRUE;
    5.64  }
    5.65 @@ -371,14 +372,14 @@
    5.66      SDL_DriverXboxOne_Context *ctx = (SDL_DriverXboxOne_Context *)device->context;
    5.67      Uint8 rumble_packet[] = { 0x09, 0x00, 0x00, 0x09, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xFF };
    5.68  
    5.69 -    SynchronizeRumbleSequence(device->dev, ctx);
    5.70 +    SynchronizeRumbleSequence(device, ctx);
    5.71  
    5.72      /* Magnitude is 1..100 so scale the 16-bit input here */
    5.73      rumble_packet[2] = ctx->sequence++;
    5.74      rumble_packet[8] = low_frequency_rumble / 655;
    5.75      rumble_packet[9] = high_frequency_rumble / 655;
    5.76  
    5.77 -    if (hid_write(device->dev, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    5.78 +    if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    5.79          return SDL_SetError("Couldn't send rumble packet");
    5.80      }
    5.81      return 0;
    5.82 @@ -523,7 +524,7 @@
    5.83      if (!ctx->initialized &&
    5.84          !ControllerSendsWaitingForInit(device->vendor_id, device->product_id)) {
    5.85          if (SDL_TICKS_PASSED(SDL_GetTicks(), ctx->start_time + CONTROLLER_INIT_DELAY_MS)) {
    5.86 -            if (!SendControllerInit(device->dev, ctx)) {
    5.87 +            if (!SendControllerInit(device, ctx)) {
    5.88                  HIDAPI_JoystickDisconnected(device, joystick->instance_id);
    5.89                  return SDL_FALSE;
    5.90              }
    5.91 @@ -542,7 +543,7 @@
    5.92  #ifdef DEBUG_XBOX_PROTOCOL
    5.93                  SDL_Log("Delay after init: %ums\n", SDL_GetTicks() - ctx->start_time);
    5.94  #endif
    5.95 -                if (!SendControllerInit(device->dev, ctx)) {
    5.96 +                if (!SendControllerInit(device, ctx)) {
    5.97                      HIDAPI_JoystickDisconnected(device, joystick->instance_id);
    5.98                      return SDL_FALSE;
    5.99                  }
     6.1 --- a/src/joystick/hidapi/SDL_hidapijoystick.c	Tue Feb 04 12:48:53 2020 -0800
     6.2 +++ b/src/joystick/hidapi/SDL_hidapijoystick.c	Tue Feb 04 15:26:56 2020 -0800
     6.3 @@ -32,6 +32,7 @@
     6.4  #include "SDL_joystick.h"
     6.5  #include "../SDL_sysjoystick.h"
     6.6  #include "SDL_hidapijoystick_c.h"
     6.7 +#include "SDL_hidapi_rumble.h"
     6.8  #include "../../SDL_hints_c.h"
     6.9  
    6.10  #if defined(__WIN32__)
    6.11 @@ -682,6 +683,7 @@
    6.12          device->guid.data[14] = 'h';
    6.13          device->guid.data[15] = 0;
    6.14      }
    6.15 +    device->dev_lock = SDL_CreateMutex();
    6.16  
    6.17      /* Need the device name before getting the driver to know whether to ignore this device */
    6.18      if (!device->name) {
    6.19 @@ -768,6 +770,7 @@
    6.20  
    6.21              HIDAPI_CleanupDeviceDriver(device);
    6.22  
    6.23 +            SDL_DestroyMutex(device->dev_lock);
    6.24              SDL_free(device->name);
    6.25              SDL_free(device->path);
    6.26              SDL_free(device);
    6.27 @@ -904,7 +907,10 @@
    6.28          device = SDL_HIDAPI_devices;
    6.29          while (device) {
    6.30              if (device->driver) {
    6.31 -                device->driver->UpdateDevice(device);
    6.32 +                if (SDL_TryLockMutex(device->dev_lock) == 0) {
    6.33 +                    device->driver->UpdateDevice(device);
    6.34 +                    SDL_UnlockMutex(device->dev_lock);
    6.35 +                }
    6.36              }
    6.37              device = device->next;
    6.38          }
    6.39 @@ -1029,6 +1035,11 @@
    6.40      if (joystick->hwdata) {
    6.41          SDL_HIDAPI_Device *device = joystick->hwdata->device;
    6.42  
    6.43 +        /* Wait for pending rumble to complete */
    6.44 +        while (SDL_AtomicGet(&device->rumble_pending) > 0) {
    6.45 +            SDL_Delay(10);
    6.46 +        }
    6.47 +
    6.48          device->driver->CloseJoystick(device, joystick);
    6.49  
    6.50          SDL_free(joystick->hwdata);
    6.51 @@ -1048,6 +1059,9 @@
    6.52      while (SDL_HIDAPI_devices) {
    6.53          HIDAPI_DelDevice(SDL_HIDAPI_devices);
    6.54      }
    6.55 +
    6.56 +    SDL_HIDAPI_QuitRumble();
    6.57 +
    6.58      for (i = 0; i < SDL_arraysize(SDL_HIDAPI_drivers); ++i) {
    6.59          SDL_HIDAPI_DeviceDriver *driver = SDL_HIDAPI_drivers[i];
    6.60          SDL_DelHintCallback(driver->hint, SDL_HIDAPIDriverHintChanged, NULL);
     7.1 --- a/src/joystick/hidapi/SDL_hidapijoystick_c.h	Tue Feb 04 12:48:53 2020 -0800
     7.2 +++ b/src/joystick/hidapi/SDL_hidapijoystick_c.h	Tue Feb 04 15:26:56 2020 -0800
     7.3 @@ -23,6 +23,10 @@
     7.4  #ifndef SDL_JOYSTICK_HIDAPI_H
     7.5  #define SDL_JOYSTICK_HIDAPI_H
     7.6  
     7.7 +#include "SDL_atomic.h"
     7.8 +#include "SDL_mutex.h"
     7.9 +#include "SDL_joystick.h"
    7.10 +#include "SDL_gamecontroller.h"
    7.11  #include "../../hidapi/hidapi/hidapi.h"
    7.12  #include "../usb_ids.h"
    7.13  
    7.14 @@ -50,6 +54,9 @@
    7.15  #define SDL_JOYSTICK_HIDAPI_STEAM
    7.16  #endif
    7.17  
    7.18 +/* The maximum size of a USB packet for HID devices */
    7.19 +#define USB_PACKET_LENGTH   64
    7.20 +
    7.21  /* Forward declaration */
    7.22  struct _SDL_HIDAPI_DeviceDriver;
    7.23  
    7.24 @@ -70,7 +77,9 @@
    7.25  
    7.26      struct _SDL_HIDAPI_DeviceDriver *driver;
    7.27      void *context;
    7.28 +    SDL_mutex *dev_lock;
    7.29      hid_device *dev;
    7.30 +    SDL_atomic_t rumble_pending;
    7.31      int num_joysticks;
    7.32      SDL_JoystickID *joysticks;
    7.33  
    7.34 @@ -98,9 +107,6 @@
    7.35  } SDL_HIDAPI_DeviceDriver;
    7.36  
    7.37  
    7.38 -/* The maximum size of a USB packet for HID devices */
    7.39 -#define USB_PACKET_LENGTH   64
    7.40 -
    7.41  /* HIDAPI device support */
    7.42  extern SDL_HIDAPI_DeviceDriver SDL_HIDAPI_DriverPS4;
    7.43  extern SDL_HIDAPI_DeviceDriver SDL_HIDAPI_DriverSteam;