Re: [GIT PULL FOR v4.17] rc changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 15, 2018 at 09:16:18AM -0200, Mauro Carvalho Chehab wrote:
> Em Wed, 14 Feb 2018 21:32:07 +0000
> Sean Young <sean@xxxxxxxx> escreveu:
> 
> > 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  
> 
> No need to wait longer. Yet, it seems that I lost the above patch, as I
> couldn't find anything on my email with the above subject.
> 
> Perhaps the e-mail got lost somehow on my inbox.

Actually, this is my bad. I changed the subject line of the commit after
sending it to the list, without resending it. I should have sent out a v2.

> > > 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.
> 
> OK.
> 
> > 
> > > 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;
> 
> Agreed.
> 
> > 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).
> 
> Ok. Yet, maybe it would worth to produce a warning if this happen, and
> reset the state machine, as it would help to identify problems at
> the driver or at the hardware.

That's not a bad idea. I will look a this.

Thanks
Sean



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux