[Qemu-devel] [PATCH] vnc: add configurable keyboard delay

[Qemu-devel] [PATCH] vnc: add configurable keyboard delay

[Gerd Hoffmann]

Limits the rate kbd events from the vnc server are forwarded to the
guest, so input devices which are typically low-bandwidth can keep
up even on bulky input.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/vnc.c | 13 +++++++++++--
 ui/vnc.h |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index d2ebf1f..ea3b3d4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1629,6 +1629,7 @@ static void reset_keys(VncState *vs)
     for(i = 0; i < 256; i++) {
         if (vs->modifiers_state[i]) {
             qemu_input_event_send_key_number(vs->vd->dcl.con, i, false);
+            qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
             vs->modifiers_state[i] = 0;
         }
     }
@@ -1638,9 +1639,9 @@ static void press_key(VncState *vs, int keysym)
 {
     int keycode = keysym2scancode(vs->vd->kbd_layout, keysym) & SCANCODE_KEYMASK;
     qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
-    qemu_input_event_send_key_delay(0);
+    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
     qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
-    qemu_input_event_send_key_delay(0);
+    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
 }
 
 static int current_led_state(VncState *vs)
@@ -1792,6 +1793,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
 
     if (qemu_console_is_graphic(NULL)) {
         qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, down);
+        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
     } else {
         bool numlock = vs->modifiers_state[0x45];
         bool control = (vs->modifiers_state[0x1d] ||
@@ -1913,6 +1915,7 @@ static void vnc_release_modifiers(VncState *vs)
             continue;
         }
         qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
+        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
     }
 }
 
@@ -3249,6 +3252,9 @@ static QemuOptsList qemu_vnc_opts = {
             .name = "lock-key-sync",
             .type = QEMU_OPT_BOOL,
         },{
+            .name = "key-delay-ms",
+            .type = QEMU_OPT_NUMBER,
+        },{
             .name = "sasl",
             .type = QEMU_OPT_BOOL,
         },{
@@ -3486,6 +3492,7 @@ void vnc_display_open(const char *id, Error **errp)
 #endif
     int acl = 0;
     int lock_key_sync = 1;
+    int key_delay_ms;
 
     if (!vs) {
         error_setg(errp, "VNC display not active");
@@ -3604,6 +3611,7 @@ void vnc_display_open(const char *id, Error **errp)
 
     reverse = qemu_opt_get_bool(opts, "reverse", false);
     lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
+    key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1);
     sasl = qemu_opt_get_bool(opts, "sasl", false);
 #ifndef CONFIG_VNC_SASL
     if (sasl) {
@@ -3735,6 +3743,7 @@ void vnc_display_open(const char *id, Error **errp)
     }
 #endif
     vs->lock_key_sync = lock_key_sync;
+    vs->key_delay_ms = key_delay_ms;
 
     device_id = qemu_opt_get(opts, "display");
     if (device_id) {
diff --git a/ui/vnc.h b/ui/vnc.h
index 81a3261..6568bca 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -155,6 +155,7 @@ struct VncDisplay
     DisplayChangeListener dcl;
     kbd_layout_t *kbd_layout;
     int lock_key_sync;
+    int key_delay_ms;
     QemuMutex mutex;
 
     QEMUCursor *cursor;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] vnc: add configurable keyboard delay

[Laurent Vivier]

On 23/05/2016 15:19, Gerd Hoffmann wrote:
> Limits the rate kbd events from the vnc server are forwarded to the
> guest, so input devices which are typically low-bandwidth can keep
> up even on bulky input.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

This patch fixes also a problem with low-bandwith connection between VNC
client and server that can create (for instance) 400 ms delay between
down and up events and thus triggers key auto repeat (kernel autorepeat
delay is generally 250 ms)

With this patch and "-vnc ...,key-delay-ms=200", the problem goes away.

Thanks,
Laurent

> ---
>  ui/vnc.c | 13 +++++++++++--
>  ui/vnc.h |  1 +
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index d2ebf1f..ea3b3d4 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1629,6 +1629,7 @@ static void reset_keys(VncState *vs)
>      for(i = 0; i < 256; i++) {
>          if (vs->modifiers_state[i]) {
>              qemu_input_event_send_key_number(vs->vd->dcl.con, i, false);
> +            qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>              vs->modifiers_state[i] = 0;
>          }
>      }
> @@ -1638,9 +1639,9 @@ static void press_key(VncState *vs, int keysym)
>  {
>      int keycode = keysym2scancode(vs->vd->kbd_layout, keysym) & SCANCODE_KEYMASK;
>      qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
> -    qemu_input_event_send_key_delay(0);
> +    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>      qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
> -    qemu_input_event_send_key_delay(0);
> +    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>  }
>  
>  static int current_led_state(VncState *vs)
> @@ -1792,6 +1793,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>  
>      if (qemu_console_is_graphic(NULL)) {
>          qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, down);
> +        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>      } else {
>          bool numlock = vs->modifiers_state[0x45];
>          bool control = (vs->modifiers_state[0x1d] ||
> @@ -1913,6 +1915,7 @@ static void vnc_release_modifiers(VncState *vs)
>              continue;
>          }
>          qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
> +        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>      }
>  }
>  
> @@ -3249,6 +3252,9 @@ static QemuOptsList qemu_vnc_opts = {
>              .name = "lock-key-sync",
>              .type = QEMU_OPT_BOOL,
>          },{
> +            .name = "key-delay-ms",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
>              .name = "sasl",
>              .type = QEMU_OPT_BOOL,
>          },{
> @@ -3486,6 +3492,7 @@ void vnc_display_open(const char *id, Error **errp)
>  #endif
>      int acl = 0;
>      int lock_key_sync = 1;
> +    int key_delay_ms;
>  
>      if (!vs) {
>          error_setg(errp, "VNC display not active");
> @@ -3604,6 +3611,7 @@ void vnc_display_open(const char *id, Error **errp)
>  
>      reverse = qemu_opt_get_bool(opts, "reverse", false);
>      lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
> +    key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1);
>      sasl = qemu_opt_get_bool(opts, "sasl", false);
>  #ifndef CONFIG_VNC_SASL
>      if (sasl) {
> @@ -3735,6 +3743,7 @@ void vnc_display_open(const char *id, Error **errp)
>      }
>  #endif
>      vs->lock_key_sync = lock_key_sync;
> +    vs->key_delay_ms = key_delay_ms;
>  
>      device_id = qemu_opt_get(opts, "display");
>      if (device_id) {
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 81a3261..6568bca 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -155,6 +155,7 @@ struct VncDisplay
>      DisplayChangeListener dcl;
>      kbd_layout_t *kbd_layout;
>      int lock_key_sync;
> +    int key_delay_ms;
>      QemuMutex mutex;
>  
>      QEMUCursor *cursor;
> 

Re: [Qemu-devel] [PATCH] vnc: add configurable keyboard delay

[Laurent Vivier]

On 23/05/2016 15:19, Gerd Hoffmann wrote:
> Limits the rate kbd events from the vnc server are forwarded to the
> guest, so input devices which are typically low-bandwidth can keep
> up even on bulky input.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/vnc.c | 13 +++++++++++--
>  ui/vnc.h |  1 +
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index d2ebf1f..ea3b3d4 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1629,6 +1629,7 @@ static void reset_keys(VncState *vs)
>      for(i = 0; i < 256; i++) {
>          if (vs->modifiers_state[i]) {
>              qemu_input_event_send_key_number(vs->vd->dcl.con, i, false);
> +            qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>              vs->modifiers_state[i] = 0;
>          }
>      }
> @@ -1638,9 +1639,9 @@ static void press_key(VncState *vs, int keysym)
>  {
>      int keycode = keysym2scancode(vs->vd->kbd_layout, keysym) & SCANCODE_KEYMASK;
>      qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
> -    qemu_input_event_send_key_delay(0);
> +    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>      qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
> -    qemu_input_event_send_key_delay(0);
> +    qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>  }
>  
>  static int current_led_state(VncState *vs)
> @@ -1792,6 +1793,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>  
>      if (qemu_console_is_graphic(NULL)) {
>          qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, down);
> +        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>      } else {
>          bool numlock = vs->modifiers_state[0x45];
>          bool control = (vs->modifiers_state[0x1d] ||
> @@ -1913,6 +1915,7 @@ static void vnc_release_modifiers(VncState *vs)
>              continue;
>          }
>          qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
> +        qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
>      }
>  }
>  
> @@ -3249,6 +3252,9 @@ static QemuOptsList qemu_vnc_opts = {
>              .name = "lock-key-sync",
>              .type = QEMU_OPT_BOOL,
>          },{
> +            .name = "key-delay-ms",
> +            .type = QEMU_OPT_NUMBER,
> +        },{
>              .name = "sasl",
>              .type = QEMU_OPT_BOOL,
>          },{
> @@ -3486,6 +3492,7 @@ void vnc_display_open(const char *id, Error **errp)
>  #endif
>      int acl = 0;
>      int lock_key_sync = 1;
> +    int key_delay_ms;
>  
>      if (!vs) {
>          error_setg(errp, "VNC display not active");
> @@ -3604,6 +3611,7 @@ void vnc_display_open(const char *id, Error **errp)
>  
>      reverse = qemu_opt_get_bool(opts, "reverse", false);
>      lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true);
> +    key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1);

