IR remote control autorepeat / evdev

IR remote control autorepeat / evdev

[Anssi Hannula]

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

Re: IR remote control autorepeat / evdev

[Peter Hutterer]

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-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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-devel@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

Re: IR remote control autorepeat / evdev

[Peter Hutterer]

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, though)
> >>
> >> -- 
> >> Anssi Hannula
> >> _______________________________________________
> >> xorg-devel@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

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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 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-devel@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
> 


-- 
Anssi Hannula

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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

The issue seems to be X-specific, so, I think that the solution should be
there, and not at the kernel level. X should not use software autorepeat
for remote controllers, at this won't work properly.

> 
> 
>>>>> 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-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
>>>>> Archives: http://lists.x.org/archives/xorg-devel
>>>>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>>>>
>>>>
>>>
>>>
>>> -- 
>>> Anssi Hannula
>>
> 
> 

_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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 buttons.

By sending KeyRelease immediately after every native event from the
remote, no more additional keypresses happen when the user stops, even
if the remote doesn't send a native release event.


While having the current "unified" repeat treatment (same rate/delay) in
kernel for all remotes is nice, I think the
events-happen-after-keyrelease issue far outweighs any benefits. Hence
my push for native repeats.

> 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 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)
> 
> The issue seems to be X-specific, so, I think that the solution should be
> there, and not at the kernel level. X should not use software autorepeat
> for remote controllers, at this won't work properly.

This affects all other non-X applications as well, as the kernel does
software autorepeat for remote controllers.


>>>>>> 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-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
>>>>>> Archives: http://lists.x.org/archives/xorg-devel
>>>>>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>>>>>
>>>>>
>>>>
>>>>
>>>> -- 
>>>> Anssi Hannula
>>>
>>
>>
> 


-- 
Anssi Hannula
_______________________________________________
xorg-devel-go0+a7rfsptAfugRpC6u6w@public.gmane.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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 properly handled.
> 
> Hmm, maybe I'm missing something, but I was thinking that sending
> KeyRelease immediately *fixes* volume control.

If you disable the repeat logic, you'll get just one key press/release event.

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

As I've explained before, the initial delay and the repeat rate is
defined per device. On most places, the initial delay is 500 ms, and
drivers won't use repeat rates so high as 33ms, as it doesn't make sense,
on several protocols.

> => Volume goes up 11 notches instead of 4.
> So the volume control is basically unusable if you hold down buttons.
> 
> By sending KeyRelease immediately after every native event from the
> remote, no more additional keypresses happen when the user stops, even
> if the remote doesn't send a native release event.
> 
> 
> While having the current "unified" repeat treatment (same rate/delay) in
> kernel for all remotes is nice, I think the
> events-happen-after-keyrelease issue far outweighs any benefits. Hence
> my push for native repeats.

The unified treatment doesn't mean that all drivers use the same values. Also,
this can be set via userspace call. For example, using ir-keytable[1], you can
change the delay to 500 and the period to 100 with:
	$ ir-keytable -D 500 -P 200

[1] available at http://linuxtv.org/git/v4l-utils.git

> 
>> 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 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)
>>
>> The issue seems to be X-specific, so, I think that the solution should be
>> there, and not at the kernel level. X should not use software autorepeat
>> for remote controllers, at this won't work properly.
> 
> This affects all other non-X applications as well, as the kernel does
> software autorepeat for remote controllers.
> 
> 
>>>>>>> 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-devel@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
>>>>
>>>
>>>
>>
> 
> 

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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 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.
> 
> If you disable the repeat logic, you'll get just one key press/release event.

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).

>> 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.
> 
> As I've explained before, the initial delay and the repeat rate is
> defined per device. On most places, the initial delay is 500 ms, and
> drivers won't use repeat rates so high as 33ms, as it doesn't make sense,
> on several protocols.

I'm looking at media_tree/staging/for_v2.6.40 HEAD, and I don't see any
rc-core drivers setting REP_PERIOD, causing the input subsystem default
(33ms) to be used (I also confirmed this by testing with mainline code
before my first post).

Anyway, just fiddling the repeat rate doesn't make the ghost repeat
issue go away (unless you set it over IR_KEYPRESS_TIMEOUT = 250ms which
makes the repeat rate unnecessarily slow).

>> => Volume goes up 11 notches instead of 4.
>> So the volume control is basically unusable if you hold down buttons.
>>
>> By sending KeyRelease immediately after every native event from the
>> remote, no more additional keypresses happen when the user stops, even
>> if the remote doesn't send a native release event.
>>
>>
>> While having the current "unified" repeat treatment (same rate/delay) in
>> kernel for all remotes is nice, I think the
>> events-happen-after-keyrelease issue far outweighs any benefits. Hence
>> my push for native repeats.
> 
> The unified treatment doesn't mean that all drivers use the same values. Also,
> this can be set via userspace call. For example, using ir-keytable[1], you can
> change the delay to 500 and the period to 100 with:
> 	$ ir-keytable -D 500 -P 200
> 
> [1] available at http://linuxtv.org/git/v4l-utils.git

Unfortunately one can't (currently) get repeats working properly with a
simple ir-keytable call (if you have period under 250ms, you get ghost
repeats after keyrelease; if period is over 250ms, the repeat rate is
then a bit too slow).

>>
>>> 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 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)
>>>
>>> The issue seems to be X-specific, so, I think that the solution should be
>>> there, and not at the kernel level. X should not use software autorepeat
>>> for remote controllers, at this won't work properly.
>>
>> This affects all other non-X applications as well, as the kernel does
>> software autorepeat for remote controllers.
>>
>>
>>>>>>>> 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-devel@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
>>>>>
>>>>
>>>>
>>>
>>
>>
> 


-- 
Anssi Hannula

Re: IR remote control autorepeat / evdev

[Dmitry Torokhov]

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

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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.

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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:
	<KEY><KEY><KEY>...<KEY>
instead of:
	<KEY><REP><REP>...<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)
> 

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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 - like XBMC
> which I'm a developer of) does.

That's what we all expect to happen.

>> 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? :) ).

They share the same logic, but hardware decoders behave different than
software ones.

Mauro

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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 <mchehab@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 <mchehab@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),

Re: IR remote control autorepeat / evdev

