On Sat, May 29, 2010 at 4:01 PM, Andy Walls <awalls@xxxxxxxxxxxxxxxx> wrote: > On Sat, 2010-05-29 at 12:58 -0400, Jarod Wilson wrote: >> On Sat, May 29, 2010 at 8:39 AM, Andy Walls <awalls@xxxxxxxxxxxxxxxx> wrote: >> > On Fri, 2010-05-28 at 00:47 -0400, Jarod Wilson wrote: >> >> So I'm inching closer to a viable mceusb driver submission -- both a >> >> first-gen and a third-gen transceiver are now working perfectly with >> >> multiple different mce remotes. However, that's only when I make sure >> >> the mceusb driver is loaded w/only the rc6 decoder loaded. When >> >> ir-core comes up, it requests all decoders to load, starting with the >> >> nec decoder, followed by the rc5 decoder, then the rc6 decoder and so >> >> on (init_decoders() in ir-raw-event.c). When I call >> >> ir_raw_event_handle, all decoders get run on the ir data buffer, >> >> starting with nec. Well, the nec decoder doesn't like the rc6 data, so >> >> it pukes. The RUN_DECODER macro break's out of the routine when that >> >> happens, and the rc6 decoder never gets a chance to run. (Similarly, >> >> if only ir-nec-decoder has been removed, the rc5 decoder pukes on the >> >> rc6 data, same problem). >> > >> > Yes, if the system kernel is going to attempt to discriminate between >> > various input singals, it needs to let all its "correlators" run and >> > produce a "confidence" number from each. >> > >> > Then ideally one would take the result with the highest confidence. >> > >> > Right now it looks like all the confidence determinations are boolean (0 >> > or -EINVAL) and there is no chance to deal with the case that two >> > different decoders validly decode something. The first decoder that >> > declares a match "wins" and sends an event. >> >> Yeah, it does look that way. I wonder how likely it is that e.g. a >> valid RC6 signal would be decoded to something by say the NEC decoder, > > NEC is a pulse position code and RC-6 is manchester encoded, so that > particular case would be unlikely. > > I would think one would have a better chance of false positiive > detections between similar encoding types: pulse position, pulse width, > or manchester. > > Looking at slide 11 of this: > > http://www.audiodevelopers.com/temp/Remote_Controls.ppt > > It looks like the pulse position protocols with a header space of 8T > (where 8T is about 4ms) would be the only ones that could get confused. > > Since these are streaming decoders, it looks like JVC would come up with > false detections first, since it has the shortest payload of the pulse > position protocols. I think JVC will always claim to decode an NEC > pulse train. (I'll try to test that sometime.) > > >> with a resulting value that matched an entry in the (RC6) keymap >> loaded for the remote... Certainly seems like something that *could* >> happen somehow, but probably unlikely? I dunno... > > I don't know either. There appears to be a chance for the first 16 bits > of a transmitted NEC (Addr:Addr') or Extended NEC (AddrHi:AddrLo) > sequence, to be interpreted as JVC (Addr:Cmd), and the JVC decoder > matching a scancode in the keytable for the NEC remote. > > > > >> 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. -- 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
