From: Anssi Hannula <anssi.hannula@iki.fi>
To: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Peter Hutterer <peter.hutterer@who-t.net>,
linux-media@vger.kernel.org,
"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
xorg-devel@lists.freedesktop.org
Subject: Re: IR remote control autorepeat / evdev
Date: Sat, 14 May 2011 02:07:55 +0300 [thread overview]
Message-ID: <4DCDB9CB.7030306@iki.fi> (raw)
In-Reply-To: <4DCDB333.8000801@redhat.com>
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
next prev parent reply other threads:[~2011-05-13 23:08 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-08 4:38 IR remote control autorepeat / evdev Anssi Hannula
2011-05-10 4:11 ` Peter Hutterer
2011-05-10 5:14 ` Anssi Hannula
2011-05-10 5:30 ` Peter Hutterer
2011-05-10 13:43 ` Anssi Hannula
2011-05-11 4:46 ` Mauro Carvalho Chehab
2011-05-11 16:33 ` Anssi Hannula
2011-05-11 16:51 ` Mauro Carvalho Chehab
2011-05-11 17:59 ` Anssi Hannula
2011-05-11 20:53 ` Dmitry Torokhov
2011-05-12 0:17 ` Anssi Hannula
2011-05-12 0:55 ` Mauro Carvalho Chehab
2011-05-11 23:52 ` Mauro Carvalho Chehab
2011-05-12 0:37 ` Anssi Hannula
2011-05-12 1:10 ` Mauro Carvalho Chehab
2011-05-12 1:36 ` Mauro Carvalho Chehab
2011-05-12 3:48 ` Jarod Wilson
2011-05-12 6:05 ` Peter Hutterer
2011-05-12 13:24 ` Jarod Wilson
2011-05-13 7:51 ` Mauro Carvalho Chehab
2011-05-16 1:35 ` Peter Hutterer
2011-05-12 23:48 ` Anssi Hannula
2011-05-13 22:39 ` Mauro Carvalho Chehab
2011-05-13 23:07 ` Anssi Hannula [this message]
2011-05-15 3:41 ` Mauro Carvalho Chehab
2011-05-23 21:36 ` Anssi Hannula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DCDB9CB.7030306@iki.fi \
--to=anssi.hannula@iki.fi \
--cc=linux-input@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@redhat.com \
--cc=peter.hutterer@who-t.net \
--cc=xorg-devel@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox