[PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Wed, 2010-07-28 at 23:52 -0400, Jarod Wilson wrote: On Thu, Jul 29, 2010 at 02:40:43AM +0300, Maxim Levitsky wrote: Hi, This is second version of the patchset. Hopefully, I didn't forget to address all comments. In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. I just patched the entire series into a branch here and tested, no regressions with an mceusb transceiver with in-kernel decode, lirc decode or lirc tx. Only issue I had (which I neglected to mention earlier) was some pedantic issues w/whitespace. Here's the tree I built and tested: http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/maxim 7486d6ae3 addresses all the whitespace/formatting issues I had. Could either merge that into your patches, or I can just send it along as an additional patch after the fact. In either case, for 1-7 v2: About whitespace, I usually fix what checkpacth.pl tells me. Nothing beyond that :-) Tested-by: Jarod Wilson ja...@redhat.com I have no ene hardware to actually test with, but it did build. :) For 1-9 v2: Acked-by: Jarod Wilson ja...@redhat.com Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, Jul 29, 2010 at 06:30:28PM +0300, Maxim Levitsky wrote: On Wed, 2010-07-28 at 23:52 -0400, Jarod Wilson wrote: On Thu, Jul 29, 2010 at 02:40:43AM +0300, Maxim Levitsky wrote: Hi, This is second version of the patchset. Hopefully, I didn't forget to address all comments. In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. I just patched the entire series into a branch here and tested, no regressions with an mceusb transceiver with in-kernel decode, lirc decode or lirc tx. Only issue I had (which I neglected to mention earlier) was some pedantic issues w/whitespace. Here's the tree I built and tested: http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/maxim 7486d6ae3 addresses all the whitespace/formatting issues I had. Could either merge that into your patches, or I can just send it along as an additional patch after the fact. In either case, for 1-7 v2: About whitespace, I usually fix what checkpacth.pl tells me. Nothing beyond that :-) Yeah, I don't think any of them violate checkpatch.pl's rules, they were more for consistency with the rest of the code being patched. -- Jarod Wilson ja...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky Just for some context, the Conexant hardware generates such reports on it's hardware Rx FIFO: From section 3.8.2.3 of http://dl.ivtvdriver.org/datasheets/video/cx25840.pdf When the demodulated input signal no longer transitions, the RX pulse width timer overflows, which indicates the end of data transmission. When this occurs, the timer value contains all 1s. This value can be stored to the RX FIFO, to indicate the end of the transmission [...]. Additionally, a status bit is set which can interrupt the microprocessor, [...]. So the value in the hardware RX FIFO is the maximum time measurable given the current hardware clock divider settings, plus a flag bit indicating overflow. The CX2388[58] IR implementation currently translates that hardware notification into V4L2_SUBDEV_IR_PULSE_RX_SEQ_END: http://git.linuxtv.org/awalls/v4l-dvb.git?a=blob;f=drivers/media/video/cx23885/cx23888-ir.c;h=51f21636e639330bcf528568c0f08c7a4a674f42;hb=094fc94360cf01960da3311698fedfca566d4712#l678 which is defined here: http://git.linuxtv.org/awalls/v4l-dvb.git?a=blob;f=include/media/v4l2-subdev.h;h=bacd52568ef9fd17787554aa347f46ca6f23bdb2;hb=094fc94360cf01960da3311698fedfca566d4712#l366 as #define V4L2_SUBDEV_IR_PULSE_RX_SEQ_END 0x I didn't look too hard at it, but IIRC the in kernel decoders would have interpreted this value incorrectly (the longest possible mark). Instead, I just pass along the longest possible space: http://git.linuxtv.org/awalls/v4l-dvb.git?a=blob;f=drivers/media/video/cx23885/cx23885-input.c;h=3f924e21b9575f7d67d99d71c8585d41828aabfe;hb=094fc94360cf01960da3311698fedfca566d4712#l49 so it acts as in band signaling if anyone is looking for it, and the in kernel decoders happily treat it like a long space. With a little work, I could pass the actual time it took for the Rx timer to timeout as well (Provide the space measurement *and* the in band signal), if needed. Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 11:38 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky So, timeout report is just another sample, with a mark attached, that this is last sample? right? Christoph, right? In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. I think this is the best solution. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 19:26 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 11:38 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky So, timeout report is just another sample, with a mark attached, that this is last sample? right? On a measurement timeout, the Conexant hardware RX FIFO has this special timer overflow value in it as the last measurement: value = 0x1 = a mark with a measurement of 65535 * 4 clocks (and the measurement before this one in the FIFO is usually the last actual mark received). I ultimately translate that to pulse = false; /* a space */ duration = 0x7fff; /* 2.147 seconds */ to give the in kernel decoders a final space. What is lost is the actual space measurement by the hardware (whatever 65535 * 4 Rx clocks is), before the timeout. If LIRC likes to measure intertransmission gaps, what I am currently doing will not give LIRC a reasonable gap estimate/measurement, if the timeout is shorter than the actual gap. Christoph, right? In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. I think this is the best solution. I'm flexible. I don't know LIRC internals well enough to know what's best. I suspect sending a valid space measurement of the timeout, before the timeout report, may be useful for LIRC to obtain information on the gaps that are longer than the hardware timeout. Regards, Andy Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
Hi Andy, on 29 Jul 10 at 11:38, Andy Walls wrote: On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky Just for some context, the Conexant hardware generates such reports on it's hardware Rx FIFO: From section 3.8.2.3 of http://dl.ivtvdriver.org/datasheets/video/cx25840.pdf When the demodulated input signal no longer transitions, the RX pulse width timer overflows, which indicates the end of data transmission. When this occurs, the timer value contains all 1s. This value can be stored to the RX FIFO, to indicate the end of the transmission [...]. Additionally, a status bit is set which can interrupt the microprocessor, [...]. So the value in the hardware RX FIFO is the maximum time measurable given the current hardware clock divider settings, plus a flag bit indicating overflow. The CX2388[58] IR implementation currently translates that hardware notification into V4L2_SUBDEV_IR_PULSE_RX_SEQ_END: http://git.linuxtv.org/awalls/v4l-dvb.git?a=blob;f=drivers/media/video/cx238 85/cx23888-ir.c;h=51f21636e639330bcf528568c0f08c7a4a674f42;hb=094fc94360cf01 960da3311698fedfca566d4712#l678 which is defined here: http://git.linuxtv.org/awalls/v4l-dvb.git?a=blob;f=include/media/v4l2-subdev .h;h=bacd52568ef9fd17787554aa347f46ca6f23bdb2;hb=094fc94360cf01960da3311698f edfca566d4712#l366 as #define V4L2_SUBDEV_IR_PULSE_RX_SEQ_END 0x I didn't look too hard at it, but IIRC the in kernel decoders would have interpreted this value incorrectly (the longest possible mark). Instead, I just pass along the longest possible space: http://git.linuxtv.org/awalls/v4l-dvb.git?a=blob;f=drivers/media/video/cx238 85/cx23885-input.c;h=3f924e21b9575f7d67d99d71c8585d41828aabfe;hb=094fc94360c f01960da3311698fedfca566d4712#l49 so it acts as in band signaling if anyone is looking for it, and the in kernel decoders happily treat it like a long space. With a little work, I could pass the actual time it took for the Rx timer to timeout as well (Provide the space measurement *and* the in band signal), if needed. The value for LIRC_MODE2_TIMEOUT needs to be the exact value of the acutal time passed since the last pulse. When you just send the longest possible space instead, you'll make repeat detection impossible. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
Hi Maxim, on 29 Jul 10 at 17:41, Maxim Levitsky wrote: [...] Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? It all should be documented in the interface description. Jarod probably can point you where it can be found. Timeout reports can only be generated by the hardware because only the hardware can know the exact amount of time passed since the last pulse when any kind of buffering is used by the hardware. You see this esp. with USB devices. Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? For sure I will not agree to any constant introduced here. And I also don't see why. Can you explain why you are trying to change the lirc interface here? Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
Hi Maxim, on 29 Jul 10 at 19:26, Maxim Levitsky wrote: On Thu, 2010-07-29 at 11:38 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky So, timeout report is just another sample, with a mark attached, that this is last sample? right? No, a timeout report is just an additional hint for the decoder that a specific amount of time has passed since the last pulse _now_. [...] In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 19:15 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 19:26, Maxim Levitsky wrote: On Thu, 2010-07-29 at 11:38 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 02:40, Maxim Levitsky wrote: [...] In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? Best regards, Maxim Levitsky So, timeout report is just another sample, with a mark attached, that this is last sample? right? No, a timeout report is just an additional hint for the decoder that a specific amount of time has passed since the last pulse _now_. [...] In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Let me give an example to see if I got that right. Suppose we have this sequence of reports from the driver: 500 (pulse) 20 (timeout) 1 (space) 500 (pulse) Is that correct that time between first and second pulse is '10020' ? Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 18:58 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 17:41, Maxim Levitsky wrote: [...] Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Christoph Could you explain exactly how timeout reports work? It all should be documented in the interface description. Jarod probably can point you where it can be found. Timeout reports can only be generated by the hardware because only the hardware can know the exact amount of time passed since the last pulse when any kind of buffering is used by the hardware. You see this esp. with USB devices. In my case hardware doesn't have that capability. However, I though that timeout reports are useful to stop hardware as soon at timeout it hit. Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? For sure I will not agree to any constant introduced here. And I also don't see why. Can you explain why you are trying to change the lirc interface here? Currently, to comply with strict lirc requirements I have to send one big space between keypresses. Of course I can send it only when I get next pulse, which might happen much later. However, the in-kernel decoders depend on the last space to be sent right away. that it I need to and a keypress with a space, but currently it ends with pulse. So my idea was to wait reasonable time for next pulse, and if it doesn't arrive, send a space mark even though no new pulse is registered. Of course the size of that space can be configured. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 21:42 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: On Thu, 2010-07-29 at 18:58 +0200, Christoph Bartelmus wrote: Hi Maxim, on 29 Jul 10 at 17:41, Maxim Levitsky wrote: [...] Note that I send timeout report with zero value. I don't think that this value is importaint. This does not sound good. Of course the value is important to userspace and 2 spaces in a row will break decoding. Could you explain exactly how timeout reports work? It all should be documented in the interface description. Jarod probably can point you where it can be found. Timeout reports can only be generated by the hardware because only the hardware can know the exact amount of time passed since the last pulse when any kind of buffering is used by the hardware. You see this esp. with USB devices. In my case hardware doesn't have that capability. However, I thought that timeout reports are useful to stop hardware as soon as timeout is hit. You are starting a software timer for this? That's not the intention of timeout reports. It's just a hint to the decoder which needs to run it's own timer anyway. Having to stop the hardware is something very specific to your driver. Lirc interface isn't set to stone, so how about a reasonable compromise. After reasonable long period of inactivity (200 ms for example), space is sent, and then next report starts with a pulse. So gaps between keypresses will be maximum of 200 ms, and as a bonus I could rip of the logic that deals with remembering the time? For sure I will not agree to any constant introduced here. And I also don't see why. Can you explain why you are trying to change the lirc interface here? Currently, to comply with strict lirc requirements I have to send one big space between keypresses. Of course I can send it only when I get next pulse, which might happen much later. However, the in-kernel decoders depend on the last space to be sent right away. Ugh. What's the most polite way to express my disgust? ;) That what I think lately about that too. A space is really a space. After which a pulse is received. Therefore, lets just remove that cruft from in-kernel decoders? Anyway, even a 200 ms is still time. Its useless to add unnecessary latency to input. that it I need to and a keypress with a space, but currently it ends with pulse. So my idea was to wait reasonable time for next pulse, and if it doesn't arrive, send a space mark even though no new pulse is registered. Of course the size of that space can be configured. The reasonable time is protocol specific and must be handled by the decoder IMHO and not by the driver. It can't do that. Due to requirement of alternation between pulses and spaces, decoder doesn't see a hide nor hair of the last space, even though internally it keeps growing because hardware continues to send spaces. I am now confident that its just a decoder fault, and must be fixed. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 21:35 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: [...] Could you explain exactly how timeout reports work? [...] So, timeout report is just another sample, with a mark attached, that this is last sample? right? No, a timeout report is just an additional hint for the decoder that a specific amount of time has passed since the last pulse _now_. [...] In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Let me give an example to see if I got that right. Suppose we have this sequence of reports from the driver: 500 (pulse) 20 (timeout) 1 (space) 500 (pulse) Is that correct that time between first and second pulse is '10020' ? No, it's 1. The timeout is optional and just a hint to the decoder how much time has passed already since the last pulse. It does not change the meaning of the next space. its like a carrier report then I guess. Its clear to me now. So, I really don't need to send/support timeout reports because hw doesn't support that. I can however support timeout (LIRC_SET_REC_TIMEOUT) and and use it to adjust threshold upon which I stop the hardware, and remember current time. I can put that in generic function for ene like hardware (hw that sends small packs of samples very often) Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, Jul 29, 2010 at 11:04:47PM +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 21:35 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: [...] Could you explain exactly how timeout reports work? [...] So, timeout report is just another sample, with a mark attached, that this is last sample? right? No, a timeout report is just an additional hint for the decoder that a specific amount of time has passed since the last pulse _now_. [...] In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Let me give an example to see if I got that right. Suppose we have this sequence of reports from the driver: 500 (pulse) 20 (timeout) 1 (space) 500 (pulse) Is that correct that time between first and second pulse is '10020' ? No, it's 1. The timeout is optional and just a hint to the decoder how much time has passed already since the last pulse. It does not change the meaning of the next space. its like a carrier report then I guess. Its clear to me now. So, I really don't need to send/support timeout reports because hw doesn't support that. I can however support timeout (LIRC_SET_REC_TIMEOUT) and and use it to adjust threshold upon which I stop the hardware, and remember current time. I can put that in generic function for ene like hardware (hw that sends small packs of samples very often) So... I presume this means a v3 patchset? And/or, is it worth merging patches 1, 2, 3, 6 and 7 now, then having you work on top of that? -- Jarod Wilson ja...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, Jul 29, 2010 at 5:28 PM, Jarod Wilson ja...@redhat.com wrote: On Thu, Jul 29, 2010 at 11:04:47PM +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 21:35 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: [...] Could you explain exactly how timeout reports work? [...] So, timeout report is just another sample, with a mark attached, that this is last sample? right? No, a timeout report is just an additional hint for the decoder that a specific amount of time has passed since the last pulse _now_. [...] In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Let me give an example to see if I got that right. Suppose we have this sequence of reports from the driver: 500 (pulse) 20 (timeout) 1 (space) 500 (pulse) Is that correct that time between first and second pulse is '10020' ? No, it's 1. The timeout is optional and just a hint to the decoder how much time has passed already since the last pulse. It does not change the meaning of the next space. its like a carrier report then I guess. Its clear to me now. So, I really don't need to send/support timeout reports because hw doesn't support that. I can however support timeout (LIRC_SET_REC_TIMEOUT) and and use it to adjust threshold upon which I stop the hardware, and remember current time. I can put that in generic function for ene like hardware (hw that sends small packs of samples very often) So... I presume this means a v3 patchset? And/or, is it worth merging patches 1, 2, 3, 6 and 7 now, then having you work on top of that? This branch is a as-of-a-few-minutes-ago, up-to-date linuxtv staging/other plus a few outstanding patches and your patches 1, 2, 3, 6 and 7: http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/staging -- Jarod Wilson ja...@wilsonet.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 17:57 -0400, Jarod Wilson wrote: On Thu, Jul 29, 2010 at 5:28 PM, Jarod Wilson ja...@redhat.com wrote: On Thu, Jul 29, 2010 at 11:04:47PM +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 21:35 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: [...] Could you explain exactly how timeout reports work? [...] So, timeout report is just another sample, with a mark attached, that this is last sample? right? No, a timeout report is just an additional hint for the decoder that a specific amount of time has passed since the last pulse _now_. [...] In that case, lets do that this way: As soon as timeout is reached, I just send lirc the timeout report. Then next keypress will start with pulse. When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Let me give an example to see if I got that right. Suppose we have this sequence of reports from the driver: 500 (pulse) 20 (timeout) 1 (space) 500 (pulse) Is that correct that time between first and second pulse is '10020' ? No, it's 1. The timeout is optional and just a hint to the decoder how much time has passed already since the last pulse. It does not change the meaning of the next space. its like a carrier report then I guess. Its clear to me now. So, I really don't need to send/support timeout reports because hw doesn't support that. I can however support timeout (LIRC_SET_REC_TIMEOUT) and and use it to adjust threshold upon which I stop the hardware, and remember current time. I can put that in generic function for ene like hardware (hw that sends small packs of samples very often) So... I presume this means a v3 patchset? And/or, is it worth merging patches 1, 2, 3, 6 and 7 now, then having you work on top of that? This branch is a as-of-a-few-minutes-ago, up-to-date linuxtv staging/other plus a few outstanding patches and your patches 1, 2, 3, 6 and 7: I am surely send V3 and likely V4. I changed many of my patches, I now am chasing a very strange leak of samples I see. (sometimes, randomaly a sample goes missing, and that breaks in-kernel decoding...) It appears to be not my driver fault, nor fifo overflow... Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Fri, 2010-07-30 at 01:07 +0300, Maxim Levitsky wrote:
On Thu, 2010-07-29 at 17:57 -0400, Jarod Wilson wrote:
On Thu, Jul 29, 2010 at 5:28 PM, Jarod Wilson ja...@redhat.com wrote:
On Thu, Jul 29, 2010 at 11:04:47PM +0300, Maxim Levitsky wrote:
On Thu, 2010-07-29 at 21:35 +0200, Christoph Bartelmus wrote:
Hi!
Maxim Levitsky maximlevit...@gmail.com wrote:
[...]
Could you explain exactly how timeout reports work?
[...]
So, timeout report is just another sample, with a mark attached,
that
this is last sample? right?
No, a timeout report is just an additional hint for the decoder
that a
specific amount of time has passed since the last pulse _now_.
[...]
In that case, lets do that this way:
As soon as timeout is reached, I just send lirc the timeout report.
Then next keypress will start with pulse.
When timeout reports are enabled the sequence must be:
pulse timeout space pulse
where timeout is optional.
lircd will not work when you leave out the space. It must know the
exact
time between the pulses. Some hardware generates timeout reports
that are
too short to distinguish between spaces that are so short that the
next
sequence can be interpreted as a repeat or longer spaces which
indicate
that this is a new key press.
Let me give an example to see if I got that right.
Suppose we have this sequence of reports from the driver:
500 (pulse)
20 (timeout)
1 (space)
500 (pulse)
Is that correct that time between first and second pulse is
'10020' ?
No, it's 1. The timeout is optional and just a hint to the
decoder
how much time has passed already since the last pulse. It does not
change
the meaning of the next space.
its like a carrier report then I guess.
Its clear to me now.
So, I really don't need to send/support timeout reports because hw
doesn't support that.
I can however support timeout (LIRC_SET_REC_TIMEOUT) and and use it to
adjust threshold upon which I stop the hardware, and remember current
time.
I can put that in generic function for ene like hardware
(hw that sends small packs of samples very often)
So... I presume this means a v3 patchset? And/or, is it worth merging
patches 1, 2, 3, 6 and 7 now, then having you work on top of that?
This branch is a as-of-a-few-minutes-ago, up-to-date linuxtv
staging/other plus a few outstanding patches and your patches 1, 2, 3,
6 and 7:
I am surely send V3 and likely V4.
I changed many of my patches,
I now am chasing a very strange leak of samples I see. (sometimes,
randomaly a sample goes missing, and that breaks in-kernel decoding...)
It appears to be not my driver fault, nor fifo overflow...
Rolls eyes
void ir_raw_event_handle(struct input_dev *input_dev)
{
struct ir_input_dev *ir = input_get_drvdata(input_dev);
if (!ir-raw)
return;
schedule_work(ir-raw-rx_work);
}
EXPORT_SYMBOL_GPL(ir_raw_event_handle);
This is workqueue, so who said two of them can run at same time.
Best regards,
Maxim Levitsky
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, 2010-07-29 at 19:15 +0200, Christoph Bartelmus wrote: on 29 Jul 10 at 19:26, Maxim Levitsky wrote: On Thu, 2010-07-29 at 11:38 -0400, Andy Walls wrote: On Thu, 2010-07-29 at 17:41 +0300, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:23 +0200, Christoph Bartelmus wrote: Hi Christoph, Mauro, and Jarrod When timeout reports are enabled the sequence must be: pulse timeout space pulse where timeout is optional. OK. This is some of the detail I needed. I'm looking at the master branch of Jarrod's wip git repo, but: 1. I don't see how a timeout report is going to make it through drivers/media/IR/ir-lirc-codec.c:lir_lirc_decode() I guess I'll have to add a flags field (or something) to include/media/ir_core.h:struct ir_raw_event to modify the events beyond just mark or space reports. Mauro, do you have any preferences or comments here? 2. The in-kernel decoders normally need a the final space to close out the decoding, so the above sequence will cause them to wait. It would only really be noticiable if no repeat came from the remote, but it still would be annoying. If I send a space for the timeout to get the in kernel decoders to close out decoding, then I end up sending a double space out to LIRC, which I just read will confuse LIRC. So... If I'm going to add a timeout event flag to struct ir_raw_event, I suppose we could either: a. have the current in kernel decoders interpret an ir_raw_event with a timeout event as a cue to conclude decoding the current pulse train b. add a finish decode flag and have the in kernel decoders respond to that. I beleive this will also let the in-kernel decoders still respond to final space as they currently do. Objections? comments? 3. When my hardware times out, it stops measuring anything, until it sees a new edge. For short timeout settings (smaller than the intertransmission gap), I will have generate a space in software to provide the length of the gap. I'll have to store the time of reading the timeout flag of the hardware Rx FIFO, compute an approximate gap length when the next mark measurement comes in, and insert a the space. The gaps space time will only be approximate, as the Rx FIFO watermark is set to interrupt at 4 measurements in the FIFO. If I can't do #1 #2 above, I'm not sure how I can send any in band signaling out to user space. Regards, Andy lircd will not work when you leave out the space. It must know the exact time between the pulses. Some hardware generates timeout reports that are too short to distinguish between spaces that are so short that the next sequence can be interpreted as a repeat or longer spaces which indicate that this is a new key press. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/9 v2] IR: few fixes, additions and ENE driver
Hi, This is third revision of my patchset. Notable changes: * Added whitespace fixes from Jarod Wilson * 4 new bugs fixed (patches 04-07). Now in-kernel decoding works perfectly with all protocols it supports. * lirc interface additions cleaned up. no more wrong support for timeout reports new ioctl for learning mode still need to add carrier detect, timeout reports, and rx filter * replaced int with bool in my driver, plus few cleanups. * added myself to maintainers of the ene driver * added another PNP ID to ene driver Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9 v2] IR: few fixes, additions and ENE driver
On Thu, Jul 29, 2010 at 02:40:43AM +0300, Maxim Levitsky wrote: Hi, This is second version of the patchset. Hopefully, I didn't forget to address all comments. In addition to comments, I changed helper function that processes samples so it sends last space as soon as timeout is reached. This breaks somewhat lirc, because now it gets 2 spaces in row. However, if it uses timeout reports (which are now fully supported) it will get such report in middle. Note that I send timeout report with zero value. I don't think that this value is importaint. I just patched the entire series into a branch here and tested, no regressions with an mceusb transceiver with in-kernel decode, lirc decode or lirc tx. Only issue I had (which I neglected to mention earlier) was some pedantic issues w/whitespace. Here's the tree I built and tested: http://git.wilsonet.com/linux-2.6-ir-wip.git/?a=shortlog;h=refs/heads/maxim 7486d6ae3 addresses all the whitespace/formatting issues I had. Could either merge that into your patches, or I can just send it along as an additional patch after the fact. In either case, for 1-7 v2: Tested-by: Jarod Wilson ja...@redhat.com I have no ene hardware to actually test with, but it did build. :) For 1-9 v2: Acked-by: Jarod Wilson ja...@redhat.com -- Jarod Wilson ja...@redhat.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
