Thread (26 messages) 26 messages, 6 authors, 2011-05-23

Re: IR remote control autorepeat / evdev

From: Mauro Carvalho Chehab <hidden>
Date: 2011-05-15 03:42:02
Also in: linux-media

Em 14-05-2011 01:07, Anssi Hannula escreveu:
On 14.05.2011 01:39, Mauro Carvalho Chehab wrote:
quoted
Em 13-05-2011 01:48, Anssi Hannula escreveu:
quoted
On 12.05.2011 04:36, Mauro Carvalho Chehab wrote:
quoted
Em 12-05-2011 03:10, Mauro Carvalho Chehab escreveu:
quoted
Em 12-05-2011 02:37, Anssi Hannula escreveu:
quoted
quoted
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.
quoted
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.
quoted
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help