Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

  Hi,

> This approach doesn't work, although the problem is different. Input is now 
> recognized, but unwanted key repeats creep in. For example, I typed "exit" and 
> got "exiitt". I tested this several times while typing as accurately and 
> quickly as I could.
> 
> My theory is that the key releases aren't being detected by the application 
> until the next key press/release delay, thus effectively leaving the keys 
> pressed.

Sounds plausible.  Then adding a delay after queuing the release events
should fix it.  Can you try the attached patch (as incremental fix on
top of the other three patches)?

thanks,
  Gerd


[-- Attachment #2: 0001-curses-add-kbd-delay-between-keydown-and-keyup-event.patch --]
[-- Type: text/x-patch, Size: 747 bytes --]

>From 29f48811c69c9e734ce22894e93852af9c9f9de6 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 2 Jun 2014 09:03:21 +0200
Subject: [PATCH] curses: add kbd delay between keydown and keyup events
 [fixup]

---
 ui/curses.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/curses.c b/ui/curses.c
index ff143cf..d8dea9f 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -304,6 +304,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
             if (keycode & SHIFT) {
                 qemu_input_event_send_key_number(NULL, SHIFT_CODE, false);
             }
+            qemu_input_event_send_key_delay(100);
         } else {
             keysym = curses2qemu[chr];
             if (keysym == -1)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Dave Mielke]

[quoted lines by Gerd Hoffmann on 2014/06/02 at 09:07 +0200]

Hi:

>Sounds plausible.  Then adding a delay after queuing the release events
>should fix it.  Can you try the attached patch (as incremental fix on
>top of the other three patches)?

It still isn't working correctly. I've just played with it now, with the 
incremental patch, and see some very odd things.

For example: I held down the letter "j" (a lowercase j), and, as expected, "j"s 
were echoed back at about the right rate. When I let go of the "j", they kept 
on coming. I then held down backspace, and they started to disappear. When they 
were all gone, I let go of backspace. At that point, the keyboard appeared to 
be dead. I played around with only short presses of "a" and 's" for a while, 
and suddenly the "j"s started to come back.

Something else I noticed was that typing a control character would result in a 
doubling of that character. For example, control "u" resulted in "UU".

Now to go figure out how to stop those "j"s again. I left them repeating while 
I wrote this. :-)

-- 
Dave Mielke           | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: dave@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Dave Mielke]

Hi:

A follow-up to my last message. I'm glad I left those "j"s repeating while I 
wrote it. After sending it, I returned to that session and discovered that 
they'd finally stopped. This caused me to do some more experimentation.

I held "j" down for quite a short period of time, and, sure enough, they did 
stop about two seconds after. I then held backspace to get rid of them, letting 
go of it as soon as they were all gone. The keyboard only appeared to be dead 
for, again, a couple of seconds.

I then tried these same tests with my patch. Autorepeating worked exactly as it 
should, stopping as soon as I let go of the key. Backspacing till they were all 
gone also left me with a live keyboard immediately. I did the control letter 
test and they did what they should do instead of just double echoing the 
non-control letter.

-- 
Dave Mielke           | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: dave@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]

On Mo, 2014-06-02 at 05:00 -0400, Dave Mielke wrote:
> Hi:
> 
> A follow-up to my last message. I'm glad I left those "j"s repeating while I 
> wrote it. After sending it, I returned to that session and discovered that 
> they'd finally stopped. This caused me to do some more experimentation.
> 
> I held "j" down for quite a short period of time, and, sure enough, they did 
> stop about two seconds after. I then held backspace to get rid of them, letting 
> go of it as soon as they were all gone. The keyboard only appeared to be dead 
> for, again, a couple of seconds.
> 
> I then tried these same tests with my patch. Autorepeating worked exactly as it 
> should, stopping as soon as I let go of the key. Backspacing till they were all 
> gone also left me with a live keyboard immediately. I did the control letter 
> test and they did what they should do instead of just double echoing the 
> non-control letter.

What mdelay you are using with your patch?

I've have hardcoded 100ms (same delay used by send-key monitor command).
Which is probably too much, especially with autorepeat.  Key events come
in faster than they are sent to the guest, so they pile up in the queue.
You lift the key, and qemu continues streaming the events to the guest
until the queue is empty.