[Jarod Wilson]

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 <mchehab@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 <mchehab@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 <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com

Re: IR remote control autorepeat / evdev

[Peter Hutterer]

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 <mchehab@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 <mchehab@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

Re: IR remote control autorepeat / evdev

[Jarod Wilson]

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 Chehab<mchehab@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<mchehab@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
jarod@redhat.com

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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 <mchehab@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 <mchehab@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

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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 <mchehab@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 <mchehab@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 0000000..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 <mchehab@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;
+    struct UEvents *next;
+};
+
+struct SysfsNames  {
+    char              *name;
+    struct SysfsNames *next;
+};
+
+struct RCDevice {
+    char *SysfsName;    /* Device sysfs node name */
+    char *InputName;    /* Input device file name */
+};
+
+static void FreeNames(struct SysfsNames *names)
+{
+    struct SysfsNames *old;
+    do {
+        old = names;
+        names = names->next;
+        if (old->name)
+            free(old->name);
+        free(old);
+    } while (names);
+}
+
+static struct SysfsNames *SeekSysfsDir(char *dname, char *NodeName)
+{
+    DIR                *dir;
+    struct dirent      *entry;
+    struct SysfsNames  *names, *CurName;
+
+    names = calloc(sizeof(*names), 1);
+
+    CurName = names;
+
+    dir = opendir(dname);
+    if (!dir) {
+        perror(dname);
+        return NULL;
+    }
+    entry = readdir(dir);
+    while (entry) {
+        if (!NodeName || !strncmp(entry->d_name, NodeName, strlen(NodeName))) {
+            CurName->name = malloc(strlen(dname) + strlen(entry->d_name) + 2);
+            if (!CurName->name)
+                goto err;
+            strcpy(CurName->name, dname);
+            strcat(CurName->name, entry->d_name);
+            if (NodeName)
+                strcat(CurName->name, "/");
+            CurName->next = calloc(sizeof(*CurName), 1);
+            if (!CurName->next)
+                goto err;
+            CurName = CurName->next;
+        }
+        entry = readdir(dir);
+    }
+    closedir(dir);
+
+    if (names == CurName) {
+        xf86Msg(X_ERROR, "Couldn't find any node at %s%s*.\n",
+            dname, NodeName);
+        free (names);
+        names = NULL;
+    }
+    return names;
+
+err:
+    perror("Seek dir");
+    FreeNames(names);
+    return NULL;
+}
+
+static void FreeUevent(struct UEvents *uevent)
+{
+    struct UEvents *old;
+    do {
+        old = uevent;
+        uevent = uevent->next;
+        if (old->key)
+            free(old->key);
+        if (old->value)
+            free(old->value);
+        free(old);
+    } while (uevent);
+}
+
+static struct UEvents *ReadSysfsUevents(char *dname)
+{
+    FILE           *fp;
+    struct UEvents *next, *uevent;
+    char           *event = "uevent", *file, s[4096];
+
+    next = uevent = calloc(1, sizeof(*uevent));
+
+    file = malloc(strlen(dname) + strlen(event) + 1);
+    strcpy(file, dname);
+    strcat(file, event);
+
+    fp = fopen(file, "r");
+    if (!fp) {
+        perror(file);
+        free(file);
+        return NULL;
+    }
+    while (fgets(s, sizeof(s), fp)) {
+        char *p = strtok(s, "=");
+        if (!p)
+            continue;
+        next->key = malloc(strlen(p) + 1);
+        if (!next->key) {
+            perror("next->key");
+            free(file);
+            FreeUevent(uevent);
+            return NULL;
+        }
+        strcpy(next->key, p);
+
+        p = strtok(NULL, "\n");
+        if (!p) {
+            xf86Msg(X_ERROR, "Error on uevent information\n");
+            fclose(fp);
+            free(file);
+            FreeUevent(uevent);
+            return NULL;
+        }
+        next->value = malloc(strlen(p) + 1);
+        if (!next->value) {
+            perror("next->value");
+            free(file);
+            FreeUevent(uevent);
+            return NULL;
+        }
+        strcpy(next->value, p);
+
+        next->next = calloc(1, sizeof(*next));
+        if (!next->next) {
+            perror("next->next");
+            free(file);
+            FreeUevent(uevent);
+            return NULL;
+        }
+        next = next->next;
+    }
+    fclose(fp);
+    free(file);
+
+    return uevent;
+}
+
+static struct SysfsNames *FindDevice(char *name)
+{
+    char                     dname[256];
+    char                     *input = "rc";
+    static struct SysfsNames *names, *cur;
+    /*
+     * Get event sysfs node
+     */
+    snprintf(dname, sizeof(dname), "/sys/class/rc/");
+
+    names = SeekSysfsDir(dname, input);
+    if (!names)
+        return NULL;
+
+    if (name) {
+        static struct SysfsNames *tmp;
+        char *p, *n;
+        int found = 0;
+
+        n = malloc(strlen(name) + 2);
+        strcpy(n, name);
+        strcat(n,"/");
+        for (cur = names; cur->next; cur = cur->next) {
+            if (cur->name) {
+                p = cur->name + strlen(dname);
+                if (p && !strcmp(p, n)) {
+                    found = 1;
+                    break;
+                }
+            }
+        }
+        free(n);
+        if (!found) {
+            FreeNames(names);
+            xf86Msg(X_ERROR, "Not found device %s\n", name);
+            return NULL;
+        }
+        tmp = calloc(sizeof(*names), 1);
+        tmp->name = cur->name;
+        cur->name = NULL;
+        FreeNames(names);
+        return tmp;
+    }
+
+    return names;
+}
+
+static int GetAttribs(struct RCDevice *RCDev, char *SysfsName)
+{
+    struct UEvents           *uevent;
+    char                     *input = "input", *event = "event";
+    char                     *DEV = "/dev/";
+    static struct SysfsNames *InputNames, *event_names;
+
+    /* Clean the attributes */
+    memset(RCDev, 0, sizeof(*RCDev));
+
+    RCDev->SysfsName = SysfsName;
+
+    InputNames = SeekSysfsDir(RCDev->SysfsName, input);
+    if (!InputNames)
+        return EINVAL;
+    if (InputNames->next->next) {
+        xf86Msg(X_ERROR, "Found more than one input interface."
+                "This is currently unsupported\n");
+        return EINVAL;
+    }
+
+    event_names = SeekSysfsDir(InputNames->name, event);
+    FreeNames(InputNames);
+    if (!event_names) {
+        FreeNames(event_names);
+        return EINVAL;
+    }
+    if (event_names->next->next) {
+        FreeNames(event_names);
+        xf86Msg(X_ERROR, "Found more than one event interface."
+                "This is currently unsupported\n");
+        return EINVAL;
+    }
+
+    uevent = ReadSysfsUevents(event_names->name);
+    FreeNames(event_names);
+
+    if (!uevent)
+        return EINVAL;
+
+    while (uevent->next) {
+        if (!strcmp(uevent->key, "DEVNAME")) {
+            RCDev->InputName = malloc(strlen(uevent->value) + strlen(DEV) + 1);
+            strcpy(RCDev->InputName, DEV);
+            strcat(RCDev->InputName, uevent->value);
+            break;
+        }
+        uevent = uevent->next;
+    }
+    FreeUevent(uevent);
+
+    if (!RCDev->InputName) {
+        xf86Msg(X_ERROR, "Input device name not found.\n");
+        return EINVAL;
+    }
+
+    return 0;
+}
+
+RCDeviceListPtr GetRCInputDevices(int *num)
+{
+    struct RCDevice          RCDev;
+    static struct SysfsNames *names, *cur;
+    RCDeviceListPtr          RCDevList;
+    int                      n = 0;
+
+    *num = 0;
+
+    names = FindDevice(NULL);
+    if (!names)
+        return NULL;
+    for (cur = names; cur->next; cur = cur->next)
+        if (cur->name)
+            n++;
+
+    RCDevList = malloc(n * sizeof(RCDeviceList));
+    if (!RCDevList)
+        goto error;
+
+    *num = n;
+
+    n = 0;
+    for (cur = names; cur->next; cur = cur->next) {
+        if (cur->name) {
+            if (GetAttribs(&RCDev, cur->name)) {
+                free (RCDevList);
+                return NULL;
+            }
+            xf86Msg(X_INFO, "Found %s (%s)\n",
+                    RCDev.SysfsName, RCDev.InputName);
+
+            strncpy(RCDevList[n].SysfsName, RCDev.SysfsName,
+                    sizeof(RCDevList[n].SysfsName) - 1);
+	    RCDevList[n].SysfsName[sizeof(RCDevList[n].SysfsName) - 1] = '\0';
+            strncpy(RCDevList[n].InputName, RCDev.InputName,
+                    sizeof(RCDevList[n].InputName) - 1);
+	    RCDevList[n].InputName[sizeof(RCDevList[n].InputName) - 1] = '\0';
+            n++;
+        }
+    }
+
+error:
+    FreeNames(names);
+
+    return RCDevList;
+}
diff --git a/src/Makefile.am b/src/Makefile.am
index a5c89ac..a42fe35 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -36,5 +36,6 @@ AM_CPPFLAGS =-I$(top_srcdir)/include
                                @DRIVER_NAME@.h \
                                emuMB.c \
                                emuWheel.c \
