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 > >> > >> 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 > >> > >> 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
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 > > 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 > > 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
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 - > > 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 > &
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 - 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