Highest repeat rate ps/2 allows is 30Hz.  Which is 33 ms delay per key
event.  We have two delays in there (one after pressing all keys, one
after releasing all keys).  So something between 10 and 15 ms should be
short enough to make sure the queue doesn't grow.  Lets try that,
incremental patch attached.

cheers,
  Gerd


[-- Attachment #2: 0001-curses-add-kbd-delay-between-keydown-and-keyup-event.patch --]
[-- Type: text/x-patch, Size: 1207 bytes --]

>From 20e2f797dc18524ec58ccb7a7924b574fd6a22f5 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Mon, 2 Jun 2014 11:28:39 +0200
Subject: [PATCH] curses: add kbd delay between keydown and keyup events
 [fixup]

---
 ui/curses.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/curses.c b/ui/curses.c
index d8dea9f..ade4bea 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -289,7 +289,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
             }
 
             qemu_input_event_send_key_number(NULL, keycode & KEY_MASK, true);
-            qemu_input_event_send_key_delay(100);
+            qemu_input_event_send_key_delay(10);
             qemu_input_event_send_key_number(NULL, keycode & KEY_MASK, false);
 
             if (keycode & ALTGR) {
@@ -304,7 +304,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
             if (keycode & SHIFT) {
                 qemu_input_event_send_key_number(NULL, SHIFT_CODE, false);
             }
-            qemu_input_event_send_key_delay(100);
+            qemu_input_event_send_key_delay(10);
         } else {
             keysym = curses2qemu[chr];
             if (keysym == -1)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Gerd Hoffmann]

  Hi,

> Something else I noticed was that typing a control character would result in a 
> doubling of that character. For example, control "u" resulted in "UU".

Does that also happen with the "sendkey ctrl-u" monitor command?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Dave Mielke]

[quoted lines by Gerd Hoffmann on 2014/06/02 at 11:29 +0200]

Hi:

When using my patch, I'm specifying -k delay=20 (so 20ms).

>Highest repeat rate ps/2 allows is 30Hz.  Which is 33 ms delay per key
>event.  We have two delays in there (one after pressing all keys, one
>after releasing all keys).  So something between 10 and 15 ms should be
>short enough to make sure the queue doesn't grow.  Lets try that,
>incremental patch attached.

Key repeat now stops when it should, and letting go of backspace as soon as all 
the characters are gone no longer leaves me with a temporarily dead keyboard. 
There are still problems, though. When I hold the shift key and type a letter, 
I get a doubled lowercase letter rather than a single uppercase letter. Also, I 
still get doubled lowercase letters when holding the control key.

-- 
Dave Mielke           | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: dave@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Dave Mielke]

[quoted lines by Gerd Hoffmann on 2014/06/02 at 11:31 +0200]

Hi:

>Does that also happen with the "sendkey ctrl-u" monitor command?

Yes, it does. This test was with the newest (10ms) delay patch.

-- 
Dave Mielke           | 2213 Fox Crescent | The Bible is the very Word of God.
Phone: 1-613-726-0014 | Ottawa, Ontario   | http://Mielke.cc/bible/
EMail: dave@mielke.cc | Canada  K2A 1H7   | http://FamilyRadio.com/

Re: [Qemu-devel] [PATCH 0/3] input: add support for kbd delays

[Gerd Hoffmann]

On Mo, 2014-06-02 at 06:40 -0400, Dave Mielke wrote:
> [quoted lines by Gerd Hoffmann on 2014/06/02 at 11:29 +0200]
> 
> Hi:
> 
> When using my patch, I'm specifying -k delay=20 (so 20ms).
> 
> >Highest repeat rate ps/2 allows is 30Hz.  Which is 33 ms delay per key
> >event.  We have two delays in there (one after pressing all keys, one
> >after releasing all keys).  So something between 10 and 15 ms should be
> >short enough to make sure the queue doesn't grow.  Lets try that,
> >incremental patch attached.
> 
> Key repeat now stops when it should, and letting go of backspace as soon as all 
> the characters are gone no longer leaves me with a temporarily dead keyboard. 
> There are still problems, though. When I hold the shift key and type a letter, 
> I get a doubled lowercase letter rather than a single uppercase letter. Also, I 
> still get doubled lowercase letters when holding the control key.

Ok, so looks like the guest really needs a delay after every single key
event, grouping all down and all up events doesn't work.  New version of
the patch series goes out in a moment.  That should fix both curses ui
and monitor sendkey command.

cheers,
  Gerd