Wait SET_CLIPBOARD ack before Ctrl+v via HID

app/meson.build

@@ -25,6 +25,7 @@ src = [
     'src/server.c',
     'src/stream.c',
     'src/video_buffer.c',
+    'src/util/acksync.c',
     'src/util/file.c',
     'src/util/intr.c',
     'src/util/log.c',

app/src/aoa_hid.c

@@ -35,7 +35,7 @@ sc_hid_event_init(struct sc_hid_event *hid_event, uint16_t accessory_id,
     hid_event->accessory_id = accessory_id;
     hid_event->buffer = buffer;
     hid_event->size = buffer_size;
-    hid_event->delay = 0;
+    hid_event->ack_to_wait = SC_SEQUENCE_INVALID;
 }
 
 void
@@ -118,7 +118,10 @@ sc_aoa_open_usb_handle(libusb_device *device, libusb_device_handle **handle) {
 }
 
 bool
-sc_aoa_init(struct sc_aoa *aoa, const char *serial) {
+sc_aoa_init(struct sc_aoa *aoa, const char *serial,
+            struct sc_acksync *acksync) {
+    assert(acksync);
+
     cbuf_init(&aoa->queue);
 
     if (!sc_mutex_init(&aoa->mutex)) {
@@ -155,6 +158,7 @@ sc_aoa_init(struct sc_aoa *aoa, const char *serial) {
     }
 
     aoa->stopped = false;
+    aoa->acksync = acksync;
 
     return true;
 }
@@ -332,23 +336,28 @@ run_aoa_thread(void *data) {
         assert(non_empty);
         (void) non_empty;
 
-        assert(event.delay >= 0);
-        if (event.delay) {
-            // Wait during the specified delay before injecting the HID event
-            sc_tick deadline = sc_tick_now() + event.delay;
-            bool timed_out = false;
-            while (!aoa->stopped && !timed_out) {
-                timed_out = !sc_cond_timedwait(&aoa->event_cond, &aoa->mutex,
-                                               deadline);
-            }
-            if (aoa->stopped) {
-                sc_mutex_unlock(&aoa->mutex);
+        uint64_t ack_to_wait = event.ack_to_wait;
+        sc_mutex_unlock(&aoa->mutex);
+
+        if (ack_to_wait != SC_SEQUENCE_INVALID) {
+            LOGD("Waiting ack from server sequence=%" PRIu64_, ack_to_wait);
+            // Do not block the loop indefinitely if the ack never comes (it should
+            // never happen)
+            sc_tick deadline = sc_tick_now() + SC_TICK_FROM_MS(500);
+            enum sc_acksync_wait_result result =
+                sc_acksync_wait(aoa->acksync, ack_to_wait, deadline);
+
+            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;
             }
         }
 
-        sc_mutex_unlock(&aoa->mutex);
-
         bool ok = sc_aoa_send_hid_event(aoa, &event);
         sc_hid_event_destroy(&event);
         if (!ok) {
@@ -377,6 +386,8 @@ sc_aoa_stop(struct sc_aoa *aoa) {
     aoa->stopped = true;
     sc_cond_signal(&aoa->event_cond);
     sc_mutex_unlock(&aoa->mutex);
+
+    sc_acksync_interrupt(aoa->acksync);
 }
 
 void

app/src/aoa_hid.h

@@ -6,6 +6,7 @@
 
 #include <libusb-1.0/libusb.h>
 
+#include "util/acksync.h"
 #include "util/cbuf.h"
 #include "util/thread.h"
 #include "util/tick.h"
@@ -14,7 +15,7 @@ struct sc_hid_event {
     uint16_t accessory_id;
     unsigned char *buffer;
     uint16_t size;
-    sc_tick delay;
+    uint64_t ack_to_wait;
 };
 
 // Takes ownership of buffer
@@ -36,10 +37,12 @@ struct sc_aoa {
     sc_cond event_cond;
     bool stopped;
     struct sc_hid_event_queue queue;
+
+    struct sc_acksync *acksync;
 };
 
 bool
-sc_aoa_init(struct sc_aoa *aoa, const char *serial);
+sc_aoa_init(struct sc_aoa *aoa, const char *serial, struct sc_acksync *acksync);
 
 void
 sc_aoa_destroy(struct sc_aoa *aoa);

app/src/compat.h

@@ -6,6 +6,12 @@
 #include <libavformat/version.h>
 #include <SDL2/SDL_version.h>
 
+#ifndef __WIN32
+# define PRIu64_ PRIu64
+#else
+# define PRIu64_ "I64u"  // Windows...
+#endif
+
 // In ffmpeg/doc/APIchanges:
 // 2018-02-06 - 0694d87024 - lavf 58.9.100 - avformat.h
 //   Deprecate use of av_register_input_format(), av_register_output_format(),

app/src/control_msg.c

@@ -118,11 +118,12 @@ control_msg_serialize(const struct control_msg *msg, unsigned char *buf) {
             buf[1] = msg->inject_keycode.action;
             return 2;
         case CONTROL_MSG_TYPE_SET_CLIPBOARD: {
-            buf[1] = !!msg->set_clipboard.paste;
+            buffer_write64be(&buf[1], msg->set_clipboard.sequence);
+            buf[9] = !!msg->set_clipboard.paste;
             size_t len = write_string(msg->set_clipboard.text,
                                       CONTROL_MSG_CLIPBOARD_TEXT_MAX_LENGTH,
-                                      &buf[2]);
-            return 2 + len;
+                                      &buf[10]);
+            return 10 + len;
         }
         case CONTROL_MSG_TYPE_SET_SCREEN_POWER_MODE:
             buf[1] = msg->set_screen_power_mode.mode;
@@ -170,11 +171,6 @@ control_msg_log(const struct control_msg *msg) {
                          (long) msg->inject_touch_event.buttons);
             } else {
                 // numeric pointer id
-#ifndef __WIN32
-# define PRIu64_ PRIu64
-#else
-# define PRIu64_ "I64u"  // Windows...
-#endif
                 LOG_CMSG("touch [id=%" PRIu64_ "] %-4s position=%" PRIi32 ",%"
                              PRIi32 " pressure=%g buttons=%06lx",
                          id,
@@ -199,7 +195,8 @@ control_msg_log(const struct control_msg *msg) {
                      KEYEVENT_ACTION_LABEL(msg->inject_keycode.action));
             break;
         case CONTROL_MSG_TYPE_SET_CLIPBOARD:
-            LOG_CMSG("clipboard %s \"%s\"",
+            LOG_CMSG("clipboard %" PRIu64_ " %s \"%s\"",
+                     msg->set_clipboard.sequence,
                      msg->set_clipboard.paste ? "paste" : "copy",
                      msg->set_clipboard.text);
             break;

app/src/control_msg.h

@@ -70,6 +70,7 @@ struct control_msg {
             // screen may only be turned on on ACTION_DOWN
         } back_or_screen_on;
         struct {
+            uint64_t sequence;
             char *text; // owned, to be freed by free()
             bool paste;
         } set_clipboard;

app/src/controller.c

@@ -5,10 +5,11 @@
 #include "util/log.h"
 
 bool
-controller_init(struct controller *controller, sc_socket control_socket) {
+controller_init(struct controller *controller, sc_socket control_socket,
+                struct sc_acksync *acksync) {
     cbuf_init(&controller->queue);
 
-    bool ok = receiver_init(&controller->receiver, control_socket);
+    bool ok = receiver_init(&controller->receiver, control_socket, acksync);
     if (!ok) {
         return false;
     }

app/src/controller.h

@@ -7,6 +7,7 @@
 
 #include "control_msg.h"
 #include "receiver.h"
+#include "util/acksync.h"
 #include "util/cbuf.h"
 #include "util/net.h"
 #include "util/thread.h"
@@ -24,7 +25,8 @@ struct controller {
 };
 
 bool
-controller_init(struct controller *controller, sc_socket control_socket);
+controller_init(struct controller *controller, sc_socket control_socket,
+                struct sc_acksync *acksync);
 
 void
 controller_destroy(struct controller *controller);

app/src/device_msg.c

@@ -1,5 +1,6 @@
 #include "device_msg.h"
 
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -34,6 +35,11 @@ device_msg_deserialize(const unsigned char *buf, size_t len,
             msg->clipboard.text = text;
             return 5 + clipboard_len;
         }
+        case DEVICE_MSG_TYPE_ACK_CLIPBOARD: {
+            uint64_t sequence = buffer_read64be(&buf[1]);
+            msg->ack_clipboard.sequence = sequence;
+            return 9;
+        }
         default:
             LOGW("Unknown device message type: %d", (int) msg->type);
             return -1; // error, we cannot recover

app/src/device_msg.h

@@ -13,6 +13,7 @@
 
 enum device_msg_type {
     DEVICE_MSG_TYPE_CLIPBOARD,
+    DEVICE_MSG_TYPE_ACK_CLIPBOARD,
 };
 
 struct device_msg {
@@ -21,6 +22,9 @@ struct device_msg {
         struct {
             char *text; // owned, to be freed by free()
         } clipboard;
+        struct {
+            uint64_t sequence;
+        } ack_clipboard;
     };
 };
 

app/src/hid_keyboard.c

@@ -278,7 +278,8 @@ push_mod_lock_state(struct sc_hid_keyboard *kb, uint16_t sdl_mod) {
 
 static void
 sc_key_processor_process_key(struct sc_key_processor *kp,
-                             const SDL_KeyboardEvent *event) {
+                             const SDL_KeyboardEvent *event,
+                             uint64_t ack_to_wait) {
     if (event->repeat) {
         // In USB HID protocol, key repeat is handled by the host (Android), so
         // just ignore key repeat here.
@@ -298,16 +299,12 @@ sc_key_processor_process_key(struct sc_key_processor *kp,
             }
         }
 
-        SDL_Keycode keycode = event->keysym.sym;
-        bool down = event->type == SDL_KEYDOWN;
-        bool ctrl = event->keysym.mod & KMOD_CTRL;
-        bool shift = event->keysym.mod & KMOD_SHIFT;
-        if (ctrl && !shift && keycode == SDLK_v && down) {
+        if (ack_to_wait) {
             // Ctrl+v is pressed, so clipboard synchronization has been
-            // requested. Wait a bit so that the clipboard is set before
-            // injecting Ctrl+v via HID, otherwise it would paste the old
-            // clipboard content.
-            hid_event.delay = SC_TICK_FROM_MS(5);
+            // requested. Wait until clipboard synchronization is acknowledged
+            // by the server, otherwise it could paste the old clipboard
+            // content.
+            hid_event.ack_to_wait = ack_to_wait;
         }
 
         if (!sc_aoa_push_hid_event(kb->aoa, &hid_event)) {
@@ -348,6 +345,10 @@ sc_hid_keyboard_init(struct sc_hid_keyboard *kb, struct sc_aoa *aoa) {
         .process_text = sc_key_processor_process_text,
     };
 
+    // Clipboard synchronization is requested over the control socket, while HID
+    // events are sent over AOA, so it must wait for clipboard synchronization
+    // to be acknowledged by the device before injecting Ctrl+v.
+    kb->key_processor.async_paste = true;
     kb->key_processor.ops = &ops;
 
     return true;

app/src/input_manager.c

@@ -82,6 +82,8 @@ input_manager_init(struct input_manager *im, struct controller *controller,
     im->last_keycode = SDLK_UNKNOWN;
     im->last_mod = 0;
     im->key_repeat = 0;
+
+    im->next_sequence = 1; // 0 is reserved for SC_SEQUENCE_INVALID
 }
 
 static void
@@ -208,35 +210,35 @@ collapse_panels(struct controller *controller) {
     }
 }
 
-static void
-set_device_clipboard(struct controller *controller, bool paste) {
+static bool
+set_device_clipboard(struct controller *controller, bool paste,
+                     uint64_t sequence) {
     char *text = SDL_GetClipboardText();
     if (!text) {
         LOGW("Could not get clipboard text: %s", SDL_GetError());
-        return;
-    }
-    if (!*text) {
-        // empty text
-        SDL_free(text);
-        return;
+        return false;
     }
 
     char *text_dup = strdup(text);
     SDL_free(text);
     if (!text_dup) {
         LOGW("Could not strdup input text");
-        return;
+        return false;
     }
 
     struct control_msg msg;
     msg.type = CONTROL_MSG_TYPE_SET_CLIPBOARD;
+    msg.set_clipboard.sequence = sequence;
     msg.set_clipboard.text = text_dup;
     msg.set_clipboard.paste = paste;
 
     if (!controller_push_msg(controller, &msg)) {
         free(text_dup);
         LOGW("Could not request 'set device clipboard'");
+        return false;
     }
+
+    return true;
 }
 
 static void
@@ -462,8 +464,10 @@ input_manager_process_key(struct input_manager *im,
                         // inject the text as input events
                         clipboard_paste(controller);
                     } else {
-                        // store the text in the device clipboard and paste
-                        set_device_clipboard(controller, true);
+                        // store the text in the device clipboard and paste,
+                        // without requesting an acknowledgment
+                        set_device_clipboard(controller, true,
+                                             SC_SEQUENCE_INVALID);
                     }
                 }
                 return;
@@ -512,18 +516,36 @@ input_manager_process_key(struct input_manager *im,
         return;
     }
 
-    if (ctrl && !shift && keycode == SDLK_v && down && !repeat) {
+    uint64_t ack_to_wait = SC_SEQUENCE_INVALID;
+    bool is_ctrl_v = ctrl && !shift && keycode == SDLK_v && down && !repeat;
+    if (is_ctrl_v) {
         if (im->legacy_paste) {
             // inject the text as input events
             clipboard_paste(controller);
             return;
         }
+
+        // Request an acknowledgement only if necessary
+        uint64_t sequence = im->kp->async_paste ? im->next_sequence
+                                                : SC_SEQUENCE_INVALID;
+
         // Synchronize the computer clipboard to the device clipboard before
         // sending Ctrl+v, to allow seamless copy-paste.
-        set_device_clipboard(controller, false);
+        bool ok = set_device_clipboard(controller, false, sequence);
+        if (!ok) {
+            LOGW("Clipboard could not be synchronized, Ctrl+v not injected");
+            return;
+        }
+
+        if (im->kp->async_paste) {
+            // The key processor must wait for this ack before injecting Ctrl+v
+            ack_to_wait = sequence;
+            // Increment only when the request succeeded
+            ++im->next_sequence;
+        }
     }
 
-    im->kp->ops->process_key(im->kp, event);
+    im->kp->ops->process_key(im->kp, event, ack_to_wait);
 }
 
 static void