Re: IR remote control autorepeat / evdev
On 15.05.2011 06:41, Mauro Carvalho Chehab wrote: Em 14-05-2011 01:07, Anssi Hannula escreveu: On 14.05.2011 01:39, Mauro Carvalho Chehab wrote: Em 13-05-2011 01:48, Anssi Hannula escreveu: On 12.05.2011 04:36, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. It will indeed reduce the amount of ghost events so it brings us in the right direction. I'd still like to get rid of the ghost repeats entirely, or at least some way for users to do it if we don't do it by default. Maybe we could replace the kernel softrepeat with native repeats (for those protocols/drivers that have them), while making sure that repeat events before REP_DELAY are ignored and repeat events less than REP_PERIOD since the previous event are ignored, so the users can still configure them as they like? This doesn't seem to be the right thing to do. If the kernel will accept 33 ms as the value or REP_PERIOD, but it will internally set the maximum repeat rate is 115 ms (no matter what logic it would use for that), Kernel (or X) shouldn't allow the user to set a smaller value. The thing is that writing a logic to block a small value is not easy, since the max value is protocol-dependent (worse than that, on some cases, it is device-specific). It seems better to add a warning at the userspace tools that delays lower than 115 ms can produce ghost events on IR's. From what I see, even periods longer than 115 ms can produce ghost events. For example with your patch softrepeat period is 125ms, release timeout 250ms, and a native rate of 110ms: There are 4 native events transmitted at 000 ms 110 ms 220 ms 330 ms (user stops between 330ms and 440ms) This causes these events in the evdev interface: 000: 1 125: 2 250: 2 375: 2 500: 2 550: 0 So we got 1-2 ghost repeat events. Or maybe just a module option that causes rc-core to use native repeat events, for those of us that want accurate repeat events without ghosting? If the user already knows about the possibility to generate ghost effects, with low delays, he can simply not pass a bad value to the kernel, instead of forcing a modprobe parameter that will limit the minimal value. There is no good value for REP_PERIOD (as in ghost repeats guaranteed gone like with native repeats). Sufficiently large values will make ghost repeats increasingly rare, but the period becomes so long the autorepeat becomes frustratingly slow to use. The 250 ms delay used internally to wait for a repeat code is there because shorter periods weren't working on one of the first boards we've added to rc core (it was a saa7134 - can't remember much details... too much time ago). I remember that I added it as a per-board timer (or per protocol?), as it seemed to high for me, but later, David sent a series of patches rewriting the entire stuff and proposing to have just one timer, arguing that later this could be changed. As his series were improving rc-core, I ended by acking with the changes. The fact is that REP_PERIODS shorter than that timer makes non-sense, as that timer is used to actually wait for a repeat message. I suspect we should re-work the code, perhaps replacing the 250 ms fixed value by REP_PERIOD. Well, that still has a 50% chance of a ghost repeat with length 1-125ms (e.g. native rate 110ms, user releases button at 900ms, last native event at 880ms, evdev repeat events at 500,625,750,875,1000ms). It would be significantly better than it was before, though, and I'll have to test it myself to see if it is good enough (though I fear it is not). I can't work on it this weekend, as I'm about to leave Hungary to return back home. I suspect that I'll have lots of fun next week, due to a one-week travel, and due to the .40 merge window (I suspect it will be opened next week). Maybe Jarod can find some time to do such patch and test it. Thanks, Mauro. -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On Fri, May 13, 2011 at 09:51:02AM +0200, Mauro Carvalho Chehab wrote: Em 12-05-2011 08:05, Peter Hutterer escreveu: On Thu, May 12, 2011 at 03:36:47AM +0200, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. - Subject: Use a more consistent value for RC repeat period From: Mauro Carvalho Chehab mche...@redhat.com The default REP_PERIOD is 33 ms. This doesn't make sense for IR's, as, in general, an IR repeat scancode is provided at every 110/115ms, depending on the RC protocol. So, increase its default, to do a better job avoiding ghost repeat events. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index f53f9c6..ee67169 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -1044,6 +1044,13 @@ int rc_register_device(struct rc_dev *dev) */ dev-input_dev-rep[REP_DELAY] = 500; + /* + * As a repeat event on protocols like RC-5 and NEC take as long as + * 110/114ms, using 33ms as a repeat period is not the right thing + * to do. + */ + dev-input_dev-rep[REP_PERIOD] = 125; + path = kobject_get_path(dev-dev.kobj, GFP_KERNEL); printk(KERN_INFO %s: %s as %s\n, dev_name(dev-dev), so if I get this right, a XkbSetControls(.. XkbRepeatKeysMask ...) by a client to set the repeat rate would provide the same solution - for those clients/devices affected. Yes, if we preserve the same logic. The above will probably still generate ghost repeats if the user keeps the IR pressed for a long time, as we're using a separate timer at the rc-core logic than the one used inside evdev. The interesting question is how clients would identify the devices that are affected by this (other than trial and error). That is easy. I've added a logic that detects it on Xorg evdev on some RFC patches I've prepared in the past to allow using a keycode with more than 247 for IR's. I'll work on a new version for it and submit again when I have some time. Anyway, I'm enclosing a patch with the old version. Note that clients refers to X clients, i.e. applications. While it's usually trivial to add new functionality to evdev or other drivers, exposing information to the actual client requires some protocol extension or additions to existing extensions. While we have mechanisms in place for devices to be labelled, we don't have anyone to actually read this information (or even some standard on how to apply the labels). From the implementation-side, we not only need to flag the devices in the driver (like you've outlined in the patch below), we then need to get this information into the X server so that the server doesn't repeat. XKB has a per-key-repeat flag which we may be able to use but we need to also override client-side key repeat setting (for this device only). XKB doesn't allow for a repeat rate change request to fail, so we have to essentially tell client they have succeeded in setting their repeat rate while using a completely different one. Technically, you can override the repeat setting requested by the client if you simply send out an event when you change it back to the hardware setting. This then looks like some other client has changed it but the danger is that it may send stubborn clients into an infinite loop. How much that really matters I don't know. Letting clients know which device is an RC control at least means that the overriding should be expected, but that brings us back to the labelling issue. But either way, to even get this to the override stage you need three patches: - evdev: recognise and flag the devices - X server: introduce an API to pass this information on to the server - X server: fixes to the XKB system to disable autorepeat for devices flagged accordingly and override requests to change the repeat rate. Cheers, Peter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
Em 14-05-2011 01:07, Anssi Hannula escreveu: On 14.05.2011 01:39, Mauro Carvalho Chehab wrote: Em 13-05-2011 01:48, Anssi Hannula escreveu: On 12.05.2011 04:36, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. It will indeed reduce the amount of ghost events so it brings us in the right direction. I'd still like to get rid of the ghost repeats entirely, or at least some way for users to do it if we don't do it by default. Maybe we could replace the kernel softrepeat with native repeats (for those protocols/drivers that have them), while making sure that repeat events before REP_DELAY are ignored and repeat events less than REP_PERIOD since the previous event are ignored, so the users can still configure them as they like? This doesn't seem to be the right thing to do. If the kernel will accept 33 ms as the value or REP_PERIOD, but it will internally set the maximum repeat rate is 115 ms (no matter what logic it would use for that), Kernel (or X) shouldn't allow the user to set a smaller value. The thing is that writing a logic to block a small value is not easy, since the max value is protocol-dependent (worse than that, on some cases, it is device-specific). It seems better to add a warning at the userspace tools that delays lower than 115 ms can produce ghost events on IR's. From what I see, even periods longer than 115 ms can produce ghost events. For example with your patch softrepeat period is 125ms, release timeout 250ms, and a native rate of 110ms: There are 4 native events transmitted at 000 ms 110 ms 220 ms 330 ms (user stops between 330ms and 440ms) This causes these events in the evdev interface: 000: 1 125: 2 250: 2 375: 2 500: 2 550: 0 So we got 1-2 ghost repeat events. Or maybe just a module option that causes rc-core to use native repeat events, for those of us that want accurate repeat events without ghosting? If the user already knows about the possibility to generate ghost effects, with low delays, he can simply not pass a bad value to the kernel, instead of forcing a modprobe parameter that will limit the minimal value. There is no good value for REP_PERIOD (as in ghost repeats guaranteed gone like with native repeats). Sufficiently large values will make ghost repeats increasingly rare, but the period becomes so long the autorepeat becomes frustratingly slow to use. The 250 ms delay used internally to wait for a repeat code is there because shorter periods weren't working on one of the first boards we've added to rc core (it was a saa7134 - can't remember much details... too much time ago). I remember that I added it as a per-board timer (or per protocol?), as it seemed to high for me, but later, David sent a series of patches rewriting the entire stuff and proposing to have just one timer, arguing that later this could be changed. As his series were improving rc-core, I ended by acking with the changes. The fact is that REP_PERIODS shorter than that timer makes non-sense, as that timer is used to actually wait for a repeat message. I suspect we should re-work the code, perhaps replacing the 250 ms fixed value by REP_PERIOD. I can't work on it this weekend, as I'm about to leave Hungary to return back home. I suspect that I'll have lots of fun next week, due to a one-week travel, and due to the .40 merge window (I suspect it will be opened next week). Maybe Jarod can find some time to do such patch and test it. Thanks, Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
Em 12-05-2011 08:05, Peter Hutterer escreveu:
On Thu, May 12, 2011 at 03:36:47AM +0200, Mauro Carvalho Chehab wrote:
Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu:
Em 12-05-2011 02:37, Anssi Hannula escreveu:
I don't see any other places:
$ git grep 'REP_PERIOD' .
dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] =
d-props.rc.legacy.rc_interval;
Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we
should change it to something like 125ms, for example, as 33ms is too
short, as it takes up to 114ms for a repeat event to arrive.
IMO, the enclosed patch should do a better job with repeat events, without
needing to change rc-core/input/event logic.
-
Subject: Use a more consistent value for RC repeat period
From: Mauro Carvalho Chehab mche...@redhat.com
The default REP_PERIOD is 33 ms. This doesn't make sense for IR's,
as, in general, an IR repeat scancode is provided at every 110/115ms,
depending on the RC protocol. So, increase its default, to do a
better job avoiding ghost repeat events.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index f53f9c6..ee67169 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1044,6 +1044,13 @@ int rc_register_device(struct rc_dev *dev)
*/
dev-input_dev-rep[REP_DELAY] = 500;
+/*
+ * As a repeat event on protocols like RC-5 and NEC take as long as
+ * 110/114ms, using 33ms as a repeat period is not the right thing
+ * to do.
+ */
+dev-input_dev-rep[REP_PERIOD] = 125;
+
path = kobject_get_path(dev-dev.kobj, GFP_KERNEL);
printk(KERN_INFO %s: %s as %s\n,
dev_name(dev-dev),
so if I get this right, a XkbSetControls(.. XkbRepeatKeysMask ...) by a
client to set the repeat rate would provide the same solution - for those
clients/devices affected.
Yes, if we preserve the same logic. The above will probably still generate
ghost repeats if the user keeps the IR pressed for a long time, as we're using
a separate timer at the rc-core logic than the one used inside evdev.
The interesting question is how clients would identify the devices that are
affected by this (other than trial and error).
That is easy. I've added a logic that detects it on Xorg evdev on some RFC
patches
I've prepared in the past to allow using a keycode with more than 247 for IR's.
I'll work on a new version for it and submit again when I have some time.
Anyway, I'm enclosing a patch with the old version.
Basically, GetRCInputs.c file adds a logic that returns a list of input devices
that are Remote Controllers, using rc-core.
This logic inside evdev gets the RC devices and adds a flag identifying them
as such:
+/* Check if the device is a remote controller */
+pRCDevList = GetRCInputDevices(NumDevices);
+for (i = 0; i NumDevices; i++) {
+if (!strcmp(device, pRCDevList[i].InputName)) {
+ pEvdev-flags |= EVDEV_RC_EVENTS;
+ break;
+}
+}
Thanks,
Mauro
-
diff --git a/src/GetRCInputs.c b/src/GetRCInputs.c
new file mode 100644
index 000..0e03e3a
--- /dev/null
+++ b/src/GetRCInputs.c
@@ -0,0 +1,358 @@
+/*
+ * Copyright © 2011 Red Hat, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of Red Hat
+ * not be used in advertising or publicity pertaining to distribution
+ * of the software without specific, written prior permission. Red
+ * Hat makes no representations about the suitability of this software
+ * for any purpose. It is provided as is without express or implied
+ * warranty.
+ *
+ * THE AUTHORS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Author:
+ * Mauro Carvalho Chehab mche...@redhat.com
+ */
+
+#ifdef HAVE_CONFIG_H
+#include config.h
+#endif
+
+#include evdev.h
+
+#include X11/keysym.h
+#include X11/extensions/XI.h
+
+#include ctype.h
+#include errno.h
+#include fcntl.h
+#include stdio.h
+#include unistd.h
+#include stdlib.h
+#include string.h
+#include linux/input.h
+#include sys/ioctl.h
+#include dirent.h
+
+#include xf86.h
+#include xf86Xinput.h
+#include exevents.h
+#include xorgVersion.h
+#include xkbsrv.h
+
+struct UEvents {
+char *key;
+char *value;
+
Re: IR remote control autorepeat / evdev
Em 13-05-2011 01:48, Anssi Hannula escreveu: On 12.05.2011 04:36, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. It will indeed reduce the amount of ghost events so it brings us in the right direction. I'd still like to get rid of the ghost repeats entirely, or at least some way for users to do it if we don't do it by default. Maybe we could replace the kernel softrepeat with native repeats (for those protocols/drivers that have them), while making sure that repeat events before REP_DELAY are ignored and repeat events less than REP_PERIOD since the previous event are ignored, so the users can still configure them as they like? This doesn't seem to be the right thing to do. If the kernel will accept 33 ms as the value or REP_PERIOD, but it will internally set the maximum repeat rate is 115 ms (no matter what logic it would use for that), Kernel (or X) shouldn't allow the user to set a smaller value. The thing is that writing a logic to block a small value is not easy, since the max value is protocol-dependent (worse than that, on some cases, it is device-specific). It seems better to add a warning at the userspace tools that delays lower than 115 ms can produce ghost events on IR's. Or maybe just a module option that causes rc-core to use native repeat events, for those of us that want accurate repeat events without ghosting? If the user already knows about the possibility to generate ghost effects, with low delays, he can simply not pass a bad value to the kernel, instead of forcing a modprobe parameter that will limit the minimal value. Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On 14.05.2011 01:39, Mauro Carvalho Chehab wrote: Em 13-05-2011 01:48, Anssi Hannula escreveu: On 12.05.2011 04:36, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. It will indeed reduce the amount of ghost events so it brings us in the right direction. I'd still like to get rid of the ghost repeats entirely, or at least some way for users to do it if we don't do it by default. Maybe we could replace the kernel softrepeat with native repeats (for those protocols/drivers that have them), while making sure that repeat events before REP_DELAY are ignored and repeat events less than REP_PERIOD since the previous event are ignored, so the users can still configure them as they like? This doesn't seem to be the right thing to do. If the kernel will accept 33 ms as the value or REP_PERIOD, but it will internally set the maximum repeat rate is 115 ms (no matter what logic it would use for that), Kernel (or X) shouldn't allow the user to set a smaller value. The thing is that writing a logic to block a small value is not easy, since the max value is protocol-dependent (worse than that, on some cases, it is device-specific). It seems better to add a warning at the userspace tools that delays lower than 115 ms can produce ghost events on IR's. From what I see, even periods longer than 115 ms can produce ghost events. For example with your patch softrepeat period is 125ms, release timeout 250ms, and a native rate of 110ms: There are 4 native events transmitted at 000 ms 110 ms 220 ms 330 ms (user stops between 330ms and 440ms) This causes these events in the evdev interface: 000: 1 125: 2 250: 2 375: 2 500: 2 550: 0 So we got 1-2 ghost repeat events. Or maybe just a module option that causes rc-core to use native repeat events, for those of us that want accurate repeat events without ghosting? If the user already knows about the possibility to generate ghost effects, with low delays, he can simply not pass a bad value to the kernel, instead of forcing a modprobe parameter that will limit the minimal value. There is no good value for REP_PERIOD (as in ghost repeats guaranteed gone like with native repeats). Sufficiently large values will make ghost repeats increasingly rare, but the period becomes so long the autorepeat becomes frustratingly slow to use. -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On Thu, May 12, 2011 at 03:36:47AM +0200, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. - Subject: Use a more consistent value for RC repeat period From: Mauro Carvalho Chehab mche...@redhat.com The default REP_PERIOD is 33 ms. This doesn't make sense for IR's, as, in general, an IR repeat scancode is provided at every 110/115ms, depending on the RC protocol. So, increase its default, to do a better job avoiding ghost repeat events. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index f53f9c6..ee67169 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -1044,6 +1044,13 @@ int rc_register_device(struct rc_dev *dev) */ dev-input_dev-rep[REP_DELAY] = 500; + /* + * As a repeat event on protocols like RC-5 and NEC take as long as + * 110/114ms, using 33ms as a repeat period is not the right thing + * to do. + */ + dev-input_dev-rep[REP_PERIOD] = 125; + path = kobject_get_path(dev-dev.kobj, GFP_KERNEL); printk(KERN_INFO %s: %s as %s\n, dev_name(dev-dev), so if I get this right, a XkbSetControls(.. XkbRepeatKeysMask ...) by a client to set the repeat rate would provide the same solution - for those clients/devices affected. The interesting question is how clients would identify the devices that are affected by this (other than trial and error). Cheers, Peter -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
Peter Hutterer wrote: On Thu, May 12, 2011 at 03:36:47AM +0200, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. - Subject: Use a more consistent value for RC repeat period From: Mauro Carvalho Chehabmche...@redhat.com The default REP_PERIOD is 33 ms. This doesn't make sense for IR's, as, in general, an IR repeat scancode is provided at every 110/115ms, depending on the RC protocol. So, increase its default, to do a better job avoiding ghost repeat events. Signed-off-by: Mauro Carvalho Chehabmche...@redhat.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index f53f9c6..ee67169 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -1044,6 +1044,13 @@ int rc_register_device(struct rc_dev *dev) */ dev-input_dev-rep[REP_DELAY] = 500; + /* +* As a repeat event on protocols like RC-5 and NEC take as long as +* 110/114ms, using 33ms as a repeat period is not the right thing +* to do. +*/ + dev-input_dev-rep[REP_PERIOD] = 125; + path = kobject_get_path(dev-dev.kobj, GFP_KERNEL); printk(KERN_INFO %s: %s as %s\n, dev_name(dev-dev), so if I get this right, a XkbSetControls(.. XkbRepeatKeysMask ...) by a client to set the repeat rate would provide the same solution - for those clients/devices affected. The interesting question is how clients would identify the devices that are affected by this (other than trial and error). ir-keytable in v4l-utils is able to identify rc event devices by way of prodding in /sys/class/rc/, but I'm assuming that means every client would have to grow insight into how to do the same. -- Jarod Wilson ja...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On 12.05.2011 04:36, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. It will indeed reduce the amount of ghost events so it brings us in the right direction. I'd still like to get rid of the ghost repeats entirely, or at least some way for users to do it if we don't do it by default. Maybe we could replace the kernel softrepeat with native repeats (for those protocols/drivers that have them), while making sure that repeat events before REP_DELAY are ignored and repeat events less than REP_PERIOD since the previous event are ignored, so the users can still configure them as they like? Or maybe just a module option that causes rc-core to use native repeat events, for those of us that want accurate repeat events without ghosting? - Subject: Use a more consistent value for RC repeat period From: Mauro Carvalho Chehab mche...@redhat.com The default REP_PERIOD is 33 ms. This doesn't make sense for IR's, as, in general, an IR repeat scancode is provided at every 110/115ms, depending on the RC protocol. So, increase its default, to do a better job avoiding ghost repeat events. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index f53f9c6..ee67169 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -1044,6 +1044,13 @@ int rc_register_device(struct rc_dev *dev) */ dev-input_dev-rep[REP_DELAY] = 500; + /* + * As a repeat event on protocols like RC-5 and NEC take as long as + * 110/114ms, using 33ms as a repeat period is not the right thing + * to do. + */ + dev-input_dev-rep[REP_PERIOD] = 125; + path = kobject_get_path(dev-dev.kobj, GFP_KERNEL); printk(KERN_INFO %s: %s as %s\n, dev_name(dev-dev), -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
Hi Anssi/Peter, Em 10-05-2011 15:43, Anssi Hannula escreveu: On 10.05.2011 08:30, Peter Hutterer wrote: On Tue, May 10, 2011 at 08:14:30AM +0300, Anssi Hannula wrote: On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. While 500/33 is good for keyboards, this is generally not good for remote controllers. The bit rate for IR transmissions are slow. So, one keypress can last up to about 110 ms[1]. That means that the maximum repeat rate for IR devices with such protocol should be bellow than 10 keystrokes/sec. Also, the minimum initial delay for IR needs to be different on a few hardware that have a broken IR implementation. We default it to 500ms, but a few drivers change it to fit into some hardware constraits. So, a few kernel driver have some tweaks of repeat times, to be sure that the device will work properly. [1] http://www.sbprojects.com/knowledge/ir/nec.htm It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. The repeat events always generated troubles, as it basically depends on how the hardware actually handles it. Some hardware decoders and some protocols support repeat events, while others don't. There are even some remote controllers that, instead of generating repeat codes, they just generate multiple keypresses. With the rc-core, we've unified the repeat treatment (yet, there are some exceptions to the default way, for some devices where that uses broken hardware decoders). right. we used to have hardware repeats in X a few releases back. I think 1.6 was the first one that shifted to pure software autorepeat. One of the results we saw in the transition period was the clash of hw autorepeat (in X's input system, anything that comes out of the kernel counts as hw) and software repeat. Integrating them back in is going to be a bit iffy, especially since you need the integration with XKB on each device, essentially disallowing the clients from enabling autorepeat. Not 100% what's required there. The evtev part is going to be the simplest part of all that. I suspected it might be tricky. So maybe (at least for the time being) remote controls in X should simply get KeyRelease immediately after every KeyPress? This will probably cause some hurt. Things like volume control only work nice on userspace if repeat events are properly handled. I think we should try to fix XKB/evdev to not use software events on remote controllers. It is easy to detect that an input device is a remote controller on evdev. I wrote a patch for it some time ago (unfortunately, hadn't time to finish it, as I got some jobs with higher priority). Peter, is that a way to pass a flag to XKB to say that a hw input device is not a keyboard, and need a different treatment for repeat events? Meaning that either a) kernel does it (while maybe providing some new extra info for those evdev users that want to distinguish repeats from new keypresses - original suggestion 4), or b) kernel
Re: IR remote control autorepeat / evdev
On 11.05.2011 07:46, Mauro Carvalho Chehab wrote: Hi Anssi/Peter, Hi! Em 10-05-2011 15:43, Anssi Hannula escreveu: On 10.05.2011 08:30, Peter Hutterer wrote: On Tue, May 10, 2011 at 08:14:30AM +0300, Anssi Hannula wrote: On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. While 500/33 is good for keyboards, this is generally not good for remote controllers. 500/33 is the one kernel uses for remotes. The bit rate for IR transmissions are slow. So, one keypress can last up to about 110 ms[1]. That means that the maximum repeat rate for IR devices with such protocol should be bellow than 10 keystrokes/sec. Also, the minimum initial delay for IR needs to be different on a few hardware that have a broken IR implementation. We default it to 500ms, but a few drivers change it to fit into some hardware constraits. So, a few kernel driver have some tweaks of repeat times, to be sure that the device will work properly. [1] http://www.sbprojects.com/knowledge/ir/nec.htm It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. The repeat events always generated troubles, as it basically depends on how the hardware actually handles it. Some hardware decoders and some protocols support repeat events, while others don't. There are even some remote controllers that, instead of generating repeat codes, they just generate multiple keypresses. With the rc-core, we've unified the repeat treatment (yet, there are some exceptions to the default way, for some devices where that uses broken hardware decoders). right. we used to have hardware repeats in X a few releases back. I think 1.6 was the first one that shifted to pure software autorepeat. One of the results we saw in the transition period was the clash of hw autorepeat (in X's input system, anything that comes out of the kernel counts as hw) and software repeat. Integrating them back in is going to be a bit iffy, especially since you need the integration with XKB on each device, essentially disallowing the clients from enabling autorepeat. Not 100% what's required there. The evtev part is going to be the simplest part of all that. I suspected it might be tricky. So maybe (at least for the time being) remote controls in X should simply get KeyRelease immediately after every KeyPress? This will probably cause some hurt. Things like volume control only work nice on userspace if repeat events are properly handled. Hmm, maybe I'm missing something, but I was thinking that sending KeyRelease immediately *fixes* volume control. Currently (both on X and on standalone evdev programs): 1. User holds down volumeup for 4 notches, then releases it. 2. The (kernel or X.org) autorepeat keeps sending events for 250ms after releases at rate of 33ms, so 250/33 = 7 additional events get transmitted. = Volume goes up 11 notches instead of 4. So the volume control is basically unusable if you hold down
Re: IR remote control autorepeat / evdev
Em 11-05-2011 18:33, Anssi Hannula escreveu: On 11.05.2011 07:46, Mauro Carvalho Chehab wrote: Hi Anssi/Peter, Hi! Em 10-05-2011 15:43, Anssi Hannula escreveu: On 10.05.2011 08:30, Peter Hutterer wrote: On Tue, May 10, 2011 at 08:14:30AM +0300, Anssi Hannula wrote: On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. While 500/33 is good for keyboards, this is generally not good for remote controllers. 500/33 is the one kernel uses for remotes. No. It actually depends on what driver you're using. For example, for most dvb-usb devices, this is given by the logic bellow: if (d-props.rc.legacy.rc_interval 40) d-props.rc.legacy.rc_interval = 100; /* default */ input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; input_dev-rep[REP_DELAY] = d-props.rc.legacy.rc_interval + 150; where the rc_interval defined by device entry at the dvb usb drivers. The bit rate for IR transmissions are slow. So, one keypress can last up to about 110 ms[1]. That means that the maximum repeat rate for IR devices with such protocol should be bellow than 10 keystrokes/sec. Also, the minimum initial delay for IR needs to be different on a few hardware that have a broken IR implementation. We default it to 500ms, but a few drivers change it to fit into some hardware constraits. So, a few kernel driver have some tweaks of repeat times, to be sure that the device will work properly. [1] http://www.sbprojects.com/knowledge/ir/nec.htm It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. The repeat events always generated troubles, as it basically depends on how the hardware actually handles it. Some hardware decoders and some protocols support repeat events, while others don't. There are even some remote controllers that, instead of generating repeat codes, they just generate multiple keypresses. With the rc-core, we've unified the repeat treatment (yet, there are some exceptions to the default way, for some devices where that uses broken hardware decoders). right. we used to have hardware repeats in X a few releases back. I think 1.6 was the first one that shifted to pure software autorepeat. One of the results we saw in the transition period was the clash of hw autorepeat (in X's input system, anything that comes out of the kernel counts as hw) and software repeat. Integrating them back in is going to be a bit iffy, especially since you need the integration with XKB on each device, essentially disallowing the clients from enabling autorepeat. Not 100% what's required there. The evtev part is going to be the simplest part of all that. I suspected it might be tricky. So maybe (at least for the time being) remote controls in X should simply get KeyRelease immediately after every KeyPress? This will probably cause some hurt. Things like volume control only work nice on userspace if repeat events are
Re: IR remote control autorepeat / evdev
On 11.05.2011 19:51, Mauro Carvalho Chehab wrote: Em 11-05-2011 18:33, Anssi Hannula escreveu: On 11.05.2011 07:46, Mauro Carvalho Chehab wrote: Hi Anssi/Peter, Hi! Em 10-05-2011 15:43, Anssi Hannula escreveu: On 10.05.2011 08:30, Peter Hutterer wrote: On Tue, May 10, 2011 at 08:14:30AM +0300, Anssi Hannula wrote: On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. While 500/33 is good for keyboards, this is generally not good for remote controllers. 500/33 is the one kernel uses for remotes. No. It actually depends on what driver you're using. For example, for most dvb-usb devices, this is given by the logic bellow: if (d-props.rc.legacy.rc_interval 40) d-props.rc.legacy.rc_interval = 100; /* default */ input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; input_dev-rep[REP_DELAY] = d-props.rc.legacy.rc_interval + 150; where the rc_interval defined by device entry at the dvb usb drivers. Isn't that function only called for DVB_RC_LEGACY mode? Maybe I wasn't clear, but I'm talking only about the devices handled by rc-core. The bit rate for IR transmissions are slow. So, one keypress can last up to about 110 ms[1]. That means that the maximum repeat rate for IR devices with such protocol should be bellow than 10 keystrokes/sec. Also, the minimum initial delay for IR needs to be different on a few hardware that have a broken IR implementation. We default it to 500ms, but a few drivers change it to fit into some hardware constraits. So, a few kernel driver have some tweaks of repeat times, to be sure that the device will work properly. [1] http://www.sbprojects.com/knowledge/ir/nec.htm It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. The repeat events always generated troubles, as it basically depends on how the hardware actually handles it. Some hardware decoders and some protocols support repeat events, while others don't. There are even some remote controllers that, instead of generating repeat codes, they just generate multiple keypresses. With the rc-core, we've unified the repeat treatment (yet, there are some exceptions to the default way, for some devices where that uses broken hardware decoders). right. we used to have hardware repeats in X a few releases back. I think 1.6 was the first one that shifted to pure software autorepeat. One of the results we saw in the transition period was the clash of hw autorepeat (in X's input system, anything that comes out of the kernel counts as hw) and software repeat. Integrating them back in is going to be a bit iffy, especially since you need the integration with XKB on each device, essentially disallowing the clients from enabling autorepeat. Not 100% what's required there. The evtev part is going to be the simplest part of all that. I suspected it might be tricky. So maybe (at least for the time being) remote
Re: IR remote control autorepeat / evdev
On Wed, May 11, 2011 at 08:59:16PM +0300, Anssi Hannula wrote: I meant replacing the softrepeat with native repeat for such devices that have native repeats but no native release events: - keypress from device = keydown + keyup - repeat from device = keydown + keyup - repeat from device = keydown + keyup This is what e.g. ati_remote driver now does. Or alternatively - keypress from device = keydown - repeat from device = repeat - repeat from device = repeat - nothing for 250ms = keyup (doing it this way requires some extra handling in X server to stop its softrepeat from triggering, though, as previously noted) With either of these, if one holds down volumeup, the repeat works, and stops volumeup'ing immediately when user releases the button (as it is supposed to). Unfortunately this does not work for devices that do not have hardware autorepeat and also stops users from adjusting autorepeat parameters to their liking. It appears that the delay to check whether the key has been released is too long (almost order of magnitude longer than our typical autorepeat period). I think we should increase the period for remotes (both in kernel and in X, and also see if the release check delay can be made shorter, like 50-100 ms. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
Em 11-05-2011 19:59, Anssi Hannula escreveu: No. It actually depends on what driver you're using. For example, for most dvb-usb devices, this is given by the logic bellow: if (d-props.rc.legacy.rc_interval 40) d-props.rc.legacy.rc_interval = 100; /* default */ input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; input_dev-rep[REP_DELAY] = d-props.rc.legacy.rc_interval + 150; where the rc_interval defined by device entry at the dvb usb drivers. Isn't that function only called for DVB_RC_LEGACY mode? I just picked one random piece of the code that covers several RC remotes (most dvb-usb drivers are still using the legacy mode). Similar code are there for other devices. Maybe I wasn't clear, but I'm talking only about the devices handled by rc-core. With just a few exceptions, the repeat period/delay that were there before porting to rc-core were maintained. There are space for adjustments, as we did on a few cases. Em 11-05-2011 22:53, Dmitry Torokhov escreveu: On Wed, May 11, 2011 at 08:59:16PM +0300, Anssi Hannula wrote: I meant replacing the softrepeat with native repeat for such devices that have native repeats but no native release events: - keypress from device = keydown + keyup - repeat from device = keydown + keyup - repeat from device = keydown + keyup This is what e.g. ati_remote driver now does. Or alternatively - keypress from device = keydown - repeat from device = repeat - repeat from device = repeat - nothing for 250ms = keyup (doing it this way requires some extra handling in X server to stop its softrepeat from triggering, though, as previously noted) With either of these, if one holds down volumeup, the repeat works, and stops volumeup'ing immediately when user releases the button (as it is supposed to). Unfortunately this does not work for devices that do not have hardware autorepeat and also stops users from adjusting autorepeat parameters to their liking. Yes. A solution like the above would limit the usage. There are some remotes (like for example, the Hauppauge Grey remotes I have here) that a simple keypress generates, in general, up to 3 scancodes (the normal keypress and up to two bounced repeat keycodes). So, the software key delay also serves as a way to de-bounce the keypress. It appears that the delay to check whether the key has been released is too long (almost order of magnitude longer than our typical autorepeat period). Yes, because, for example, with NEC and RC-5 protocols, one keystroke or one repeat event takes 110/114 ms to be transmitted. The delay actually varies from protocol to protocol, so it is possible to do some adjustments, based on the protocol type, but it is an order of magnitude longer than a keyboard or mouse. I think we should increase the period for remotes (both in kernel and in X, and also see if the release check delay can be made shorter, like 50-100 ms. Some adjustments like that can be made, but they are device-driver specific. For example, some in-hardware IR decoders with KS007 micro-controller just removes all repeat keycodes and replace them with new keystrokes. There are some shipped remotes that don't support the RC-5 or NEC key repeat event. So, on those, if you keep a key pressed, you just receive the same scancode several times. The last time I double checked all remotes I have here, on all cases the auto-repeat logic were doing the right job, but I won't doubt that we need to add some additional adjustments for some boards/devices. Anssi, what's the hardware that you're using? Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On 11.05.2011 23:53, Dmitry Torokhov wrote: On Wed, May 11, 2011 at 08:59:16PM +0300, Anssi Hannula wrote: I meant replacing the softrepeat with native repeat for such devices that have native repeats but no native release events: - keypress from device = keydown + keyup - repeat from device = keydown + keyup - repeat from device = keydown + keyup This is what e.g. ati_remote driver now does. Or alternatively - keypress from device = keydown - repeat from device = repeat - repeat from device = repeat - nothing for 250ms = keyup (doing it this way requires some extra handling in X server to stop its softrepeat from triggering, though, as previously noted) With either of these, if one holds down volumeup, the repeat works, and stops volumeup'ing immediately when user releases the button (as it is supposed to). Unfortunately this does not work for devices that do not have hardware autorepeat Devices that have no hardware autorepeat have hardware release events, no? I'm only suggesting to do this for devices with hardware autorepeat. If there are no hw repeat events and no hw release events, obviously making repeat work at all is impossible. and also stops users from adjusting autorepeat parameters to their liking. True. However, I don't think adjustable autorepeat parameters are much of use for the users if the autorepeat itself is unusable (due to ghost repeats). It appears that the delay to check whether the key has been released is too long (almost order of magnitude longer than our typical autorepeat period). I think we should increase the period for remotes (both in kernel and in X, and also see if the release check delay can be made shorter, like 50-100 ms. To make the ghost repeat issue disappear, one would have to use a release timeout of just over the native repeat rate of the remote, and a softrepeat period of just over the release timeout, right? This will make the repeat rate slower than the native repeats. I'm not 100% sure if that is an issue, but I'd guess there might be some devices that already have a slowish repeat rate, where we wouldn't want to add such additional delay. (plus there is the issue of having to fiddle the rates for every device/protocol) -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On 12.05.2011 02:52, Mauro Carvalho Chehab wrote: Em 11-05-2011 19:59, Anssi Hannula escreveu: No. It actually depends on what driver you're using. For example, for most dvb-usb devices, this is given by the logic bellow: if (d-props.rc.legacy.rc_interval 40) d-props.rc.legacy.rc_interval = 100; /* default */ input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; input_dev-rep[REP_DELAY] = d-props.rc.legacy.rc_interval + 150; where the rc_interval defined by device entry at the dvb usb drivers. Isn't that function only called for DVB_RC_LEGACY mode? I just picked one random piece of the code that covers several RC remotes (most dvb-usb drivers are still using the legacy mode). Similar code are there for other devices. I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Maybe I wasn't clear, but I'm talking only about the devices handled by rc-core. With just a few exceptions, the repeat period/delay that were there before porting to rc-core were maintained. There are space for adjustments, as we did on a few cases. The above is the only place where the repeat period is set in the drivers/media tree, and it is not an rc-core device. Other devices therefore use the 33ms kernel default. Maybe I am missing something? Em 11-05-2011 22:53, Dmitry Torokhov escreveu: On Wed, May 11, 2011 at 08:59:16PM +0300, Anssi Hannula wrote: I meant replacing the softrepeat with native repeat for such devices that have native repeats but no native release events: - keypress from device = keydown + keyup - repeat from device = keydown + keyup - repeat from device = keydown + keyup This is what e.g. ati_remote driver now does. Or alternatively - keypress from device = keydown - repeat from device = repeat - repeat from device = repeat - nothing for 250ms = keyup (doing it this way requires some extra handling in X server to stop its softrepeat from triggering, though, as previously noted) With either of these, if one holds down volumeup, the repeat works, and stops volumeup'ing immediately when user releases the button (as it is supposed to). Unfortunately this does not work for devices that do not have hardware autorepeat and also stops users from adjusting autorepeat parameters to their liking. Yes. A solution like the above would limit the usage. There are some remotes (like for example, the Hauppauge Grey remotes I have here) that a simple keypress generates, in general, up to 3 scancodes (the normal keypress and up to two bounced repeat keycodes). So, the software key delay also serves as a way to de-bounce the keypress. I probably forgot to mention it, but I'm not suggesting removal of the repetition delay (500ms), it can stay for this reason exactly. It appears that the delay to check whether the key has been released is too long (almost order of magnitude longer than our typical autorepeat period). Yes, because, for example, with NEC and RC-5 protocols, one keystroke or one repeat event takes 110/114 ms to be transmitted. The delay actually varies from protocol to protocol, so it is possible to do some adjustments, based on the protocol type, but it is an order of magnitude longer than a keyboard or mouse. I think we should increase the period for remotes (both in kernel and in X, and also see if the release check delay can be made shorter, like 50-100 ms. Some adjustments like that can be made, but they are device-driver specific. For example, some in-hardware IR decoders with KS007 micro-controller just removes all repeat keycodes and replace them with new keystrokes. There are some shipped remotes that don't support the RC-5 or NEC key repeat event. So, on those, if you keep a key pressed, you just receive the same scancode several times. The last time I double checked all remotes I have here, on all cases the auto-repeat logic were doing the right job, but I won't doubt that we need to add some additional adjustments for some boards/devices. Does doing the right job mean that you are getting zero repeat (2) events after releasing a remote button? Because that is what I expect to happen, and that is what e.g. LIRC (which most people seem to still use with HTPC software - like XBMC which I'm a developer of) does. Anssi, what's the hardware that you're using? I'm using ati_remote ported to rc-core (don't know yet if it makes any sense, though). However, as noted, reading ir-main.c I fail to see why this wouldn't happen with all rc-core devices, as all devices seem to use same IR_KEYPRESS_TIMEOUT and REP_PERIOD (though you seem to suggest otherwise above, maybe you can show me wrong? :) ). -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: IR remote control autorepeat / evdev
Em 12-05-2011 02:17, Anssi Hannula escreveu: On 11.05.2011 23:53, Dmitry Torokhov wrote: On Wed, May 11, 2011 at 08:59:16PM +0300, Anssi Hannula wrote: I meant replacing the softrepeat with native repeat for such devices that have native repeats but no native release events: - keypress from device = keydown + keyup - repeat from device = keydown + keyup - repeat from device = keydown + keyup This is what e.g. ati_remote driver now does. Or alternatively - keypress from device = keydown - repeat from device = repeat - repeat from device = repeat - nothing for 250ms = keyup (doing it this way requires some extra handling in X server to stop its softrepeat from triggering, though, as previously noted) With either of these, if one holds down volumeup, the repeat works, and stops volumeup'ing immediately when user releases the button (as it is supposed to). Unfortunately this does not work for devices that do not have hardware autorepeat Devices that have no hardware autorepeat have hardware release events, no? I'm only suggesting to do this for devices with hardware autorepeat. What I meant to say is that some devices produce: KEYKEYKEY...KEY instead of: KEYREPREP...REP In other words, there's not a solution that fits all. Adjustments need to be done per device, and not as a change at the core. If there are no hw repeat events and no hw release events, obviously making repeat work at all is impossible. and also stops users from adjusting autorepeat parameters to their liking. True. However, I don't think adjustable autorepeat parameters are much of use for the users if the autorepeat itself is unusable (due to ghost repeats). It appears that the delay to check whether the key has been released is too long (almost order of magnitude longer than our typical autorepeat period). I think we should increase the period for remotes (both in kernel and in X, and also see if the release check delay can be made shorter, like 50-100 ms. To make the ghost repeat issue disappear, one would have to use a release timeout of just over the native repeat rate of the remote, and a softrepeat period of just over the release timeout, right? There's no known ghost repeat issue at rc-core. If you're seeing an issue, it is probably device-specific, and should be handle are such. Don't blame the core [1]. On what device(s) are you noticing such ghost repeat, and with what protocols? please provide the rc-core and driver-specific debug information and let's work to fix on the case you've detected. [1] I'm referring to the way input/event is providing the events. If Xorg is discarding the in-kernel repeat logic and using its own logic, then we may have ghost events there, as RC timings are different than keyboards. So a 250/33 would produce a crappy result with RC's. This will make the repeat rate slower than the native repeats. I'm not 100% sure if that is an issue, but I'd guess there might be some devices that already have a slowish repeat rate, where we wouldn't want to add such additional delay. If we use the native repeat, we would have one keystroke on every 110ms, and the initial delay would be also 110ms. On my experiences, while repeating keys close to 110ms is generally ok, 110ms is too short for the initial events, especially due to the debouncing. Depending on the RC device, as I said before, a normal keypress in general produces more than just one event, due to debounce issues. In other words, It really doesn't makes sense to wait for 250ms at a keyboard to start repeat, and to wait for just 110ms to start repeat at IR. The rc-core logic does the right thing here: - a keystroke generates a key down and starts a timer; - a repeat event (or the same keypress) resets the timer; - at timer's timeout, it produces a keyup. This way, the input-event knows when the key was pressed and when the key were released, and can produce repeat events at the right way, provided that the timer periods are compatible with the timings generated by the IR protocol. So, all that it is needed is to adjust the two EV_REP timings (delay and period) and the rc-core timer's timeout to remove the ghost effects. It is possible to control two of the 2 EV_REP timers via userspace. For now, we're using just one timer for the rc-core logic (IR_KEYPRESS_TIMEOUT, currently hardcoded as 250 ms). Such value is a little bigger than 2 x 114 ms, to avoid bouncing effects with RC-5 REPEAT events. Values smaller than 230 ms produced ghost keystrokes on some tests I did at the time such logic were added. (plus there is the issue of having to fiddle the rates for every device/protocol) -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
Em 12-05-2011 02:37, Anssi Hannula escreveu: On 12.05.2011 02:52, Mauro Carvalho Chehab wrote: Em 11-05-2011 19:59, Anssi Hannula escreveu: No. It actually depends on what driver you're using. For example, for most dvb-usb devices, this is given by the logic bellow: if (d-props.rc.legacy.rc_interval 40) d-props.rc.legacy.rc_interval = 100; /* default */ input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; input_dev-rep[REP_DELAY] = d-props.rc.legacy.rc_interval + 150; where the rc_interval defined by device entry at the dvb usb drivers. Isn't that function only called for DVB_RC_LEGACY mode? I just picked one random piece of the code that covers several RC remotes (most dvb-usb drivers are still using the legacy mode). Similar code are there for other devices. I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. The REP_PERIOD is adjusted, however, on several drivers. Also, RC core changes its default to 500ms, to avoid ghost keystrokes: dev-input_dev-rep[REP_DELAY] = 500; Maybe I wasn't clear, but I'm talking only about the devices handled by rc-core. With just a few exceptions, the repeat period/delay that were there before porting to rc-core were maintained. There are space for adjustments, as we did on a few cases. The above is the only place where the repeat period is set in the drivers/media tree, and it is not an rc-core device. Other devices therefore use the 33ms kernel default. Maybe I am missing something? Em 11-05-2011 22:53, Dmitry Torokhov escreveu: On Wed, May 11, 2011 at 08:59:16PM +0300, Anssi Hannula wrote: I meant replacing the softrepeat with native repeat for such devices that have native repeats but no native release events: - keypress from device = keydown + keyup - repeat from device = keydown + keyup - repeat from device = keydown + keyup This is what e.g. ati_remote driver now does. Or alternatively - keypress from device = keydown - repeat from device = repeat - repeat from device = repeat - nothing for 250ms = keyup (doing it this way requires some extra handling in X server to stop its softrepeat from triggering, though, as previously noted) With either of these, if one holds down volumeup, the repeat works, and stops volumeup'ing immediately when user releases the button (as it is supposed to). Unfortunately this does not work for devices that do not have hardware autorepeat and also stops users from adjusting autorepeat parameters to their liking. Yes. A solution like the above would limit the usage. There are some remotes (like for example, the Hauppauge Grey remotes I have here) that a simple keypress generates, in general, up to 3 scancodes (the normal keypress and up to two bounced repeat keycodes). So, the software key delay also serves as a way to de-bounce the keypress. I probably forgot to mention it, but I'm not suggesting removal of the repetition delay (500ms), it can stay for this reason exactly. It appears that the delay to check whether the key has been released is too long (almost order of magnitude longer than our typical autorepeat period). Yes, because, for example, with NEC and RC-5 protocols, one keystroke or one repeat event takes 110/114 ms to be transmitted. The delay actually varies from protocol to protocol, so it is possible to do some adjustments, based on the protocol type, but it is an order of magnitude longer than a keyboard or mouse. I think we should increase the period for remotes (both in kernel and in X, and also see if the release check delay can be made shorter, like 50-100 ms. Some adjustments like that can be made, but they are device-driver specific. For example, some in-hardware IR decoders with KS007 micro-controller just removes all repeat keycodes and replace them with new keystrokes. There are some shipped remotes that don't support the RC-5 or NEC key repeat event. So, on those, if you keep a key pressed, you just receive the same scancode several times. The last time I double checked all remotes I have here, on all cases the auto-repeat logic were doing the right job, but I won't doubt that we need to add some additional adjustments for some boards/devices. Does doing the right job mean that you are getting zero repeat (2) events after releasing a remote button? I mean that the logic is ok. The timings may be not. The timings were not touched when we've ported the already supported IR's to the rc-core, except when we noticed some troubles with them. Because that is what I expect to happen, and that is what e.g. LIRC (which most people seem to still use with HTPC software
Re: IR remote control autorepeat / evdev
Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. - Subject: Use a more consistent value for RC repeat period From: Mauro Carvalho Chehab mche...@redhat.com The default REP_PERIOD is 33 ms. This doesn't make sense for IR's, as, in general, an IR repeat scancode is provided at every 110/115ms, depending on the RC protocol. So, increase its default, to do a better job avoiding ghost repeat events. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c index f53f9c6..ee67169 100644 --- a/drivers/media/rc/rc-main.c +++ b/drivers/media/rc/rc-main.c @@ -1044,6 +1044,13 @@ int rc_register_device(struct rc_dev *dev) */ dev-input_dev-rep[REP_DELAY] = 500; + /* +* As a repeat event on protocols like RC-5 and NEC take as long as +* 110/114ms, using 33ms as a repeat period is not the right thing +* to do. +*/ + dev-input_dev-rep[REP_PERIOD] = 125; + path = kobject_get_path(dev-dev.kobj, GFP_KERNEL); printk(KERN_INFO %s: %s as %s\n, dev_name(dev-dev), -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On May 11, 2011, at 9:36 PM, Mauro Carvalho Chehab wrote: Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu: Em 12-05-2011 02:37, Anssi Hannula escreveu: I don't see any other places: $ git grep 'REP_PERIOD' . dvb/dvb-usb/dvb-usb-remote.c: input_dev-rep[REP_PERIOD] = d-props.rc.legacy.rc_interval; Indeed, the REP_PERIOD is not adjusted on other drivers. I agree that we should change it to something like 125ms, for example, as 33ms is too short, as it takes up to 114ms for a repeat event to arrive. IMO, the enclosed patch should do a better job with repeat events, without needing to change rc-core/input/event logic. - Subject: Use a more consistent value for RC repeat period From: Mauro Carvalho Chehab mche...@redhat.com The default REP_PERIOD is 33 ms. This doesn't make sense for IR's, as, in general, an IR repeat scancode is provided at every 110/115ms, depending on the RC protocol. So, increase its default, to do a better job avoiding ghost repeat events. Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com Yeah, I definitely like this change, and I think it should do some good. I've been pointing a number of people at ir-keytable and its ability to tweak these values from userspace. Most people have been bumping REP_PERIOD up a bit, but also REP_DELAY down a bit, so that repeats actually kick in a bit sooner. I'm fine with leaving 500 as the default there though, and letting individual drivers adjust it if they really want. Acked-by: Jarod Wilson ja...@redhat.com -- Jarod Wilson ja...@wilsonet.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On 10.05.2011 08:30, Peter Hutterer wrote: On Tue, May 10, 2011 at 08:14:30AM +0300, Anssi Hannula wrote: On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. right. we used to have hardware repeats in X a few releases back. I think 1.6 was the first one that shifted to pure software autorepeat. One of the results we saw in the transition period was the clash of hw autorepeat (in X's input system, anything that comes out of the kernel counts as hw) and software repeat. Integrating them back in is going to be a bit iffy, especially since you need the integration with XKB on each device, essentially disallowing the clients from enabling autorepeat. Not 100% what's required there. The evtev part is going to be the simplest part of all that. I suspected it might be tricky. So maybe (at least for the time being) remote controls in X should simply get KeyRelease immediately after every KeyPress? Meaning that either a) kernel does it (while maybe providing some new extra info for those evdev users that want to distinguish repeats from new keypresses - original suggestion 4), or b) kernel provides a flag which causes the X evdev driver to follow-up every keydown/repeat event with an immediate release event. (both of these include kernel changed to use native repeats instead of softrepeats, which is trivial) Now, IMO something should be done to fix this. But what exactly? Here are two ideas that would remove these ghost repeats: 1. Do not provide any repeat/release simulation in the kernel for RC devices (by default?), just provide both keydown and immediate release events for every native keypress or repeat received from the device. + Very simple to implement - We lose the ability to track repeats, i.e. if a new event was a repeat or a new keypress; holding down a key becomes impossible or 2. Replace kernel autorepeat simulation by passing through the native repeat events (probably filtering them according to REP_DELAY and REP_PERIOD), and have a device property bit (fetchable via EVIOCGPROP) indicating that the keyrelease is simulated, and have the X server use the native repeats instead of softrepeats for such a device. + The userspace correctly gets repeat events tagged as repeats and release events when appropriate (albeit a little late) - Adds complexity. Also, while the kernel part is quite easy to implement, I'm not sure if the X server part is. or 3. Same as 1., but indicate the repeatness of an event with a new additional special event before EV_SYN (sync event). + Simple to implement - Quite hacky, and userspace still can't guess from initial keypress/release if the key is still pressed down or not. 4. Same as 1., but have a new EV_RC with RC_KEYDOWN and RC_KEYUP events, with RC_KEYDOWN sent when a key is pressed down a first time along with the normal EV_KEY event, and RC_KEYUP
Re: IR remote control autorepeat / evdev
On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. Cheers, Peter Now, IMO something should be done to fix this. But what exactly? Here are two ideas that would remove these ghost repeats: 1. Do not provide any repeat/release simulation in the kernel for RC devices (by default?), just provide both keydown and immediate release events for every native keypress or repeat received from the device. + Very simple to implement - We lose the ability to track repeats, i.e. if a new event was a repeat or a new keypress; holding down a key becomes impossible or 2. Replace kernel autorepeat simulation by passing through the native repeat events (probably filtering them according to REP_DELAY and REP_PERIOD), and have a device property bit (fetchable via EVIOCGPROP) indicating that the keyrelease is simulated, and have the X server use the native repeats instead of softrepeats for such a device. + The userspace correctly gets repeat events tagged as repeats and release events when appropriate (albeit a little late) - Adds complexity. Also, while the kernel part is quite easy to implement, I'm not sure if the X server part is. or 3. Same as 1., but indicate the repeatness of an event with a new additional special event before EV_SYN (sync event). + Simple to implement - Quite hacky, and userspace still can't guess from initial keypress/release if the key is still pressed down or not. 4. Same as 1., but have a new EV_RC with RC_KEYDOWN and RC_KEYUP events, with RC_KEYDOWN sent when a key is pressed down a first time along with the normal EV_KEY event, and RC_KEYUP sent when the key is surely released (e.g. 250ms without native repeat events or another key got pressed, i.e. like the simulated keyup now). + Simple to implement, works as expected with most userspace apps with no changes to them; and if an app wants to know the repeatness of an event or held-down-ness of a key, it can do that. - Repeatness of the event is hidden behind a new API. What do you think? Or any other ideas? 2 and 4 seem nicest to me. (I don't know how feasible 2 would be on X server side, though) -- Anssi Hannula ___ xorg-de...@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. Cheers, Peter Now, IMO something should be done to fix this. But what exactly? Here are two ideas that would remove these ghost repeats: 1. Do not provide any repeat/release simulation in the kernel for RC devices (by default?), just provide both keydown and immediate release events for every native keypress or repeat received from the device. + Very simple to implement - We lose the ability to track repeats, i.e. if a new event was a repeat or a new keypress; holding down a key becomes impossible or 2. Replace kernel autorepeat simulation by passing through the native repeat events (probably filtering them according to REP_DELAY and REP_PERIOD), and have a device property bit (fetchable via EVIOCGPROP) indicating that the keyrelease is simulated, and have the X server use the native repeats instead of softrepeats for such a device. + The userspace correctly gets repeat events tagged as repeats and release events when appropriate (albeit a little late) - Adds complexity. Also, while the kernel part is quite easy to implement, I'm not sure if the X server part is. or 3. Same as 1., but indicate the repeatness of an event with a new additional special event before EV_SYN (sync event). + Simple to implement - Quite hacky, and userspace still can't guess from initial keypress/release if the key is still pressed down or not. 4. Same as 1., but have a new EV_RC with RC_KEYDOWN and RC_KEYUP events, with RC_KEYDOWN sent when a key is pressed down a first time along with the normal EV_KEY event, and RC_KEYUP sent when the key is surely released (e.g. 250ms without native repeat events or another key got pressed, i.e. like the simulated keyup now). + Simple to implement, works as expected with most userspace apps with no changes to them; and if an app wants to know the repeatness of an event or held-down-ness of a key, it can do that. - Repeatness of the event is hidden behind a new API. What do you think? Or any other ideas? 2 and 4 seem nicest to me. (I don't know how feasible 2 would be on X server side, though) -- Anssi Hannula ___ xorg-de...@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: IR remote control autorepeat / evdev
On Tue, May 10, 2011 at 08:14:30AM +0300, Anssi Hannula wrote: On 10.05.2011 07:11, Peter Hutterer wrote: On Sun, May 08, 2011 at 07:38:00AM +0300, Anssi Hannula wrote: Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. I got a bit confused reading this description. Does this mean that remotes usually send: key press - repeat - repeat - ... - repeat - silence where the silence indicates that the key has been released? Which the kernel after 250ms translates into a release event. And the kernel discards the repeats and generates it's own on 500/33? Do I get this right so far? Yes. If so, I'm not sure how to avoid the 250ms delay since we have no indication from the hardware when the silence will stop, right? Yes. AFAICS what we need is to not use softrepeat for these devices and instead use the native repeats. The 250ms release delay could then be kept (as it wouldn't cause unwanted repeats anymore) or it could be made 0ms if that is deemed better. I listed some ways to do that below in my original post. Note that the repeat delay and ratio are configurable per-device using XKB, so you could set up the 500/33 in X too. It wouldn't make any difference with the actual issue which is autorepeat happening after physical key released. I guess the reason this hasn't come up earlier is that the unified IR/RC subsystem in the linux kernel is still quite new. It definitely needs to be improved regarding this issue - just trying to figure out the best way to do it. right. we used to have hardware repeats in X a few releases back. I think 1.6 was the first one that shifted to pure software autorepeat. One of the results we saw in the transition period was the clash of hw autorepeat (in X's input system, anything that comes out of the kernel counts as hw) and software repeat. Integrating them back in is going to be a bit iffy, especially since you need the integration with XKB on each device, essentially disallowing the clients from enabling autorepeat. Not 100% what's required there. The evtev part is going to be the simplest part of all that. Cheers, Peter Now, IMO something should be done to fix this. But what exactly? Here are two ideas that would remove these ghost repeats: 1. Do not provide any repeat/release simulation in the kernel for RC devices (by default?), just provide both keydown and immediate release events for every native keypress or repeat received from the device. + Very simple to implement - We lose the ability to track repeats, i.e. if a new event was a repeat or a new keypress; holding down a key becomes impossible or 2. Replace kernel autorepeat simulation by passing through the native repeat events (probably filtering them according to REP_DELAY and REP_PERIOD), and have a device property bit (fetchable via EVIOCGPROP) indicating that the keyrelease is simulated, and have the X server use the native repeats instead of softrepeats for such a device. + The userspace correctly gets repeat events tagged as repeats and release events when appropriate (albeit a little late) - Adds complexity. Also, while the kernel part is quite easy to implement, I'm not sure if the X server part is. or 3. Same as 1., but indicate the repeatness of an event with a new additional special event before EV_SYN (sync event). + Simple to implement - Quite hacky, and userspace still can't guess from initial keypress/release if the key is still pressed down or not. 4. Same as 1., but have a new EV_RC with RC_KEYDOWN and RC_KEYUP events, with RC_KEYDOWN sent when a key is pressed down a first time along with the normal EV_KEY event, and RC_KEYUP sent when the key is surely released (e.g. 250ms without native repeat events or another key got pressed, i.e. like the simulated keyup now). + Simple to implement, works as expected with most userspace apps with no changes to them; and if an app wants to know the repeatness of an event or held-down-ness of a key, it can do that. - Repeatness of the event is hidden behind a new API. What do you think? Or any other ideas? 2 and 4 seem nicest to me. (I don't know how feasible 2 would be on X server side,
IR remote control autorepeat / evdev
Hi all! Most IR/RF remotes differ from normal keyboards in that they don't provide release events. They do provide native repeat events, though. Currently the Linux kernel RC/input subsystems provide a simulated autorepeat for remote controls (default delay 500ms, period 33ms), and X.org server ignores these events and generates its own autorepeat for them. The kernel RC subsystem provides a simulated release event when 250ms has passed since the last native event (repeat or non-repeat) was received from the device. This is problematic, since it causes lots of extra repeat events to be always sent (for up to 250ms) after the user has released the remote control button, which makes the remote quite uncomfortable to use. Now, IMO something should be done to fix this. But what exactly? Here are two ideas that would remove these ghost repeats: 1. Do not provide any repeat/release simulation in the kernel for RC devices (by default?), just provide both keydown and immediate release events for every native keypress or repeat received from the device. + Very simple to implement - We lose the ability to track repeats, i.e. if a new event was a repeat or a new keypress; holding down a key becomes impossible or 2. Replace kernel autorepeat simulation by passing through the native repeat events (probably filtering them according to REP_DELAY and REP_PERIOD), and have a device property bit (fetchable via EVIOCGPROP) indicating that the keyrelease is simulated, and have the X server use the native repeats instead of softrepeats for such a device. + The userspace correctly gets repeat events tagged as repeats and release events when appropriate (albeit a little late) - Adds complexity. Also, while the kernel part is quite easy to implement, I'm not sure if the X server part is. or 3. Same as 1., but indicate the repeatness of an event with a new additional special event before EV_SYN (sync event). + Simple to implement - Quite hacky, and userspace still can't guess from initial keypress/release if the key is still pressed down or not. 4. Same as 1., but have a new EV_RC with RC_KEYDOWN and RC_KEYUP events, with RC_KEYDOWN sent when a key is pressed down a first time along with the normal EV_KEY event, and RC_KEYUP sent when the key is surely released (e.g. 250ms without native repeat events or another key got pressed, i.e. like the simulated keyup now). + Simple to implement, works as expected with most userspace apps with no changes to them; and if an app wants to know the repeatness of an event or held-down-ness of a key, it can do that. - Repeatness of the event is hidden behind a new API. What do you think? Or any other ideas? 2 and 4 seem nicest to me. (I don't know how feasible 2 would be on X server side, though) -- Anssi Hannula -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
