On Sun, May 30, 2010 at 10:02 AM, Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> wrote: > Em 29-05-2010 23:24, Jarod Wilson escreveu: >> On Sat, May 29, 2010 at 4:01 PM, Andy Walls <awalls@xxxxxxxxxxxxxxxx> wrote: ... >>>> We do have the >>>> option to disable all but the relevant protocol handler on a >>>> per-device basis though, if that's a problem. Hrm, the key tables also >>>> have a protocol tied to them, not sure if that's taken into account >>>> when doing matching... Still getting to know the code. :) >>> >>> It does not look like >>> >>> ir_keydown() >>> ir_g_keycode_from_table() >>> ir_getkeycode() >>> >>> bother to check the ir_type (e.g. IR_TYPE_NEC) of the keymap against the >>> decoders type. Neither do the decoders themselves. >>> >>> >>> If a decoder decodes something and thinks its valid, it tries to send a >>> key event with ir_keydown(). ir_keydown() won't send a key event if the >>> lookup comes back KEY_RESERVED, but it doesn't tell the decoder about >>> the failure to find a key mapping. A decoder can come back saying it >>> did it's job, without knowing whether or not the decoding corresponded >>> to a valid key in the loaded keymap. :( >>> >>> >>>>> You will have to deal with the case that two or more decoders may match >>>>> and each sends an IR event. (Unless the ir-core already deals with this >>>>> somehow...) >>>> >>>> Well, its gotta decode correctly to a value, and then match a value in >>>> the loaded key table for an input event to get sent through. At least >>>> for the RC6 MCE remotes, I haven't seen any of the other decoders take >>>> the signal and interpret it as valid -- which ought to be by design, >>>> if you consider that people use several different remotes with varying >>>> ir signals with different devices all receiving them all the time >>>> without problems (usually). And if we're not already, we could likely >>>> add some logic to give higher precedence to values arrived at using >>>> the protocol decoder that matches the key table we've got loaded for a >>>> given device. >>> >>> After looking at things, the only potential problem I can see right now >>> is with the JVC decoder and NEC remotes. >>> >>> I think that problem is most easily eliminated either by >>> >>> a. having ir_keydown() (or the functions it calls) check to see that the >>> decoder matches the loaded keymap, or >>> >>> b. only calling the decoder that matches the loaded keymap's protocol >>> >>> Of the above, b. saves processor cycles and frees up the global >>> ir_raw_handler spin lock sooner. That spin lock is serializing pulse >>> decoding for all the IR receivers in the system (pulse decoding can >>> still be interleaved, just only one IR receiver's pulses are be >>> processed at any time). What's the point of running decoders that >>> should never match the loaded keymap? >> >> For the daily use case where a known-good keymap is in place, I'm >> coming to the conclusion that there's no point, we're only wasting >> resources. For initial "figure out what this remote is" type of stuff, >> running all decoders makes sense. One thought I had was that perhaps >> we start by running through the decoder that is listed in the keymap. >> If it decodes to a scancode and we find a valid key in the key table >> (i.e., not KEY_RESERVED), we're done. If decoding fails or we don't >> find a valid key, then try the other decoders. However, this is >> possibly also wasteful -- most people with any somewhat involved htpc >> setup are going to be constantly sending IR signals intended for other >> devices that we pick up and try to decode. >> >> So I'd say we go with your option b, and only call the decoder that >> matches the loaded keymap. One could either pass in a modparam or >> twiddle a sysfs attr or use ir-keytable to put the receiver into a >> mode that called all decoders -- i.e., set protocol to >> IR_TYPE_UNKNOWN, with the intention being to figure it out based on >> running all decoders, and create a new keymap where IR_TYPE_FOO is >> known. > > There's no need to extra parameters. Decoders can be disabled by userspace, > per each rc sysfs node. Btw, the current version of ir-keytable already sets > the enabled protocols based on the protocol reported by the rc keymap. > > What it makes sense is to add a patch at RC core that will properly enable/disable > the protocols based on IR_TYPE, when the rc-map is stored in-kernel. Ah, yeah, that does make sense. And if we add that, ir-keytable doesn't actually have to worry about doing similar itself any longer. If you're not already working on it, I can try to whip something up, though I'm knee-deep in an ir-lirc-codec bridge right now... -- Jarod Wilson jarod@xxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