-                               draglock.c
+                               draglock.c \
+                               GetRCInputs.c
 
diff --git a/src/Makefile.in b/src/Makefile.in
index 3d6195d..1299c58 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -95,7 +95,7 @@ am__installdirs = "$(DESTDIR)$(@DRIVER_NAME@_drv_ladir)"
 LTLIBRARIES = $(@DRIVER_NAME@_drv_la_LTLIBRARIES)
 @DRIVER_NAME@_drv_la_LIBADD =
 am_@DRIVER_NAME@_drv_la_OBJECTS = @DRIVER_NAME@.lo emuMB.lo \
-	emuWheel.lo draglock.lo
+	emuWheel.lo draglock.lo GetRCInputs.lo
 @DRIVER_NAME@_drv_la_OBJECTS = $(am_@DRIVER_NAME@_drv_la_OBJECTS)
 AM_V_lt = $(am__v_lt_$(V))
 am__v_lt_ = $(am__v_lt_$(AM_DEFAULT_VERBOSITY))
@@ -281,7 +281,8 @@ AM_CPPFLAGS = -I$(top_srcdir)/include
                                @DRIVER_NAME@.h \
                                emuMB.c \
                                emuWheel.c \
-                               draglock.c
+                               draglock.c \
+                               GetRCInputs.c
 
 all: all-am
 
diff --git a/src/evdev.c b/src/evdev.c
index 512e957..7a0af1c 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -296,6 +296,575 @@ EvdevQueueKbdEvent(InputInfoPtr pInfo, struct input_event *ev, int value)
     pEvdev->num_queue++;
 }
 
