Hi Mauro, On Wed, Feb 14, 2018 at 04:44:48PM -0200, Mauro Carvalho Chehab wrote: > Hi Sean, > > Em Mon, 12 Feb 2018 20:03:18 +0000 > Sean Young <sean@xxxxxxxx> escreveu: > > > Hi Mauro, > > > > Just very minor changes this time (other stuff is not ready yet). I would > > really appreciate if you could cast an extra critical eye on the commit > > "no need to check for transitions", just to be sure it is the right change. > > Did you send all patches in separate? This is important to allow us > to comment on an specific issue inside a patch... All the patches were emailed to linux-media, some of them on the same day as the pull request. Maybe I should wait longer. The patch below was sent out on the 28th of January. > > media: rc: no need to check for transitions > > I don't remember the exact reason for that, but, as far as I > remember, on a few devices, a pulse (or space) event could be > broken into two consecutive events of the same type, e. g., > a pulse with a 125 ms could be broken into two pulses, like > one with 100 ms and the other with 25 ms. If that is the case, then the IR decoders could not deal with this anyway. For example, the first state transition rc6 is: if (!eq_margin(ev.duration, RC6_PREFIX_PULSE, RC6_UNIT)) So if ev.duration is not the complete duration, then decoding will fail; I tried to explain in the commit message that if this was the case, then decoding would not work so the check was unnecessary. > That's said, I'm not sure if the current implementation are > adding the timings for both pulses into a single one. That depends on whether the driver uses ir_raw_event_store() or ir_raw_event_store_with_filter(). The latter exists precisely for this reason. > For now, I'll keep this patch out of the merge. Ok. So in summary, I think: 1. Any driver which produces consequentive pulse events is broken and should be fixed; 2. The IR decoders cannot deal with consequentive pulses and the current prev_ev code does not help with this (possibly in very special cases). Sean
