public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: "Marko Mäkelä" <marko.makela@iki.fi>
To: Sean Young <sean@mess.org>
Cc: linux-media@vger.kernel.org
Subject: Re: Inconsistent RC5 ir-keytable events
Date: Sun, 10 Apr 2022 17:07:50 +0300	[thread overview]
Message-ID: <YlLktqqcX0i38g14@jyty> (raw)
In-Reply-To: <Yj9fArSg/fFU2MB7@jyty>

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

      reply	other threads:[~2022-04-10 14:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <YdKdPosyzj2urFpS@jyty>
2022-01-03  9:21 ` Inconsistent RC5 ir-keytable events Sean Young
2022-01-03 10:35   ` Marko Mäkelä
2022-01-03 11:07     ` Sean Young
2022-01-03 12:21       ` Marko Mäkelä
2022-01-04 16:07         ` Marko Mäkelä
2022-01-05  9:53           ` Sean Young
2022-01-06 11:41             ` Marko Mäkelä
2022-01-13 16:55               ` Sean Young
2022-01-14  6:31                 ` Marko Mäkelä
2022-01-29 17:15                   ` Marko Mäkelä
2022-02-08 16:46                     ` Sean Young
2022-02-09  8:12                       ` Marko Mäkelä
2022-02-12 11:16                         ` Sean Young
2022-02-12 16:34                           ` Sean Young
2022-02-13 17:12                             ` Marko Mäkelä
2022-03-26 18:44                               ` Marko Mäkelä
2022-04-10 14:07                                 ` Marko Mäkelä [this message]

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=YlLktqqcX0i38g14@jyty \
    --to=marko.makela@iki.fi \
    --cc=linux-media@vger.kernel.org \
    --cc=sean@mess.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