Improve latency of IR decoding

The current IR decoding is much too slow. Many IR protocols rely on a trailing space for decoding (e.g. rc-6 needs to know when the bits end). The trailing space is generated by the IR timeout, and if this is longer than required, keys can be perceived as sticky and slugish.

The other issue the keyup timer. IR has no concept of a keyup message, this is implied by the absence of IR. So, minimising the timeout for this further improves the handling.

With these patches in place, using IR with the builtin decoders is much improved and feels very snappy.

Sean Young (3):
media: rc: set timeout to smallest value required by enabled protocols
media: rc: add ioctl to get the current timeout
media: rc: per-protocol repeat period and minimum keyup timer

 Documentation/media/uapi/rc/lirc-func.rst          |  1 +
 .../media/uapi/rc/lirc-set-rec-timeout.rst         | 14 +++--
 drivers/media/cec/cec-core.c                       |  2 +-
 drivers/media/rc/ir-imon-decoder.c                 |  1 +
 drivers/media/rc/ir-jvc-decoder.c                  |  1 +
 drivers/media/rc/ir-mce_kbd-decoder.c              |  1 +
 drivers/media/rc/ir-nec-decoder.c                  |  1 +
 drivers/media/rc/ir-rc5-decoder.c                  |  1 +
 drivers/media/rc/ir-rc6-decoder.c                  |  1 +
 drivers/media/rc/ir-sanyo-decoder.c                |  1 +
 drivers/media/rc/ir-sharp-decoder.c                |  1 +
 drivers/media/rc/ir-sony-decoder.c                 |  1 +
 drivers/media/rc/ir-xmp-decoder.c                  |  1 +
 drivers/media/rc/lirc_dev.c                        |  9 ++-
 drivers/media/rc/rc-core-priv.h                    |  1 +
 drivers/media/rc/rc-ir-raw.c                       | 31 +++++++++-
 drivers/media/rc/rc-main.c                         | 68 +++++++++++-----------
 include/uapi/linux/lirc.h                          |  6 ++
 18 files changed, 101 insertions(+), 41 deletions(-)

Signature · 1 line

2.14.3

Hi Sean,

Quoted 11 lines · Sean Young

The current IR decoding is much too slow. Many IR protocols rely on a trailing space for decoding (e.g. rc-6 needs to know when the bits end). The trailing space is generated by the IR timeout, and if this is longer than required, keys can be perceived as sticky and slugish.

The other issue the keyup timer. IR has no concept of a keyup message, this is implied by the absence of IR. So, minimising the timeout for this further improves the handling.

With these patches in place, using IR with the builtin decoders is much improved and feels very snappy.

thanks a lot for the patches!

I didn't have much time to test yet, but quick checks on Amlogic/meson-ir (kernel 4.16-rc7 + media tree + your patches) and Raspberry Pi (RPi foundation kernel 4.14 + my backport of your patches) look really promising.

I found one issue, though, in ir-sharp-decoder.c max_space must be set to SHARP_ECHO_SPACE - otherwise we get a timeout between the normal and inverted message part and decoding fails.

One thing I'm wondering is if the keyup timer marging might be too tight now. Basically we have just the fixed 10ms marging from idle timeout. The repeat periods of the protocols are rather accurate/strict, so (programmable) remotes not sticking to the official timing might cause repeated keyup/down events if they are repeating a tad to slow.

I'm not sure if this could be an issue, but maybe we should add a safety margin to the repeat periods as well? For example 10 or 20 percent of the specced repeat periods. What do you think?

To get some more test coverage I've asked my colleague to include my backport patch in the LibreELEC testbuilds for x86 and RPi. We've got some 500 regular users of these so if something's not working we should find out soon.

This is in my nightly test builds since 28 March, and no problems reported so far.

On my NUC with Harmony One/RC6 remote these commits are working just fine.

I just hope I
didn't mess up the backport... Here's the link to my 4.14 patch:

