Update for bug 4923 - Calling SDL_GameControllerRumble() often takes 8 ms
authorSam Lantinga <slouken@libsdl.org>
Fri, 07 Feb 2020 11:02:34 -0800
changeset 134901d8cb0c5236b
parent 13489 cec913fbe656
child 13491 5de509fd3a96
Update for bug 4923 - Calling SDL_GameControllerRumble() often takes 8 ms

meyraud705

Dualshock4 on bluetooth need 78 bytes for the rumble data while SDL_HIDAPI_RumbleRequest can only hold 64 bytes.

'volatile' is not meant for thread synchronization.

The list of rumble request could grow infinitely if user call SDL_JoystickRumble too much. The documentation says "Each call to this function cancels any previous rumble effect", so overwriting pending request seem like a good idea.
src/joystick/hidapi/SDL_hidapi_rumble.c
src/joystick/hidapi/SDL_hidapi_rumble.h
src/joystick/hidapi/SDL_hidapi_xboxone.c
     1.1 --- a/src/joystick/hidapi/SDL_hidapi_rumble.c	Wed Feb 05 13:16:17 2020 -0500
     1.2 +++ b/src/joystick/hidapi/SDL_hidapi_rumble.c	Fri Feb 07 11:02:34 2020 -0800
     1.3 @@ -34,7 +34,7 @@
     1.4  typedef struct SDL_HIDAPI_RumbleRequest
     1.5  {
     1.6      SDL_HIDAPI_Device *device;
     1.7 -    Uint8 data[USB_PACKET_LENGTH];
     1.8 +    Uint8 data[2*USB_PACKET_LENGTH]; /* need enough space for the biggest report: dualshock4 is 78 bytes */
     1.9      int size;
    1.10      struct SDL_HIDAPI_RumbleRequest *prev;
    1.11  
    1.12 @@ -42,7 +42,8 @@
    1.13  
    1.14  typedef struct SDL_HIDAPI_RumbleContext
    1.15  {
    1.16 -    volatile SDL_bool running;
    1.17 +    SDL_atomic_t initialized;
    1.18 +    SDL_atomic_t running;
    1.19      SDL_Thread *thread;
    1.20      SDL_mutex *lock;
    1.21      SDL_sem *request_sem;
    1.22 @@ -58,7 +59,7 @@
    1.23  
    1.24      SDL_SetThreadPriority(SDL_THREAD_PRIORITY_HIGH);
    1.25  
    1.26 -    while (ctx->running) {
    1.27 +    while (SDL_AtomicGet(&ctx->running)) {
    1.28          SDL_HIDAPI_RumbleRequest *request = NULL;
    1.29  
    1.30          SDL_SemWait(ctx->request_sem);
    1.31 @@ -87,7 +88,7 @@
    1.32  static void
    1.33  SDL_HIDAPI_StopRumbleThread(SDL_HIDAPI_RumbleContext *ctx)
    1.34  {
    1.35 -    ctx->running = SDL_FALSE;
    1.36 +    SDL_AtomicSet(&ctx->running, SDL_FALSE);
    1.37  
    1.38      if (ctx->thread) {
    1.39          int result;
    1.40 @@ -110,6 +111,8 @@
    1.41          SDL_DestroyMutex(ctx->lock);
    1.42          ctx->lock = NULL;
    1.43      }
    1.44 +
    1.45 +    SDL_AtomicSet(&ctx->initialized, SDL_FALSE);
    1.46  }
    1.47  
    1.48  static int
    1.49 @@ -127,7 +130,7 @@
    1.50          return -1;
    1.51      }
    1.52  
    1.53 -    ctx->running = SDL_TRUE;
    1.54 +    SDL_AtomicSet(&ctx->running, SDL_TRUE);
    1.55      ctx->thread = SDL_CreateThreadInternal(SDL_HIDAPI_RumbleThread, "HIDAPI Rumble", 0, ctx);
    1.56      if (!ctx->thread) {
    1.57          SDL_HIDAPI_StopRumbleThread(ctx);
    1.58 @@ -136,23 +139,48 @@
    1.59      return 0;
    1.60  }
    1.61  
    1.62 -int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size)
    1.63 +int SDL_HIDAPI_LockRumble(void)
    1.64 +{
    1.65 +    SDL_HIDAPI_RumbleContext *ctx = &rumble_context;
    1.66 +
    1.67 +    if (SDL_AtomicCAS(&ctx->initialized, SDL_FALSE, SDL_TRUE)) {
    1.68 +        if (SDL_HIDAPI_StartRumbleThread(ctx) < 0) {
    1.69 +            return -1;
    1.70 +        }
    1.71 +    }
    1.72 +
    1.73 +    return SDL_LockMutex(ctx->lock);
    1.74 +}
    1.75 +
    1.76 +SDL_bool SDL_HIDAPI_GetPendingRumbleLocked(SDL_HIDAPI_Device *device, Uint8 **data, int **size, int *maximum_size)
    1.77 +{
    1.78 +    SDL_HIDAPI_RumbleContext *ctx = &rumble_context;
    1.79 +    SDL_HIDAPI_RumbleRequest *request;
    1.80 +
    1.81 +    for (request = ctx->requests_tail; request; request = request->prev) {
    1.82 +        if (request->device == device) {
    1.83 +            *data = request->data;
    1.84 +            *size = &request->size;
    1.85 +            *maximum_size = sizeof(request->data);
    1.86 +            return SDL_TRUE;
    1.87 +        }
    1.88 +    }
    1.89 +    return SDL_FALSE;
    1.90 +}
    1.91 +
    1.92 +int SDL_HIDAPI_SendRumbleAndUnlock(SDL_HIDAPI_Device *device, const Uint8 *data, int size)
    1.93  {
    1.94      SDL_HIDAPI_RumbleContext *ctx = &rumble_context;
    1.95      SDL_HIDAPI_RumbleRequest *request;
    1.96  
    1.97      if (size > sizeof(request->data)) {
    1.98 +        SDL_HIDAPI_UnlockRumble();
    1.99          return SDL_SetError("Couldn't send rumble, size %d is greater than %d", size, (int)sizeof(request->data));
   1.100      }
   1.101  
   1.102 -    if (!ctx->running) {
   1.103 -        if (SDL_HIDAPI_StartRumbleThread(ctx) < 0) {
   1.104 -            return -1;
   1.105 -        }
   1.106 -    }
   1.107 -
   1.108      request = (SDL_HIDAPI_RumbleRequest *)SDL_calloc(1, sizeof(*request));
   1.109      if (!request) {
   1.110 +        SDL_HIDAPI_UnlockRumble();
   1.111          return SDL_OutOfMemory();
   1.112      }
   1.113      request->device = device;
   1.114 @@ -161,25 +189,59 @@
   1.115  
   1.116      SDL_AtomicIncRef(&device->rumble_pending);
   1.117      
   1.118 -    SDL_LockMutex(ctx->lock);
   1.119      if (ctx->requests_head) {
   1.120          ctx->requests_head->prev = request;
   1.121      } else {
   1.122          ctx->requests_tail = request;
   1.123      }
   1.124      ctx->requests_head = request;
   1.125 -    SDL_UnlockMutex(ctx->lock);
   1.126 +
   1.127 +    /* Make sure we unlock before posting the semaphore so the rumble thread can run immediately */
   1.128 +    SDL_HIDAPI_UnlockRumble();
   1.129  
   1.130      SDL_SemPost(ctx->request_sem);
   1.131  
   1.132      return size;
   1.133  }
   1.134  
   1.135 +void SDL_HIDAPI_UnlockRumble(void)
   1.136 +{
   1.137 +    SDL_HIDAPI_RumbleContext *ctx = &rumble_context;
   1.138 +
   1.139 +    SDL_UnlockMutex(ctx->lock);
   1.140 +}
   1.141 +
   1.142 +int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size)
   1.143 +{
   1.144 +    Uint8 *pending_data;
   1.145 +    int *pending_size;
   1.146 +    int maximum_size;
   1.147 +
   1.148 +    if (SDL_HIDAPI_LockRumble() < 0) {
   1.149 +        return -1;
   1.150 +    }
   1.151 +
   1.152 +    /* check if there is a pending request for the device and update it */
   1.153 +    if (SDL_HIDAPI_GetPendingRumbleLocked(device, &pending_data, &pending_size, &maximum_size)) {
   1.154 +        if (size > maximum_size) {
   1.155 +            SDL_HIDAPI_UnlockRumble();
   1.156 +            return SDL_SetError("Couldn't send rumble, size %d is greater than %d", size, maximum_size);
   1.157 +        }
   1.158 +
   1.159 +        SDL_memcpy(pending_data, data, size);
   1.160 +        *pending_size = size;
   1.161 +        SDL_HIDAPI_UnlockRumble();
   1.162 +        return size;
   1.163 +    }
   1.164 +
   1.165 +    return SDL_HIDAPI_SendRumbleAndUnlock(device, data, size);
   1.166 +}
   1.167 +
   1.168  void SDL_HIDAPI_QuitRumble(void)
   1.169  {
   1.170      SDL_HIDAPI_RumbleContext *ctx = &rumble_context;
   1.171  
   1.172 -    if (ctx->running) {
   1.173 +    if (SDL_AtomicGet(&ctx->running)) {
   1.174          SDL_HIDAPI_StopRumbleThread(ctx);
   1.175      }
   1.176  }
     2.1 --- a/src/joystick/hidapi/SDL_hidapi_rumble.h	Wed Feb 05 13:16:17 2020 -0500
     2.2 +++ b/src/joystick/hidapi/SDL_hidapi_rumble.h	Fri Feb 07 11:02:34 2020 -0800
     2.3 @@ -23,6 +23,14 @@
     2.4  #ifdef SDL_JOYSTICK_HIDAPI
     2.5  
     2.6  /* Handle rumble on a separate thread so it doesn't block the application */
     2.7 +
     2.8 +/* Advanced API */
     2.9 +int SDL_HIDAPI_LockRumble(void);
    2.10 +SDL_bool SDL_HIDAPI_GetPendingRumbleLocked(SDL_HIDAPI_Device *device, Uint8 **data, int **size, int *maximum_size);
    2.11 +int SDL_HIDAPI_SendRumbleAndUnlock(SDL_HIDAPI_Device *device, const Uint8 *data, int size);
    2.12 +void SDL_HIDAPI_UnlockRumble(void);
    2.13 +
    2.14 +/* Simple API, will replace any pending rumble with the new data */
    2.15  int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size);
    2.16  void SDL_HIDAPI_QuitRumble(void);
    2.17  
     3.1 --- a/src/joystick/hidapi/SDL_hidapi_xboxone.c	Wed Feb 05 13:16:17 2020 -0500
     3.2 +++ b/src/joystick/hidapi/SDL_hidapi_xboxone.c	Fri Feb 07 11:02:34 2020 -0800
     3.3 @@ -118,6 +118,7 @@
     3.4      Uint8 sequence;
     3.5      Uint8 last_state[USB_PACKET_LENGTH];
     3.6      SDL_bool rumble_synchronized;
     3.7 +    SDL_bool rumble_synchronization_complete;
     3.8      SDL_bool has_paddles;
     3.9  } SDL_DriverXboxOne_Context;
    3.10  
    3.11 @@ -194,7 +195,10 @@
    3.12          SDL_memcpy(init_packet, xboxone_rumble_reset, sizeof(xboxone_rumble_reset));
    3.13          for (i = 0; i < 255; ++i) {
    3.14              init_packet[2] = ((ctx->sequence + i) % 255);
    3.15 -            if (SDL_HIDAPI_SendRumble(device, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) {
    3.16 +            if (SDL_HIDAPI_LockRumble() < 0) {
    3.17 +                return SDL_FALSE;
    3.18 +            }
    3.19 +            if (SDL_HIDAPI_SendRumbleAndUnlock(device, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) {
    3.20                  SDL_SetError("Couldn't write Xbox One initialization packet");
    3.21                  return SDL_FALSE;
    3.22              }
    3.23 @@ -371,16 +375,39 @@
    3.24  {
    3.25      SDL_DriverXboxOne_Context *ctx = (SDL_DriverXboxOne_Context *)device->context;
    3.26      Uint8 rumble_packet[] = { 0x09, 0x00, 0x00, 0x09, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xFF };
    3.27 +    Uint8 *pending_rumble;
    3.28 +    int *pending_size;
    3.29 +    int maximum_size;
    3.30  
    3.31      SynchronizeRumbleSequence(device, ctx);
    3.32  
    3.33 -    /* Magnitude is 1..100 so scale the 16-bit input here */
    3.34 -    rumble_packet[2] = ctx->sequence++;
    3.35 -    rumble_packet[8] = low_frequency_rumble / 655;
    3.36 -    rumble_packet[9] = high_frequency_rumble / 655;
    3.37 +    if (SDL_HIDAPI_LockRumble() < 0) {
    3.38 +        return -1;
    3.39 +    }
    3.40  
    3.41 -    if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    3.42 -        return SDL_SetError("Couldn't send rumble packet");
    3.43 +    if (!ctx->rumble_synchronization_complete) {
    3.44 +        if (!SDL_HIDAPI_GetPendingRumbleLocked(device, &pending_rumble, &pending_size, &maximum_size)) {
    3.45 +            /* Rumble synchronization has drained */
    3.46 +            ctx->rumble_synchronization_complete = SDL_TRUE;
    3.47 +        }
    3.48 +    }
    3.49 +
    3.50 +    /* Try to overwrite any pending rumble with the new value */
    3.51 +    if (ctx->rumble_synchronization_complete && 
    3.52 +        SDL_HIDAPI_GetPendingRumbleLocked(device, &pending_rumble, &pending_size, &maximum_size)) {
    3.53 +        /* Magnitude is 1..100 so scale the 16-bit input here */
    3.54 +        pending_rumble[8] = low_frequency_rumble / 655;
    3.55 +        pending_rumble[9] = high_frequency_rumble / 655;
    3.56 +        SDL_HIDAPI_UnlockRumble();
    3.57 +    } else {
    3.58 +        /* Magnitude is 1..100 so scale the 16-bit input here */
    3.59 +        rumble_packet[2] = ctx->sequence++;
    3.60 +        rumble_packet[8] = low_frequency_rumble / 655;
    3.61 +        rumble_packet[9] = high_frequency_rumble / 655;
    3.62 +
    3.63 +        if (SDL_HIDAPI_SendRumbleAndUnlock(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) {
    3.64 +            return SDL_SetError("Couldn't send rumble packet");
    3.65 +        }
    3.66      }
    3.67      return 0;
    3.68  }