[PATCH] media: rc: meson-ir: add timeout on idle

[Matthias Reichl]

Hi Sean,

On Sun, Mar 11, 2018 at 12:55:19PM +0000, Sean Young wrote:
> Hi Matthias,
> 
> On Sat, Mar 10, 2018 at 06:38:28PM +0100, Matthias Reichl wrote:
> > On Sat, Mar 10, 2018 at 11:27:45AM +0000, Sean Young wrote:
> > > So if the timeout is below N then you will never get a space of N or larger;
> > > the largest space I know of in an IR message is 40ms in the sanyo protocol:
> > > 
> > > https://www.sbprojects.net/knowledge/ir/sharp.php
> > > 
> > > So if timeout is set to less than 40ms, we would have trouble decoding the
> > > sharp protocol.
> > > 
> > > The space between nec repeats is a little less than 100ms. I'm trying to
> > > remember what would could go wrong if the space between them would be
> > > timeouts instead. Mauro do you remember? I can imagine some IR hardware
> > > (e.g. winbond) queuing up IR after generating a timeout, thus delaying
> > > delivering IR to the kernel and we ending up generating a key up.
> > > 
> > > The problem with a higher timeout is that the trailing space (=timeout)
> > > is sometimes needed for decoding, and decoding of the last message is
> > > delayed until the timeout is received. That means the keyup message is
> > > delayed until that time, making keys a bit "sticky" and more likely to
> > > generate repeats. I'm pretty sure that is needed for rc-5 and nec.
> > 
> > Another issue with high timeouts is the response to very short button
> > presses where the remote only transmits a single scancode. It then
> > takes signal transmission time plus timeout, so roughly a quarter
> > of a second on meson-ir and ite-cir with 200ms timeout, until the
> > scancode is decoded and the keydown event is generated.
> > 
> > On longer button presses this is less of an issue as we get the
> > space signal when the first pulse of the repeated scancode is
> > received. So the delay between button press and keydown is determined
> > by the remote scancode repeat interval and with typically ~110ms
> > on nec/rc-5 a lot lower.
> > 
> > This affects both "quick fingers" using a standard remote and
> > users of programmable remotes like the Logitech Harmony where
> > the number of scancodes transmitted on a short press can be
> > configured. With a single scancode transmission we run into
> > the long keydown delay, 2 scancodes is fine, and at 3 or 4 we
> > start running into the key repeat issue.
> > 
> > We received several reports from users that their remote felt
> > "sluggish" when we switched from the downstream "amremote" driver
> > (which IIRC decoded the nec protocol in hardware) to meson-ir.
> > 
> > Lowering the timeout to 125ms or even significantly lower
> > (depending on what the protocol and IR receiver permits)
> > removes this "sluggishness", users report that their remote
> > is more "responsive".
> 
> That makes complete sense. I'm actually keen to get this lowered, since
> this makes it possible to lower the repeat period per-protocol, see
> commit d57ea877af38 which had to be reverted (the ite driver will
> need fixing up as well before this can happen).

I remember the commit, this issue hit us in LibreELEC testbuilds
as well :-)

> Lowering to below 125ms does increase the risk of regressions, so I
> am weary of that. Do you think there is benefit in doing this?

I'd also say stick to the 125ms default. The default settings
should always be safe ones IMO.

People who want to optimize for the last bit of performance can
easily do that on their own, at their own risk.

Personally I've been using gpio-ir-recv on RPi with the default 125ms
timeout and a Hauppauge rc-5 remote for about 2 years now and I've
always been happy with that.

I have to acknowledge though that the responsiveness of a remote
with short messages, like rc-5, in combination with a low timeout
(I tested down to 10ms) is pretty impressive.

so long,

Hias