Re: Inconsistent RC5 ir-keytable events

Marko Mäkelä
2022-04-10T14:08:02Z

Hi Sean,

Were you able to repeat the problem of bogus keyup events (and lost
key-repeat events) when holding down a remote control button for several
seconds?

I tried to understand your two commits and how the subsystem works. No
new timer callback is being declared in the patch. Basically, only the
field dvb_usb_rc::timeout is being split into rawir_timeout and
keyup_delay. For all other drivers except rtl28xxu.c they are being set
to identical values.

I could not figure out what would invoke rtl2832u_rc_query() that is
actually reading the IR data from the USB driver. As far as I can tell,
it is feeding the data to the LIRC driver (in a separate thread) via
ir_raw_event_store_with_filter() and ir_raw_event_handle(). The
ir_timer_keyup() and ir_timer_repeat() are presumably called from a
timer handler thread.

Because the LIRC events were seemingly always reported consistently, the
problem should either be some kind of a glitch between rc-ir-raw.c and
the decoder, or some glitch in the ir_raw_event data that is being
supplied to the decoder. I was thinking if the problem could reside here
in rtl2832u_rc_query():

diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index a83b1107fc7f..3d292a351403 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -1802,9 +1802,13 @@ static int rtl2832u_rc_query(struct dvb_usb_device *d)
}

/* pass data to Kernel IR decoder */
- for (i = 0; i < len; i++) {
+ for (i = 0; i < len; ) {
ev.pulse = buf[i] >> 7;
- ev.duration = 51 * (buf[i] & 0x7f);
+ ev.duration = buf[i] & 0x7f;
+ while (++i < len && ev.pulse == buf[i] >> 7) {
+ ev.duration += buf[i] & 0x7f;
+ }
+ ev.duration *= 51;
ir_raw_event_store_with_filter(d->rc_dev, &ev);
}

My idea was to avoid sending multiple events for a single transition
that for some reason was split into multiple bytes in the buffer. But,
this did not help at all. Holding down the Volume+ or Volume- button
would still fail to turn the volume all the way up or down in the GNOME
desktop, without any pauses.

This would seem to suggest some glitch between ir_raw_event_thread() and
the decoder, that is, ir_rc5_decode() or ir_nec_decode(), and the keyup
timeout.

I have two ideas how to avoid bogus keyup events:

(1) Extend the keyup timeout on every ir_raw_event_handle().
(2) In the keyup callback, reschedule the timer a little later if some
incompletely decoded raw events remain in the buffers.

I think that (1) is simpler.

Marko