From 2d32557fdea3e83dd8ec2a73e122338cbe5d417b Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 25 Jan 2024 20:17:37 +0100 Subject: [PATCH] Embed HID event data In the implementation, an HID event is at most 8 bytes. Embed the data in the HID event structure to avoid allocations and simplify the code. PR #4473 --- app/src/usb/aoa_hid.c | 26 ++------------------------ app/src/usb/aoa_hid.h | 14 ++++---------- app/src/usb/hid_keyboard.c | 28 ++++++++-------------------- app/src/usb/hid_mouse.c | 31 +++++++++---------------------- 4 files changed, 23 insertions(+), 76 deletions(-) diff --git a/app/src/usb/aoa_hid.c b/app/src/usb/aoa_hid.c index 1f2f7c79..5db7ab94 100644 --- a/app/src/usb/aoa_hid.c +++ b/app/src/usb/aoa_hid.c @@ -33,20 +33,6 @@ sc_hid_event_log(const struct sc_hid_event *event) { free(buffer); } -void -sc_hid_event_init(struct sc_hid_event *hid_event, uint16_t accessory_id, - unsigned char *data, uint16_t size) { - hid_event->accessory_id = accessory_id; - hid_event->data = data; - hid_event->size = size; - hid_event->ack_to_wait = SC_SEQUENCE_INVALID; -} - -void -sc_hid_event_destroy(struct sc_hid_event *hid_event) { - free(hid_event->data); -} - bool sc_aoa_init(struct sc_aoa *aoa, struct sc_usb *usb, struct sc_acksync *acksync) { @@ -76,12 +62,7 @@ sc_aoa_init(struct sc_aoa *aoa, struct sc_usb *usb, void sc_aoa_destroy(struct sc_aoa *aoa) { - // Destroy remaining events - while (!sc_vecdeque_is_empty(&aoa->queue)) { - struct sc_hid_event *event = sc_vecdeque_popref(&aoa->queue); - assert(event); - sc_hid_event_destroy(event); - } + sc_vecdeque_destroy(&aoa->queue); sc_cond_destroy(&aoa->event_cond); sc_mutex_destroy(&aoa->mutex); @@ -177,7 +158,7 @@ sc_aoa_send_hid_event(struct sc_aoa *aoa, const struct sc_hid_event *event) { // index (arg1): 0 (unused) uint16_t value = event->accessory_id; uint16_t index = 0; - unsigned char *data = event->data; + unsigned char *data = (uint8_t *) event->data; // discard const uint16_t length = event->size; int result = libusb_control_transfer(aoa->usb->handle, request_type, request, value, index, data, length, @@ -271,17 +252,14 @@ run_aoa_thread(void *data) { if (result == SC_ACKSYNC_WAIT_TIMEOUT) { LOGW("Ack not received after 500ms, discarding HID event"); - sc_hid_event_destroy(&event); continue; } else if (result == SC_ACKSYNC_WAIT_INTR) { // stopped - sc_hid_event_destroy(&event); break; } } bool ok = sc_aoa_send_hid_event(aoa, &event); - sc_hid_event_destroy(&event); if (!ok) { LOGW("Could not send HID event to USB device"); } diff --git a/app/src/usb/aoa_hid.h b/app/src/usb/aoa_hid.h index a726938a..2cbd1a23 100644 --- a/app/src/usb/aoa_hid.h +++ b/app/src/usb/aoa_hid.h @@ -12,21 +12,15 @@ #include "util/tick.h" #include "util/vecdeque.h" +#define SC_HID_MAX_SIZE 8 + struct sc_hid_event { uint16_t accessory_id; - unsigned char *data; - uint16_t size; + uint8_t data[SC_HID_MAX_SIZE]; + uint8_t size; uint64_t ack_to_wait; }; -// Takes ownership of buffer -void -sc_hid_event_init(struct sc_hid_event *hid_event, uint16_t accessory_id, - unsigned char *data, uint16_t size); - -void -sc_hid_event_destroy(struct sc_hid_event *hid_event); - struct sc_hid_event_queue SC_VECDEQUE(struct sc_hid_event); struct sc_aoa { diff --git a/app/src/usb/hid_keyboard.c b/app/src/usb/hid_keyboard.c index 8bd0866a..dcf56313 100644 --- a/app/src/usb/hid_keyboard.c +++ b/app/src/usb/hid_keyboard.c @@ -231,21 +231,17 @@ sdl_keymod_to_hid_modifiers(uint16_t mod) { return modifiers; } -static bool +static void sc_hid_keyboard_event_init(struct sc_hid_event *hid_event) { - unsigned char *data = malloc(HID_KEYBOARD_EVENT_SIZE); - if (!data) { - LOG_OOM(); - return false; - } + hid_event->accessory_id = HID_KEYBOARD_ACCESSORY_ID; + hid_event->size = HID_KEYBOARD_EVENT_SIZE; + hid_event->ack_to_wait = SC_SEQUENCE_INVALID; + + uint8_t *data = hid_event->data; data[HID_KEYBOARD_INDEX_MODIFIER] = HID_MODIFIER_NONE; data[1] = HID_RESERVED; memset(&data[HID_KEYBOARD_INDEX_KEYS], 0, HID_KEYBOARD_MAX_KEYS); - - sc_hid_event_init(hid_event, HID_KEYBOARD_ACCESSORY_ID, data, - HID_KEYBOARD_EVENT_SIZE); - return true; } static inline bool @@ -268,10 +264,7 @@ convert_hid_keyboard_event(struct sc_hid_keyboard *kb, return false; } - if (!sc_hid_keyboard_event_init(hid_event)) { - LOGW("Could not initialize HID keyboard event"); - return false; - } + sc_hid_keyboard_event_init(hid_event); unsigned char modifiers = sdl_keymod_to_hid_modifiers(event->mods_state); @@ -324,10 +317,7 @@ push_mod_lock_state(struct sc_hid_keyboard *kb, uint16_t mods_state) { } struct sc_hid_event hid_event; - if (!sc_hid_keyboard_event_init(&hid_event)) { - LOGW("Could not initialize HID keyboard event"); - return false; - } + sc_hid_keyboard_event_init(&hid_event); unsigned i = 0; if (capslock) { @@ -340,7 +330,6 @@ push_mod_lock_state(struct sc_hid_keyboard *kb, uint16_t mods_state) { } if (!sc_aoa_push_hid_event(kb->aoa, &hid_event)) { - sc_hid_event_destroy(&hid_event); LOGW("Could not request HID event (mod lock state)"); return false; } @@ -382,7 +371,6 @@ sc_key_processor_process_key(struct sc_key_processor *kp, } if (!sc_aoa_push_hid_event(kb->aoa, &hid_event)) { - sc_hid_event_destroy(&hid_event); LOGW("Could not request HID event (key)"); } } diff --git a/app/src/usb/hid_mouse.c b/app/src/usb/hid_mouse.c index 45ae6441..a47534c1 100644 --- a/app/src/usb/hid_mouse.c +++ b/app/src/usb/hid_mouse.c @@ -131,17 +131,13 @@ static const unsigned char mouse_report_desc[] = { * +---------------+ */ -static bool +static void sc_hid_mouse_event_init(struct sc_hid_event *hid_event) { - unsigned char *data = calloc(1, HID_MOUSE_EVENT_SIZE); - if (!data) { - LOG_OOM(); - return false; - } - - sc_hid_event_init(hid_event, HID_MOUSE_ACCESSORY_ID, data, - HID_MOUSE_EVENT_SIZE); - return true; + hid_event->accessory_id = HID_MOUSE_ACCESSORY_ID; + hid_event->size = HID_MOUSE_EVENT_SIZE; + hid_event->ack_to_wait = SC_SEQUENCE_INVALID; + // Leave hid_event->data uninitialized, it will be fully initialized by + // callers } static unsigned char @@ -171,9 +167,7 @@ sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp, struct sc_hid_mouse *mouse = DOWNCAST(mp); struct sc_hid_event hid_event; - if (!sc_hid_mouse_event_init(&hid_event)) { - return; - } + sc_hid_mouse_event_init(&hid_event); unsigned char *data = hid_event.data; data[0] = buttons_state_to_hid_buttons(event->buttons_state); @@ -182,7 +176,6 @@ sc_mouse_processor_process_mouse_motion(struct sc_mouse_processor *mp, data[3] = 0; // wheel coordinates only used for scrolling if (!sc_aoa_push_hid_event(mouse->aoa, &hid_event)) { - sc_hid_event_destroy(&hid_event); LOGW("Could not request HID event (mouse motion)"); } } @@ -193,9 +186,7 @@ sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp, struct sc_hid_mouse *mouse = DOWNCAST(mp); struct sc_hid_event hid_event; - if (!sc_hid_mouse_event_init(&hid_event)) { - return; - } + sc_hid_mouse_event_init(&hid_event); unsigned char *data = hid_event.data; data[0] = buttons_state_to_hid_buttons(event->buttons_state); @@ -204,7 +195,6 @@ sc_mouse_processor_process_mouse_click(struct sc_mouse_processor *mp, data[3] = 0; // wheel coordinates only used for scrolling if (!sc_aoa_push_hid_event(mouse->aoa, &hid_event)) { - sc_hid_event_destroy(&hid_event); LOGW("Could not request HID event (mouse click)"); } } @@ -215,9 +205,7 @@ sc_mouse_processor_process_mouse_scroll(struct sc_mouse_processor *mp, struct sc_hid_mouse *mouse = DOWNCAST(mp); struct sc_hid_event hid_event; - if (!sc_hid_mouse_event_init(&hid_event)) { - return; - } + sc_hid_mouse_event_init(&hid_event); unsigned char *data = hid_event.data; data[0] = 0; // buttons state irrelevant (and unknown) @@ -229,7 +217,6 @@ sc_mouse_processor_process_mouse_scroll(struct sc_mouse_processor *mp, // Horizontal scrolling ignored if (!sc_aoa_push_hid_event(mouse->aoa, &hid_event)) { - sc_hid_event_destroy(&hid_event); LOGW("Could not request HID event (mouse scroll)"); } }