Em Mon, 7 Jun 2021 00:06:36 +0900
Seongyong Park <euphoriccatface@xxxxxxxxx> escreveu:
> `2021년 6월 6일 (일) 오후 8:00, Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>님이 작성:
> >
> >
> > Perhaps something like this would work better, keeping a more precise
> > average fps rate:
> >
> > next_jiffies = jiffies + delay;
> > do {
> > ...
> > schedule_delay_us = jiffies_to_usecs(next_jiffies - jiffies);
> > usleep_range(schedule_delay_us * 3/4, schedule_delay_us); ...
> > next_jiffies += delay;
> > }
> >
> > >
> > > I'll send another patchset if it doesn't look too bad.
> > >
> > > Thank you very much.
> > > Seongyong Park
> >
> >
> >
> > Thanks,
> > Mauro
>
> For a few hours, I couldn't get the module to make precise timing.
> It would always be a few tenths of FPS higher than I set, regardless
> of how I construct the calculation.
> And then it hit me, maybe jiffies is not granular enough - and of
> course, resolution of jiffies turns out to be 100Hz :P
It is actually worse than that, as it depends on CONFIG_HZ, which is
set by the one who built the Kernel. So, on other machines, it could
be higher (like 1200) or lower (24 is one of the options on mips) ;-)
> e.g. If you set it 16FPS, the count of jiffies to sleep results 100 / 16 = 6
> The sleep interval ends up being 60ms, resulting in 16.666... FPS.
That's basically why I suggested you to count the time from the start
of streaming, instead of just waiting for a fixed delay every time.
> This discrepancy gets quite big if you set it to 64 FPS, resulting in
> a single jiffies count, i.e. 100Hz.
> (Though you will need to either skip some data, or set I2C baud rate
> beyond the sensor's spec
> in order to realistically even achieve 64 FPS, at least on an RPi Zero
> it seems.)
The issue is probably not RPi zero, but the low speeds of I2C bus,
and the speed of the sensor.
> So, I basically changed the time source from `jiffies` to
> `ktime_to_us(ktime_get())`.
> The timing is definitely better now :)
This helps but it probably use a lot more CPU cycles than reading
jiffies.
> One last question please, before creating another patchset.
> > usleep_range(schedule_delay_us * 3/4, schedule_delay_us);
> Is it okay to make it essentially 0 microseconds sleep here, by
> setting `schedule_delay_us` to 0?
> It's going to be like,
> ```
> if (current_us > end_us) {
> end_us = current_us;
> }
> schedule_delay_us = end_us - current_us;
> ```
Not sure, but you could instead revert the logic, like:
if (current_us <= end_us)
usleep_range();
Thanks,
Mauro
