Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
[quoted lines by Gerd Hoffmann on 2014/05/27 at 07:44 +0200] What exactly is the problem? At the user level, the keyboard appears to be dead. An inspection of the udnerlying code reveals that the application itsllf is querying the MS-DOS keyboard input buffer in a bad way. Those apps can't be fixed, though, because the bad code is in a library which is statically linked into every app using it, which means lots of apps, many of which, as is always the case, may not even have source code anymore. Another thing to consider is that this, really, is a general problem. Most keyboard input code, especially in older days, would've nly ever been tested at typing speed. It should be expected, therefore, that there might be problems in some of it when input events arrive faster - in this case, infinitely fater. The solution I'm proposing, therefore, tackles it in a general way, and does nothing more than allow the user to request that, regardless of what the UI needs to do underneath, the keyboard events still arrive at the speed of a typist. Did you see my other question abot either adding -kbddelay [duration=]msecs or merging it into -k (-k [language=]language[,delay=msecs]? Which would be the better approach? -- 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: d...@mielke.cc | Canada K2A 1H7 | http://FamilyRadio.com/
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
On Di, 2014-05-27 at 04:15 -0400, Dave Mielke wrote: [quoted lines by Gerd Hoffmann on 2014/05/27 at 07:44 +0200] What exactly is the problem? At the user level, the keyboard appears to be dead. An inspection of the udnerlying code reveals that the application itsllf is querying the MS-DOS keyboard input buffer in a bad way. Which means exactly? They are not prepared to find data for more than one key event in the buffer? Those apps can't be fixed, though, because the bad code is in a library which is statically linked into every app using it, which means lots of apps, many of which, as is always the case, may not even have source code anymore. Sure. Another thing to consider is that this, really, is a general problem. Most keyboard input code, especially in older days, would've nly ever been tested at typing speed. It should be expected, therefore, that there might be problems in some of it when input events arrive faster - in this case, infinitely fater. The solution I'm proposing, therefore, tackles it in a general way, and does nothing more than allow the user to request that, regardless of what the UI needs to do underneath, the keyboard events still arrive at the speed of a typist. I don't like the idea to do it in the UI code. I don't think the problem is as generic as you think it is. HID keyboards (usb, bluetooth) report keys in a completely different way, and probably your problem is specific to the ps/2 keyboard. I'd try to do it in the ps/2 code, and try to do it without a timer. Idea: Split the ps/2 keyboard queue into two. One will be guest-visible. The existing ps2 queue will do the job. The other, new queue will not be visible to the guest. Simliar to the queue you've added to the ui code. Put not more than one key event (1-3 scancodes) into the guest visible queue. Wait for the guest read the status register (signaling no more data) once. Then go refill the guest-visible queue from the invisible queue. With some luck this makes things work without a delay. If not we are not so lucky we have to fire a timer for queue refill after signaling no more data to the guest. Even in case we need a timer this is better than doing it in the ui code as we can arm the timer when we know the guest has actually seen the key event. So the guest being busy with something else and not polling the kbd for a time span longer than the delay will not break things, so we can use a pretty short delay. cheers, Gerd
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
On 05/25/14 14:56, Dave Mielke wrote: [quoted lines by Michael Tokarev on 2014/05/25 at 16:41 +0400] And provided this hasn't been a problem for so many years, Perhaps it hasn't been observed before because the curses UI may not have that many users. I myself must use it as, being blind, it's much easier for me to work in text mode than in graphics mode. You were too modest to link your own webpage, but I'll do it for us all, because it'll help our understanding: http://mielke.cc/brltty/index.html http://mielke.cc/resume.html#exp-2000-PRESENT Thanks Laszlo PS: stunned and amazed
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
[quoted lines by Gerd Hoffmann on 2014/05/27 at 11:09 +0200] Which means exactly? They are not prepared to find data for more than one key event in the buffer? Not exactly. The bad code is an attempt to do a nonblocking keyboard read in MS-DOS. It goes like this: 1) Peek the DOS keyboard buffer start and end pointers. If the buffer is empty then return no input. 2) If no key is pressed then return no input. 3) Use the blocking MS-DOS interface to wait for the next key and return it. So, as you can see, there's a serious problem. Let's say that a key press and release go in together. That give us the following situation: 1) Peek the DOS input buffer. It isn't empty so proceed to the next step. 2) No key is pressed so return no input. I really do think, therefore, that the only way to deal with this is to slow down the input to typing speed. -- 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: d...@mielke.cc | Canada K2A 1H7 | http://FamilyRadio.com/
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
[quoted lines by Gerd Hoffmann on 2014/05/26 at 15:38 +0200] Tried to make the curses ui a bit more clever? You could try caching the modifier state, then send only the changes. That gets the number of events down to 6 max (4 to update modifier state, 2 for the actual key). Yes, except that, in my case, the problem already occurs on simple typing of lowercase letters, i.e. just two events. Also, that approach could introduce new problems due to modifeir keys appearing to be stuck. Other question: has git revert 2858ab09e6f708e381fc1a1cc87e747a690c4884 any effect? That gives me: could not revert 2858ab0... ps2: set ps/2 output buffer size as the same as kernel -- 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: d...@mielke.cc | Canada K2A 1H7 | http://FamilyRadio.com/
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
On Mo, 2014-05-26 at 11:19 -0400, Dave Mielke wrote: [quoted lines by Gerd Hoffmann on 2014/05/26 at 15:38 +0200] Tried to make the curses ui a bit more clever? You could try caching the modifier state, then send only the changes. That gets the number of events down to 6 max (4 to update modifier state, 2 for the actual key). Yes, except that, in my case, the problem already occurs on simple typing of lowercase letters, i.e. just two events. What exactly is the problem? Also, that approach could introduce new problems due to modifeir keys appearing to be stuck. Indeed. Other question: has git revert 2858ab09e6f708e381fc1a1cc87e747a690c4884 any effect? That gives me: could not revert 2858ab0... ps2: set ps/2 output buffer size as the same as kernel Given that only two keys already problematic this probably wouldn't change things anyway. cheers, Gerd
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
25.05.2014 03:29, Dave Mielke wrote: Add support for the -display curses option to accept suboptions (-display curses[,option...), and add the delay=msecs suboption. This suboption causes a millisecond-based delay to be inserted in between key events so that In addition to what Peter said, I think this suboption is named poorly. Maybe it can be named, say, kbddelay, or keydelay, or something like that. Just delay may be interpreted as _video_ delay, ie, delay updating picture for so many millisecs. After all, this is -display, which is about video, and keyboard is a secondary function... Also, you still haven't told us what application is having problem with it, it is some mystery some app. And provided this hasn't been a problem for so many years, maybe we shouldn't add it in the first place? Thanks, /mjt
Re: [Qemu-devel] [Qemu-trivial] patch: add delay=msecs suboption to -display curses
[quoted lines by Michael Tokarev on 2014/05/25 at 16:41 +0400] In addition to what Peter said, I think this suboption is named poorly. Maybe it can be named, say, kbddelay, or keydelay, or something like that. Just delay may be interpreted as _video_ delay, ie, delay updating picture for so many millisecs. After all, this is -display, which is about video, and keyboard is a secondary function... Yes, that makes good sense. Also, you still haven't told us what application is having problem with it, it is some mystery some app. Where I ran into it is DJGPP termios input on MS-DOS. And provided this hasn't been a problem for so many years, Perhaps it hasn't been observed before because the curses UI may not have that many users. I myself must use it as, being blind, it's much easier for me to work in text mode than in graphics mode. maybe we shouldn't add it in the first place? Given that this feature does resolve a problem, given that it's default retains the original behaviour, and given that we have no idea whatsoever regarding when the problem may strike again, my personal opinion is that implementing it now would be a reasonable thing to do, especially since the current situation is that no one is being asked to figure it out, code it, etc. -- 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: d...@mielke.cc | Canada K2A 1H7 | http://FamilyRadio.com/