https://github.com/HiassofT/LibreELEC.tv/blob/le9-ir-latency/packages/linux/patches/default/linux-999-improve-ir-timeout-handling.patch

so long & thanks,

Hias

Quoted 29 lines

Sean Young (3):
media: rc: set timeout to smallest value required by enabled protocols
media: rc: add ioctl to get the current timeout
media: rc: per-protocol repeat period and minimum keyup timer

 Documentation/media/uapi/rc/lirc-func.rst          |  1 +
 .../media/uapi/rc/lirc-set-rec-timeout.rst         | 14 +++--
 drivers/media/cec/cec-core.c                       |  2 +-
 drivers/media/rc/ir-imon-decoder.c                 |  1 +
 drivers/media/rc/ir-jvc-decoder.c                  |  1 +
 drivers/media/rc/ir-mce_kbd-decoder.c              |  1 +
 drivers/media/rc/ir-nec-decoder.c                  |  1 +
 drivers/media/rc/ir-rc5-decoder.c                  |  1 +
 drivers/media/rc/ir-rc6-decoder.c                  |  1 +
 drivers/media/rc/ir-sanyo-decoder.c                |  1 +
 drivers/media/rc/ir-sharp-decoder.c                |  1 +
 drivers/media/rc/ir-sony-decoder.c                 |  1 +
 drivers/media/rc/ir-xmp-decoder.c                  |  1 +
 drivers/media/rc/lirc_dev.c                        |  9 ++-
 drivers/media/rc/rc-core-priv.h                    |  1 +
 drivers/media/rc/rc-ir-raw.c                       | 31 +++++++++-
 drivers/media/rc/rc-main.c                         | 68 +++++++++++-----------
 include/uapi/linux/lirc.h                          |  6 ++
 18 files changed, 101 insertions(+), 41 deletions(-)

Signature · 1 line

2.14.3

Quoted 17 lines · Matthias Reichl

Hi Sean,

Quoted 11 lines · Sean Young

The current IR decoding is much too slow. Many IR protocols rely on a trailing space for decoding (e.g. rc-6 needs to know when the bits end). The trailing space is generated by the IR timeout, and if this is longer than required, keys can be perceived as sticky and slugish.

The other issue the keyup timer. IR has no concept of a keyup message, this is implied by the absence of IR. So, minimising the timeout for this further improves the handling.

With these patches in place, using IR with the builtin decoders is much improved and feels very snappy.

thanks a lot for the patches!

Quoted 8 lines

I didn't have much time to test yet, but quick checks on Amlogic/meson-ir (kernel 4.16-rc7 + media tree + your patches) and Raspberry Pi (RPi foundation kernel 4.14 + my backport of your patches) look really promising.

I found one issue, though, in ir-sharp-decoder.c max_space must be set to SHARP_ECHO_SPACE - otherwise we get a timeout between the normal and inverted message part and decoding fails.

Quoted 6 lines

One thing I'm wondering is if the keyup timer marging might be too tight now. Basically we have just the fixed 10ms marging from idle timeout. The repeat periods of the protocols are rather accurate/strict, so (programmable) remotes not sticking to the official timing might cause repeated keyup/down events if they are repeating a tad to slow.

Quoted 8 lines

I'm not sure if this could be an issue, but maybe we should add a safety margin to the repeat periods as well? For example 10 or 20 percent of the specced repeat periods. What do you think?

To get some more test coverage I've asked my colleague to include my backport patch in the LibreELEC testbuilds for x86 and RPi. We've got some 500 regular users of these so if something's not working we should find out soon.

Quick update: testing so far went really smooth, no issues reported since we included the backport patch in LibreELEC testbuilds.

Quote from https://github.com/LibreELEC/LibreELEC.tv/pull/2623#issuecomment-377897518

Quoted 3 lines

This is in my nightly test builds since 28 March, and no problems reported so far.

On my NUC with Harmony One/RC6 remote these commits are working just fine.

I've been using LibreELEC on RPi with the patch (using a gpio ir receiver and a Hauppauge RC-5 remote) since then and everything was fine as well.