+/*
+ * Those are new keycodes added for 2.6.38 and used by some Remote Controllers
+ */
+#ifndef KEY_10CHANNELSUP
+#define KEY_10CHANNELSUP        0x1b8   /* 10 channels up (10+) */
+#define KEY_10CHANNELSDOWN      0x1b9   /* 10 channels down (10-) */
+#endif
+
+int rc_keymap[] = {
+	[KEY_RESERVED] = KEY_RESERVED,                    /* not assigned */
+	[KEY_ESC] = KEY_ESC,
+	[KEY_1] = KEY_1,
+	[KEY_2] = KEY_2,
+	[KEY_3] = KEY_3,
+	[KEY_4] = KEY_4,
+	[KEY_5] = KEY_5,
+	[KEY_6] = KEY_6,
+	[KEY_7] = KEY_7,
+	[KEY_8] = KEY_8,
+	[KEY_9] = KEY_9,
+	[KEY_0] = KEY_0,
+	[KEY_MINUS] = KEY_MINUS,
+	[KEY_EQUAL] = KEY_EQUAL,
+	[KEY_BACKSPACE] = KEY_BACKSPACE,
+	[KEY_TAB] = KEY_TAB,
+	[KEY_Q] = KEY_Q,
+	[KEY_W] = KEY_W,
+	[KEY_E] = KEY_E,
+	[KEY_R] = KEY_R,
+	[KEY_T] = KEY_T,
+	[KEY_Y] = KEY_Y,
+	[KEY_U] = KEY_U,
+	[KEY_I] = KEY_I,
+	[KEY_O] = KEY_O,
+	[KEY_P] = KEY_P,
+	[KEY_LEFTBRACE] = KEY_LEFTBRACE,
+	[KEY_RIGHTBRACE] = KEY_RIGHTBRACE,
+	[KEY_ENTER] = KEY_ENTER,
+	[KEY_LEFTCTRL] = KEY_LEFTCTRL,
+	[KEY_A] = KEY_A,
+	[KEY_S] = KEY_S,
+	[KEY_D] = KEY_D,
+	[KEY_F] = KEY_F,
+	[KEY_G] = KEY_G,
+	[KEY_H] = KEY_H,
+	[KEY_J] = KEY_J,
+	[KEY_K] = KEY_K,
+	[KEY_L] = KEY_L,
+	[KEY_SEMICOLON] = KEY_SEMICOLON,
+	[KEY_APOSTROPHE] = KEY_APOSTROPHE,
+	[KEY_GRAVE] = KEY_GRAVE,
+	[KEY_LEFTSHIFT] = KEY_LEFTSHIFT,
+	[KEY_BACKSLASH] = KEY_BACKSLASH,
+	[KEY_Z] = KEY_Z,
+	[KEY_X] = KEY_X,
+	[KEY_C] = KEY_C,
+	[KEY_V] = KEY_V,
+	[KEY_B] = KEY_B,
+	[KEY_N] = KEY_N,
+	[KEY_M] = KEY_M,
+	[KEY_COMMA] = KEY_COMMA,
+	[KEY_DOT] = KEY_DOT,
+	[KEY_SLASH] = KEY_SLASH,
+	[KEY_RIGHTSHIFT] = KEY_RIGHTSHIFT,
+	[KEY_KPASTERISK] = KEY_KPASTERISK,
+	[KEY_LEFTALT] = KEY_LEFTALT,
+	[KEY_SPACE] = KEY_SPACE,
+	[KEY_CAPSLOCK] = KEY_CAPSLOCK,
+	[KEY_F1] = KEY_F1,
+	[KEY_F2] = KEY_F2,
+	[KEY_F3] = KEY_F3,
+	[KEY_F4] = KEY_F4,
+	[KEY_F5] = KEY_F5,
+	[KEY_F6] = KEY_F6,
+	[KEY_F7] = KEY_F7,
+	[KEY_F8] = KEY_F8,
+	[KEY_F9] = KEY_F9,
+	[KEY_F10] = KEY_F10,
+	[KEY_NUMLOCK] = KEY_NUMLOCK,
+	[KEY_SCROLLLOCK] = KEY_RESERVED,                  /* not assigned */
+	[KEY_KP7] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP8] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP9] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KPMINUS] = KEY_KPMINUS,
+	[KEY_KP4] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP5] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP6] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KPPLUS] = KEY_KPPLUS,
+	[KEY_KP1] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP2] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP3] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KP0] = KEY_RESERVED,                         /* not assigned */
+	[KEY_KPDOT] = KEY_RESERVED,                       /* not assigned */
+	[84] = KEY_RESERVED,                              /* not defined on linux/input.h yet */
+	[KEY_ZENKAKUHANKAKU] = KEY_RESERVED,              /* not assigned */
+	[KEY_102ND] = KEY_RESERVED,                       /* not assigned */
+	[KEY_F11] = KEY_F11,
+	[KEY_F12] = KEY_F12,
+	[KEY_RO] = KEY_RESERVED,                          /* not assigned */
+	[KEY_KATAKANA] = KEY_RESERVED,                    /* not assigned */
+	[KEY_HIRAGANA] = KEY_RESERVED,                    /* not assigned */
+	[KEY_HENKAN] = KEY_RESERVED,                      /* not assigned */
+	[KEY_KATAKANAHIRAGANA] = KEY_RESERVED,            /* not assigned */
+	[KEY_MUHENKAN] = KEY_RESERVED,                    /* not assigned */
+	[KEY_KPJPCOMMA] = KEY_RESERVED,                   /* not assigned */
+	[KEY_KPENTER] = KEY_RESERVED,                     /* not assigned */
+	[KEY_RIGHTCTRL] = KEY_RESERVED,                   /* not assigned */
+	[KEY_KPSLASH] = KEY_RESERVED,                     /* not assigned */
+	[KEY_SYSRQ] = KEY_RESERVED,                       /* not assigned */
+	[KEY_RIGHTALT] = KEY_RESERVED,                    /* not assigned */
+	[KEY_LINEFEED] = KEY_RESERVED,                    /* not assigned */
+	[KEY_HOME] = KEY_HOME,
+	[KEY_UP] = KEY_UP,
+	[KEY_PAGEUP] = KEY_PAGEUP,
+	[KEY_LEFT] = KEY_LEFT,
+	[KEY_RIGHT] = KEY_RIGHT,
+	[KEY_END] = KEY_END,
+	[KEY_DOWN] = KEY_DOWN,
+	[KEY_PAGEDOWN] = KEY_PAGEDOWN,
+	[KEY_INSERT] = KEY_INSERT,
+	[KEY_DELETE] = KEY_DELETE,
+	[KEY_MACRO] = KEY_MACRO,
+	[KEY_MUTE] = KEY_MUTE,
+	[KEY_VOLUMEDOWN] = KEY_VOLUMEDOWN,
+	[KEY_VOLUMEUP] = KEY_VOLUMEUP,
+	[KEY_POWER] = KEY_POWER,
+	[KEY_KPEQUAL] = KEY_RESERVED,                     /* not assigned */
+	[KEY_KPPLUSMINUS] = KEY_KPPLUSMINUS,
+	[KEY_PAUSE] = KEY_PAUSE,
+	[KEY_SCALE] = KEY_RESERVED,                       /* not assigned */
+	[KEY_KPCOMMA] = KEY_RESERVED,                     /* not assigned */
+	[KEY_HANGEUL] = KEY_RESERVED,                     /* not assigned */
+	[KEY_HANJA] = KEY_RESERVED,                       /* not assigned */
+	[KEY_YEN] = KEY_RESERVED,                         /* not assigned */
+	[KEY_LEFTMETA] = KEY_RESERVED,                    /* not assigned */
+	[KEY_RIGHTMETA] = KEY_RESERVED,                   /* not assigned */
+	[KEY_COMPOSE] = KEY_RESERVED,                     /* not assigned */
+	[KEY_STOP] = KEY_STOP,
+	[KEY_AGAIN] = KEY_AGAIN,
+	[KEY_PROPS] = KEY_PROPS,
+	[KEY_UNDO] = KEY_UNDO,
+	[KEY_FRONT] = KEY_FRONT,
+	[KEY_COPY] = KEY_RESERVED,                        /* not assigned */
+	[KEY_OPEN] = KEY_OPEN,
+	[KEY_PASTE] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FIND] = KEY_RESERVED,                        /* not assigned */
+	[KEY_CUT] = KEY_RESERVED,                         /* not assigned */
+	[KEY_HELP] = KEY_HELP,
+	[KEY_MENU] = KEY_MENU,
+	[KEY_CALC] = KEY_RESERVED,                        /* not assigned */
+	[KEY_SETUP] = KEY_SETUP,
+	[KEY_SLEEP] = KEY_SLEEP,
+	[KEY_WAKEUP] = KEY_RESERVED,                      /* not assigned */
+	[KEY_FILE] = KEY_RESERVED,                        /* not assigned */
+	[KEY_SENDFILE] = KEY_RESERVED,                    /* not assigned */
+	[KEY_DELETEFILE] = KEY_RESERVED,                  /* not assigned */
+	[KEY_XFER] = KEY_RESERVED,                        /* not assigned */
+	[KEY_PROG1] = KEY_PROG1,
+	[KEY_PROG2] = KEY_PROG2,
+	[KEY_WWW] = KEY_WWW,
+	[KEY_MSDOS] = KEY_RESERVED,                       /* not assigned */
+	[KEY_SCREENLOCK] = KEY_RESERVED,                  /* not assigned */
+	[KEY_DIRECTION] = KEY_RESERVED,                   /* not assigned */
+	[KEY_CYCLEWINDOWS] = KEY_CYCLEWINDOWS,
+	[KEY_MAIL] = KEY_MAIL,
+	[KEY_BOOKMARKS] = KEY_BOOKMARKS,
+	[KEY_COMPUTER] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BACK] = KEY_BACK,
+	[KEY_FORWARD] = KEY_FORWARD,
+	[KEY_CLOSECD] = KEY_CLOSECD,
+	[KEY_EJECTCD] = KEY_EJECTCD,
+	[KEY_EJECTCLOSECD] = KEY_EJECTCLOSECD,
+	[KEY_NEXTSONG] = KEY_NEXTSONG,
+	[KEY_PLAYPAUSE] = KEY_PLAYPAUSE,
+	[KEY_PREVIOUSSONG] = KEY_PREVIOUSSONG,
+	[KEY_STOPCD] = KEY_STOPCD,
+	[KEY_RECORD] = KEY_RECORD,
+	[KEY_REWIND] = KEY_REWIND,
+	[KEY_PHONE] = KEY_PHONE,
+	[KEY_ISO] = KEY_RESERVED,                         /* not assigned */
+	[KEY_CONFIG] = KEY_CONFIG,
+	[KEY_HOMEPAGE] = KEY_HOMEPAGE,
+	[KEY_REFRESH] = KEY_REFRESH,
+	[KEY_EXIT] = KEY_EXIT,
+	[KEY_MOVE] = KEY_RESERVED,                        /* not assigned */
+	[KEY_EDIT] = KEY_EDIT,
+	[KEY_SCROLLUP] = KEY_RESERVED,                    /* not assigned */
+	[KEY_SCROLLDOWN] = KEY_RESERVED,                  /* not assigned */
+	[KEY_KPLEFTPAREN] = KEY_RESERVED,                 /* not assigned */
+	[KEY_KPRIGHTPAREN] = KEY_RESERVED,                /* not assigned */
+	[KEY_NEW] = KEY_NEW,
+	[KEY_REDO] = KEY_RESERVED,                        /* not assigned */
+	[KEY_F13] = KEY_F13,
+	[KEY_F14] = KEY_F14,
+	[KEY_F15] = KEY_F15,
+	[KEY_F16] = KEY_F16,
+	[KEY_F17] = KEY_F17,
+	[KEY_F18] = KEY_F18,
+	[KEY_F19] = KEY_F19,
+	[KEY_F20] = KEY_RESERVED,                         /* not assigned */
+	[KEY_F21] = KEY_F21,
+	[KEY_F22] = KEY_F22,
+	[KEY_F23] = KEY_F23,
+	[KEY_F24] = KEY_F24,
+	[195] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[196] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[197] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[198] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[199] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_PLAYCD] = KEY_PLAYCD,
+	[KEY_PAUSECD] = KEY_PAUSECD,
+	[KEY_PROG3] = KEY_PROG3,
+	[KEY_PROG4] = KEY_RESERVED,                       /* not assigned */
+	[KEY_DASHBOARD] = KEY_RESERVED,                   /* not assigned */
+	[KEY_SUSPEND] = KEY_SUSPEND,
+	[KEY_CLOSE] = KEY_CLOSE,
+	[KEY_PLAY] = KEY_PLAY,
+	[KEY_FASTFORWARD] = KEY_FASTFORWARD,
+	[KEY_BASSBOOST] = KEY_RESERVED,                   /* not assigned */
+	[KEY_PRINT] = KEY_PRINT,
+	[KEY_HP] = KEY_RESERVED,                          /* not assigned */
+	[KEY_CAMERA] = KEY_CAMERA,
+	[KEY_SOUND] = KEY_SOUND,
+	[KEY_QUESTION] = KEY_RESERVED,                    /* not assigned */
+	[KEY_EMAIL] = KEY_EMAIL,
+	[KEY_CHAT] = KEY_RESERVED,                        /* not assigned */
+	[KEY_SEARCH] = KEY_SEARCH,
+	[KEY_CONNECT] = KEY_RESERVED,                     /* not assigned */
+	[KEY_FINANCE] = KEY_RESERVED,                     /* not assigned */
+	[KEY_SPORT] = KEY_RESERVED,                       /* not assigned */
+	[KEY_SHOP] = KEY_RESERVED,                        /* not assigned */
+	[KEY_ALTERASE] = KEY_RESERVED,                    /* not assigned */
+	[KEY_CANCEL] = KEY_CANCEL,
+	[KEY_BRIGHTNESSDOWN] = KEY_BRIGHTNESSDOWN,
+	[KEY_BRIGHTNESSUP] = KEY_BRIGHTNESSUP,
+	[KEY_MEDIA] = KEY_MEDIA,
+	[KEY_SWITCHVIDEOMODE] = KEY_SWITCHVIDEOMODE,
+	[KEY_KBDILLUMTOGGLE] = KEY_RESERVED,              /* not assigned */
+	[KEY_KBDILLUMDOWN] = KEY_RESERVED,                /* not assigned */
+	[KEY_KBDILLUMUP] = KEY_RESERVED,                  /* not assigned */
+	[KEY_SEND] = KEY_RESERVED,                        /* not assigned */
+	[KEY_REPLY] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FORWARDMAIL] = KEY_RESERVED,                 /* not assigned */
+	[KEY_SAVE] = KEY_SAVE,
+	[KEY_DOCUMENTS] = KEY_RESERVED,                   /* not assigned */
+	[KEY_BATTERY] = KEY_RESERVED,                     /* not assigned */
+	[KEY_BLUETOOTH] = KEY_RESERVED,                   /* not assigned */
+	[KEY_WLAN] = KEY_RESERVED,                        /* not assigned */
+	[KEY_UWB] = KEY_RESERVED,                         /* not assigned */
+	[KEY_UNKNOWN] = KEY_UNKNOWN,
+	[KEY_VIDEO_NEXT] = KEY_RESERVED,                  /* not assigned */
+	[KEY_VIDEO_PREV] = KEY_RESERVED,                  /* not assigned */
+	[KEY_BRIGHTNESS_CYCLE] = KEY_RESERVED,            /* not assigned */
+	[KEY_BRIGHTNESS_ZERO] = KEY_RESERVED,             /* not assigned */
+	[KEY_DISPLAY_OFF] = KEY_RESERVED,                 /* not assigned */
+	[KEY_WIMAX] = KEY_RESERVED,                       /* not assigned */
+	[KEY_RFKILL] = KEY_RESERVED,                      /* not assigned */
+	[248] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[249] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[250] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[251] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[252] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[253] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[254] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[255] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[256] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[257] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[258] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[259] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[260] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[261] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[262] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[263] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[264] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[265] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[266] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[267] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[268] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[269] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[270] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[271] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[272] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[273] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[274] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[275] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[276] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[277] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[278] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[279] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[280] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[281] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[282] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[283] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[284] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[285] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[286] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[287] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[288] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[289] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[290] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[291] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[292] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[293] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[294] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[295] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[296] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[297] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[298] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[299] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[300] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[301] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[302] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[303] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[304] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[305] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[306] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[307] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[308] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[309] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[310] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[311] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[312] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[313] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[314] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[315] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[316] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[317] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[318] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[319] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[320] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[321] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[322] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[323] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[324] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[325] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[326] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[327] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[328] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[329] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[330] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[331] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[332] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[333] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[334] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[335] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[336] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[337] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[338] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[339] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[340] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[341] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[342] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[343] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[344] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[345] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[346] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[347] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[348] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[349] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[350] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[351] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_OK] = 125,                                   /* KEY_LEFTMETA */
+	[KEY_SELECT] = 153,                               /* KEY_DIRECTION */
+	[KEY_GOTO] = 94,                                  /* KEY_MUHENKAN */
+	[KEY_CLEAR] = 84,                                 /* currently unused */
+	[KEY_POWER2] = 135,                               /* KEY_PASTE */
+	[KEY_OPTION] = 126,                               /* KEY_RIGHTMETA */
+	[KEY_INFO] = 96,                                  /* KEY_KPENTER */
+	[KEY_TIME] = 180,                                 /* KEY_KPRIGHTPAREN */
+	[KEY_VENDOR] = KEY_RESERVED,                      /* not assigned */
+	[KEY_ARCHIVE] = KEY_RESERVED,                     /* not assigned */
+	[KEY_PROGRAM] = KEY_RESERVED,                     /* not assigned */
+	[KEY_CHANNEL] = 81,                               /* KEY_KP3 */
+	[KEY_FAVORITES] = 91,                             /* KEY_HIRAGANA */
+	[KEY_EPG] = 90,                                   /* KEY_KATAKANA */
+	[KEY_PVR] = 137,                                  /* KEY_CUT */
+	[KEY_MHP] = 120,                                  /* KEY_SCALE */
+	[KEY_LANGUAGE] = 98,                              /* KEY_KPSLASH */
+	[KEY_TITLE] = 182,                                /* KEY_REDO */
+	[KEY_SUBTITLE] = 178,                             /* KEY_SCROLLDOWN */
+	[KEY_ANGLE] = 73,                                 /* KEY_KP9 */
+	[KEY_ZOOM] = 203,                                 /* KEY_PROG4 */
+	[KEY_MODE] = 121,                                 /* KEY_KPCOMMA */
+	[KEY_KEYBOARD] = 97,                              /* KEY_RIGHTCTRL */
+	[KEY_SCREEN] = 151,                               /* KEY_MSDOS */
+	[KEY_PC] = 127,                                   /* KEY_COMPOSE */
+	[KEY_TV] = 195,                                   /* currently unused */
+	[KEY_TV2] = 196,                                  /* currently unused */
+	[KEY_VCR] = 197,                                  /* currently unused */
+	[KEY_VCR2] = KEY_RESERVED,                        /* not assigned */
+	[KEY_SAT] = 147,                                  /* KEY_XFER */
+	[KEY_SAT2] = KEY_RESERVED,                        /* not assigned */
+	[KEY_CD] = 80,                                    /* KEY_KP2 */
+	[KEY_TAPE] = KEY_RESERVED,                        /* not assigned */
+	[KEY_RADIO] = 140,                                /* KEY_CALC */
+	[KEY_TUNER] = 190,                                /* KEY_F20 */
+	[KEY_PLAYER] = 133,                               /* KEY_COPY */
+	[KEY_TEXT] = 179,                                 /* KEY_KPLEFTPAREN */
+	[KEY_DVD] = 86,                                   /* KEY_102ND */
+	[KEY_AUX] = 76,                                   /* KEY_KP5 */
+	[KEY_MP3] = 122,                                  /* KEY_HANGEUL */
+	[KEY_AUDIO] = 75,                                 /* KEY_KP4 */
+	[KEY_VIDEO] = 198,                                /* currently unused */
+	[KEY_DIRECTORY] = KEY_RESERVED,                   /* not assigned */
+	[KEY_LIST] = 100,                                 /* KEY_RIGHTALT */
+	[KEY_MEMO] = KEY_RESERVED,                        /* not assigned */
+	[KEY_CALENDAR] = KEY_RESERVED,                    /* not assigned */
+	[KEY_RED] = 143,                                  /* KEY_WAKEUP */
+	[KEY_GREEN] = 95,                                 /* KEY_KPJPCOMMA */
+	[KEY_YELLOW] = 199,                               /* currently unused */
+	[KEY_BLUE] = 77,                                  /* KEY_KP6 */
+	[KEY_CHANNELUP] = 83,                             /* KEY_KPDOT */
+	[KEY_CHANNELDOWN] = 82,                           /* KEY_KP0 */
+	[KEY_FIRST] = 92,                                 /* KEY_HENKAN */
+	[KEY_LAST] = 99,                                  /* KEY_SYSRQ */
+	[KEY_AB] = 72,                                    /* KEY_KP8 */
+	[KEY_NEXT] = 123,                                 /* KEY_HANJA */
+	[KEY_RESTART] = 144,                              /* KEY_FILE */
+	[KEY_SLOW] = 170,                                 /* KEY_ISO */
+	[KEY_SHUFFLE] = 157,                              /* KEY_COMPUTER */
+	[KEY_BREAK] = 79,                                 /* KEY_KP1 */
+	[KEY_PREVIOUS] = 136,                             /* KEY_FIND */
+	[KEY_DIGITS] = 85,                                /* KEY_ZENKAKUHANKAKU */
+	[KEY_TEEN] = KEY_RESERVED,                        /* not assigned */
+	[KEY_TWEN] = KEY_RESERVED,                        /* not assigned */
+	[KEY_VIDEOPHONE] = KEY_RESERVED,                  /* not assigned */
+	[KEY_GAMES] = KEY_RESERVED,                       /* not assigned */
+	[KEY_ZOOMIN] = 204,                               /* KEY_DASHBOARD */
+	[KEY_ZOOMOUT] = 209,                              /* KEY_BASSBOOST */
+	[KEY_ZOOMRESET] = KEY_RESERVED,                   /* not assigned */
+	[KEY_WORDPROCESSOR] = KEY_RESERVED,               /* not assigned */
+	[KEY_EDITOR] = KEY_RESERVED,                      /* not assigned */
+	[KEY_SPREADSHEET] = KEY_RESERVED,                 /* not assigned */
+	[KEY_GRAPHICSEDITOR] = KEY_RESERVED,              /* not assigned */
+	[KEY_PRESENTATION] = KEY_RESERVED,                /* not assigned */
+	[KEY_DATABASE] = KEY_RESERVED,                    /* not assigned */
+	[KEY_NEWS] = KEY_RESERVED,                        /* not assigned */
+	[KEY_VOICEMAIL] = KEY_RESERVED,                   /* not assigned */
+	[KEY_ADDRESSBOOK] = KEY_RESERVED,                 /* not assigned */
+	[KEY_MESSENGER] = KEY_RESERVED,                   /* not assigned */
+	[KEY_DISPLAYTOGGLE] = KEY_RESERVED,               /* not assigned */
+	[KEY_SPELLCHECK] = KEY_RESERVED,                  /* not assigned */
+	[KEY_LOGOFF] = KEY_RESERVED,                      /* not assigned */
+	[KEY_DOLLAR] = KEY_RESERVED,                      /* not assigned */
+	[KEY_EURO] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FRAMEBACK] = KEY_RESERVED,                   /* not assigned */
+	[KEY_FRAMEFORWARD] = KEY_RESERVED,                /* not assigned */
+	[KEY_CONTEXT_MENU] = KEY_RESERVED,                /* not assigned */
+	[KEY_MEDIA_REPEAT] = 117,                         /* KEY_KPEQUAL */
+	[KEY_10CHANNELSUP] = 71,                          /* KEY_KP7 */
+	[KEY_10CHANNELSDOWN] = 70,                        /* KEY_SCROLLLOCK */
+	[442] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[443] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[444] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[445] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[446] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[447] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_DEL_EOL] = KEY_RESERVED,                     /* not assigned */
+	[KEY_DEL_EOS] = KEY_RESERVED,                     /* not assigned */
+	[KEY_INS_LINE] = KEY_RESERVED,                    /* not assigned */
+	[KEY_DEL_LINE] = KEY_RESERVED,                    /* not assigned */
+	[452] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[453] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[454] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[455] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[456] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[457] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[458] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[459] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[460] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[461] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[462] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[463] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_FN] = 93,                                    /* KEY_KATAKANAHIRAGANA */
+	[KEY_FN_ESC] = KEY_RESERVED,                      /* not assigned */
+	[KEY_FN_F1] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F2] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F3] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F4] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F5] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F6] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F7] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F8] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F9] = KEY_RESERVED,                       /* not assigned */
+	[KEY_FN_F10] = KEY_RESERVED,                      /* not assigned */
+	[KEY_FN_F11] = KEY_RESERVED,                      /* not assigned */
+	[KEY_FN_F12] = KEY_RESERVED,                      /* not assigned */
+	[KEY_FN_1] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FN_2] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FN_D] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FN_E] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FN_F] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FN_S] = KEY_RESERVED,                        /* not assigned */
+	[KEY_FN_B] = KEY_RESERVED,                        /* not assigned */
+	[485] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[486] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[487] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[488] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[489] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[490] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[491] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[492] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[493] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[494] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[495] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[496] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_BRL_DOT1] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT2] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT3] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT4] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT5] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT6] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT7] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT8] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT9] = KEY_RESERVED,                    /* not assigned */
+	[KEY_BRL_DOT10] = KEY_RESERVED,                   /* not assigned */
+	[507] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[508] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[509] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[510] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[511] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_NUMERIC_0] = 11,                             /* KEY_0 */
+	[KEY_NUMERIC_1] = 2,                              /* KEY_1 */
+	[KEY_NUMERIC_2] = 3,                              /* KEY_2 */
+	[KEY_NUMERIC_3] = 4,                              /* KEY_3 */
+	[KEY_NUMERIC_4] = 5,                              /* KEY_4 */
+	[KEY_NUMERIC_5] = 6,                              /* KEY_5 */
+	[KEY_NUMERIC_6] = 7,                              /* KEY_6 */
+	[KEY_NUMERIC_7] = 8,                              /* KEY_7 */
+	[KEY_NUMERIC_8] = 9,                              /* KEY_8 */
+	[KEY_NUMERIC_9] = 10,                             /* KEY_9 */
+	[KEY_NUMERIC_STAR] = 55,                          /* KEY_KPASTERISK */
+	[KEY_NUMERIC_POUND] = 124,                        /* KEY_YEN */
+	[524] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[525] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[526] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[527] = KEY_RESERVED,                             /* not defined on linux/input.h yet */
+	[KEY_CAMERA_FOCUS] = KEY_RESERVED,                /* not assigned */
+	[KEY_WPS_BUTTON] = KEY_RESERVED,                  /* not assigned */
+	[530] = KEY_RESERVED,                             /* KEY_TOUCHPAD_TOGGLE - not assigned */
+	[531] = KEY_RESERVED,                             /* KEY_TOUCHPAD_ON - not assigned */
+	[531] = KEY_RESERVED,                             /* KEY_TOUCHPAD_OFF - not assigned */
+};
+
+void
+EvdevQueueRCEvent(InputInfoPtr pInfo, struct input_event *ev, int value)
+{
+    int code;
+    EventQueuePtr pQueue;
+    EvdevPtr pEvdev = pInfo->private;
+
+    if (ev->code >= ArrayLength(rc_keymap))
+        code = KEY_RESERVED + MIN_KEYCODE;
+    else
+        code = rc_keymap[ev->code] + MIN_KEYCODE;
+
+    if (pEvdev->num_queue >= EVDEV_MAXQUEUE)
+    {
+        xf86Msg(X_NONE, "%s: dropping event due to full queue!\n", pInfo->name);
+        return;
+    }
+
+    pQueue = &pEvdev->queue[pEvdev->num_queue];
+    pQueue->type = EV_QUEUE_KEY;
+    pQueue->key = code;
+    pQueue->val = value;
+    pEvdev->num_queue++;
+}
+
 void
 EvdevQueueButtonEvent(InputInfoPtr pInfo, int button, int value)
 {
@@ -468,6 +1037,8 @@ EvdevProcessButtonEvent(InputInfoPtr pInfo, struct input_event *ev)
 
     if (button)
         EvdevQueueButtonEvent(pInfo, button, value);
+    else if (pEvdev->flags & EVDEV_RC_EVENTS)
+        EvdevQueueRCEvent(pInfo, ev, value);
     else
         EvdevQueueKbdEvent(pInfo, ev, value);
 }
