Re: [PATCH RESEND v2 0/2] media: rc: gpio-ir-recv: add timeout property

[Sean Young <sean@mess.org>]

On Tue, Nov 10, 2020 at 01:48:05PM +0100, Michael Klein wrote:
> On Tue, Nov 10, 2020 at 10:17:27AM +0000, Sean Young wrote:
> > On Mon, Nov 09, 2020 at 04:23:09PM +0100, Michael Klein wrote:
> > > The default recorder timeout of 125ms is too high for some BPF protocol
> > > decoders when a remote sends repeat codes at high rates. This makes the
> > > timeout configurable via the devicetree.
> > 
> > To be honest, 125ms is too much by any measurement. The longest space
> > in any protocol I'm aware of is 40ms in the sharp ir protocol. I think
> > changing IR_DEFAUL_TIMEOUT to something like 50ms would make sense.
> 
> Seconded. I'm happy to prepare a patch if changing the default value is
> acceptable.

Actually I don't understand why the high timeout is an issue. It means that
between ir messages you don't get a LIRC_TIMEOUT, just a LIRC_SPACE. Why is
this a problem?

I'm not opposed to such a patch, but we would need to know if it really
solves the problem you are having and it would need to sit in linux-next
for some time.

> > Also, when an BPF protocol is loaded, user-space can set the timeout
> > with the LIRC_SET_REC_TIMEOUT ioctl which can depend on the protocol
> > (set to longest space + 10ms error margin).
> 
> Right, although this is a bit cumbersome with current user-space tools. The
> BPF is loaded with ir-keytable, while the recorder timeout needs to be set
> with it-ctl. In the Debian world, those tools are even in different
> packages.

ir-keytable can use the LIRC_SET_REC_TIMEOUT ioctl to adjust the timeout.
It has opened the lirc device already to load the bpf program. ir-keytable
would need to calculate the minimum timeout needed for all enabled protocols
(bpf and non-bpf). Then it can simply do the ioctl.

> > This would mean that the
> > bare minimum timeout can be set, which means decoding is as responsive
> > as can be.
> > 
> > I'm not sure that device tree is really the place for this.
> 
> Not arguing about this, but IMHO no less than for rc-map-name. So this seems
> to be at least consistent.

Well, I guess it can be argued. However, it can also be argued that
it is not the best solution for this problem.


Sean