Why the default value is 1 and not 0?
(Are there some impacts on the "qemu_input_event_send_key_delay(0);" you
replace above?)

Laurent
>      sasl = qemu_opt_get_bool(opts, "sasl", false);
>  #ifndef CONFIG_VNC_SASL
>      if (sasl) {
> @@ -3735,6 +3743,7 @@ void vnc_display_open(const char *id, Error **errp)
>      }
>  #endif
>      vs->lock_key_sync = lock_key_sync;
> +    vs->key_delay_ms = key_delay_ms;
>  
>      device_id = qemu_opt_get(opts, "display");
>      if (device_id) {
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 81a3261..6568bca 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -155,6 +155,7 @@ struct VncDisplay
>      DisplayChangeListener dcl;
>      kbd_layout_t *kbd_layout;
>      int lock_key_sync;
> +    int key_delay_ms;
>      QemuMutex mutex;
>  
>      QEMUCursor *cursor;
> 

Re: [Qemu-devel] [PATCH] vnc: add configurable keyboard delay

[Gerd Hoffmann]

  Hi,

> > +    key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1);
> 
> Why the default value is 1 and not 0?
> (Are there some impacts on the "qemu_input_event_send_key_delay(0);" you
> replace above?)

I think it makes sense to have a delay by default, so the guest is
scheduled to run after each event, increasing the chance it picks up the
event.  And 1 ms delay is so small that you shouldn't be able to notice
it.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH] vnc: add configurable keyboard delay

[Alexander Graf]

On 05/23/2016 03:19 PM, Gerd Hoffmann wrote:
> Limits the rate kbd events from the vnc server are forwarded to the
> guest, so input devices which are typically low-bandwidth can keep
> up even on bulky input.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>   ui/vnc.c | 13 +++++++++++--
>   ui/vnc.h |  1 +

You probably want to extend the man page with this awesome new option :).


Alex