@@ -2012,6 +2583,8 @@ EvdevOpenDevice(InputInfoPtr pInfo)
 {
     EvdevPtr pEvdev = pInfo->private;
     char *device = (char*)pEvdev->device;
+    RCDeviceListPtr pRCDevList;
+    int NumDevices, i;
 
     if (!device)
     {
@@ -2047,6 +2620,25 @@ EvdevOpenDevice(InputInfoPtr pInfo)
         return FALSE;
     }
 
+    /* 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;
+        }
+    }
+    if (pEvdev->flags & EVDEV_RC_EVENTS) {
+        xf86Msg(X_INFO, "%s: device '%s' is a Remote Controller\n",
+                pInfo->name, device);
+    } else {
+        xf86Msg(X_INFO, "%s: device '%s' IS NOT a Remote Controller\n",
+                pInfo->name, device);
+    }
+
+    if (pRCDevList)
+        free(pRCDevList);
+
     return TRUE;
 }
 
diff --git a/src/evdev.h b/src/evdev.h
index 4945140..b891992 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -71,6 +71,7 @@
 #define EVDEV_UNIGNORE_ABSOLUTE (1 << 9) /* explicitly unignore abs axes */
 #define EVDEV_UNIGNORE_RELATIVE (1 << 10) /* explicitly unignore rel axes */
 #define EVDEV_RELATIVE_MODE	(1 << 11) /* Force relative events for devices with absolute axes */
+#define EVDEV_RC_EVENTS         (1 << 12) /* Device is a remote controller */
 
 #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) >= 3
 #define HAVE_PROPERTIES 1
@@ -195,6 +196,11 @@ typedef struct {
     EventQueueRec           queue[EVDEV_MAXQUEUE];
 } EvdevRec, *EvdevPtr;
 
+typedef struct {
+    char SysfsName[255];    /* Device sysfs node name */
+    char InputName[255];    /* Input device file name */
+} RCDeviceList, *RCDeviceListPtr;
+
 /* Event posting functions */
 void EvdevQueueKbdEvent(InputInfoPtr pInfo, struct input_event *ev, int value);
 void EvdevQueueButtonEvent(InputInfoPtr pInfo, int button, int value);
@@ -224,6 +230,9 @@ BOOL EvdevWheelEmuFilterMotion(InputInfoPtr pInfo, struct input_event *pEv);
 void EvdevDragLockPreInit(InputInfoPtr pInfo);
 BOOL EvdevDragLockFilterEvent(InputInfoPtr pInfo, unsigned int button, int value);
 
+/* GetRCInputs code */
+RCDeviceListPtr GetRCInputDevices(int *num);
+
 #ifdef HAVE_PROPERTIES
 void EvdevMBEmuInitProperty(DeviceIntPtr);
 void EvdevWheelEmuInitProperty(DeviceIntPtr);
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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. 

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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

Re: IR remote control autorepeat / evdev

[Mauro Carvalho Chehab]

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.

Re: IR remote control autorepeat / evdev

[Peter Hutterer]

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 <mchehab@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 <mchehab@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

Re: IR remote control autorepeat / evdev

[Anssi Hannula]

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