From: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Seongyong Park <euphoriccatface@gmail.com>
Cc: linux-media <linux-media@vger.kernel.org>,
Matt Ranostay <matt.ranostay@konsulko.com>
Subject: Re: [PATCH 1/2] media: video-i2c: frame delay based on last frame's end time
Date: Sun, 6 Jun 2021 21:18:49 +0200 [thread overview]
Message-ID: <20210606211849.70366d0d@coco.lan> (raw)
In-Reply-To: <CAJp=mWTwRePsk6ACr7Jc1uDOwNe60hVAyneh3VFU_LYii6M=Bw@mail.gmail.com>
Em Mon, 7 Jun 2021 00:06:36 +0900
Seongyong Park <euphoriccatface@gmail.com> escreveu:
> `2021년 6월 6일 (일) 오후 8:00, Mauro Carvalho Chehab <mchehab@kernel.org>님이 작성:
> >
> >
> > 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
prev parent reply other threads:[~2021-06-06 19:18 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-05 11:54 [PATCH V2 0/2] media: video-i2c: additional support for Melexis MLX90640 Seongyong Park
2021-05-16 11:09 ` [PATCH 1/2] media: video-i2c: frame delay based on last frame's end time Seongyong Park
2021-05-16 11:09 ` [PATCH 2/2] media: video-i2c: append register data on MLX90640's frame Seongyong Park
2021-05-16 20:48 ` Matt Ranostay
2021-05-17 1:39 ` Seongyong Park
2021-05-17 23:40 ` Matt Ranostay
2021-05-16 20:13 ` [PATCH 1/2] media: video-i2c: frame delay based on last frame's end time Matt Ranostay
2021-05-19 3:45 ` [PATCH V2 2/2] media: video-i2c: append register data on MLX90640's frame Seongyong Park
2021-06-05 11:54 ` Seongyong Park
2021-06-05 11:54 ` [PATCH 1/2] media: video-i2c: frame delay based on last frame's end time Seongyong Park
2021-06-05 14:00 ` Mauro Carvalho Chehab
2021-06-05 14:53 ` Mauro Carvalho Chehab
2021-06-06 7:20 ` Seongyong Park
2021-06-06 11:00 ` Mauro Carvalho Chehab
2021-06-06 15:06 ` Seongyong Park
2021-06-06 19:18 ` Mauro Carvalho Chehab [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210606211849.70366d0d@coco.lan \
--to=mchehab@kernel.org \
--cc=euphoriccatface@gmail.com \
--cc=linux-media@vger.kernel.org \
--cc=matt.ranostay@konsulko.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).