-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use requestAnimationFrame() instead of setTimeout/setInterval() #24
base: master
Are you sure you want to change the base?
Conversation
Interesting, haven't looked at the demo yet but it's not immediately obvious why cpu usage would be lower with this? Won't using raf mean we go from calculating ffts every 100ms (by default) to every frame (16ms) which should be more work no? |
The CPU usage is similar (just check the demo with and without this commit), but take into account that before this PR the interval is 100ms which produces a visual effect much worse than with this commit applied. |
But there is no visual effect. This thing runs the FFT at the given interval, detecting speech, so what's the benefit of doing it as fast as possible? |
If you want to detect voice volume is because you want to display a VU meter. The smoother the visual effect is the better it looks. |
I'm not crazy: http://creativejs.com/resources/requestanimationframe/ |
|
I don't. This library provides 2 things: audio levels and speaking / not speaking events. You don't need raf for the latter, AFAIS. |
Right. Now please show me how better is a 100ms interval than |
I didn't say it's better, I said it's not necessary :-) Doing the detection too often would probably lead to flip-flopping too many times, I guess. |
:) nobody said anything about anyone being crazy. I agree, in general for animations rAF is definitely the way to go, and obviously hark can be used to drive animations. My caution is simply that I've always had concern in the back of my mind about how performant hark is or isn't, as well as how much CPU/battery/etc, running constant FFTs on audio streams consumes; and I haven't ever got my head fully around how good/bad things are in that regard. Sure rAF allows the browser to scale back and do less frames when it's busy with other things more easily, but equally, when things aren't otherwise busy we're now doing ~60 sets of calculations per second instead of ~10. So I just want to be sure we have a good understanding of the impact of that, and whether rAF should be the default/optional/otherwise. |
Then the Anyhow, you can do stuff within the given callback such as returning if no enough time has passed. Still |
@latentflip I understand your concerns. I've the same ones :) However, when things go bad and the computer is consuming lot of CPU, a 100ms periodic timer may be much worse (in terms of CPU usage and performance) than a |
What if we do both, use rAF to trigger every frame, but then still keep the |
My last 2 cents :-) raf seems to be all about animations. If you want butter-smooth animations you need to do things many times per second, that's a given. But that's not the case here. If you CPU is overloaded then timers may fall behind, but that doesn't matter. We may miss some speaking events, but that doesn't matter either. Or maybe it does, depends on your application. Before this patch that value was configurable, now it's not, so people who use it would lose the ability to do so. Telling users "yeah whatever, handle it yourself in the callback" just doesn't sound right. |
@latentflip I do agree: let fps = 30; // User-configurable
let interval = 1000 / fps;
let before = Date.now();
function check()
{
timer = requestAnimationFrame(check);
/* emit volume levels here */
let now = Date.now();
if (now - before >= interval)
{
before = now;
/* emit speaking/non-speaking events here */
}
}
check(); |
@saghul said:
Such an interval is of very little use when your computer is consuming lot of CPU. Chrome does magic by reducing timer intervals in this scenario, but other browsers may just queue tasks and more tasks. |
I think I'd rather: a) leave interval as user configurable (rather than fps), and leave it as 100ms e.g.: let interval = 100; // User-configurable
let before = Date.now();
function check()
{
timer = requestAnimationFrame(check);
let now = Date.now();
if (now - before >= interval)
{
before = now;
/* calculate volume levels */
/* emit volume levels here */
/* emit speaking/non-speaking events here */
}
}
check(); |
@latentflip that would also work, of course. It's just that the VU meter feature looks better when IMHO the speaking/non-speaking detection should not rely on a user provided interval value (this library is supposed to know which interval is the perfect one). |
For what it's worth: Hark doesn't do animation, so I'm in the camp of optimizing for performance gathering, but if you're animating something based on events, that's a tangential concern. Often times (as was the case when hark was first written), the animation was used to transmit your volume level to another party (e.g., over a data channel) at which point they would animate it. Again, this separates the concern of gathering the volume level and animating it. Another reason the delay was set to 100ms and not 16 (for 60fp) is that this was bombarding the capture, transmission, and animation in cases where there are multiple participants. I'd suggest that if your concern is butter smooth animations, then you (collectively, across all volume meters to be animated), use rAF outside of hark, with the highest rate you want. Then tune hark to perform as well as it can with regards only to capture of the value, which is its job, and leave the animation up to the animation layer. |
This is exactly the reason for using Anyhow, I insist: arguing that a 100ms interval is better for CPU usage than raf is not true. All the browsers guarantee that raf will invoke the given callback when appropriate (each 16ms if the CPU is fine, and at lower interval if the CPU is busy). That is not true for |
Okay, to summarise, I think there are multiple concerns being conflated here:
My feeling is that the code I proposed in #24 (comment), with some kind of a tweak to deal with the issue in 3.ii. should cover all of these issues/use-cases - allowing @ibc to do what he's trying to do, and the only change for existing users will be that potential CPU issues described in 1. will be improved. |
Agreed. BTW: I would also open the door to separate intervals for both VU-meter and speaking detection features: let timer;
let vuInterval = 100; // default value (configurable)
let speakingInterval = 100; // fixed value
let vuBefore = Date.now();
let speakingBefore = Date.now();
function check()
{
timer = requestAnimationFrame(check);
let now = Date.now();
if (now - vuBefore >= vuInterval)
{
vuBefore = now;
/* emit VU-meter event here */
}
if (now - speakingBefore >= speakingInterval)
{
speakingBefore = now;
/* emit speaking/non-speaking events here */
}
}
check(); |
Hi guys, any update to this? :) |
Maybe using rAF could just be a config option, so backward behavior is maintained. |
@ibc do you mind rebasing from master? |
Hi @jadbox, which is the preferred behavior then? Something as I proposed in #24 (comment)? |
@ibc it seems like the right approach to me, but I'm still just starting to learn the code base. To give context, I'm building a large mobile application that uses Hark to drive high fidelity animations, and it'll be ideal to only process events during a frame. |
Any update on this? Although having no updates in three years is a bad symptomp... :-/ |
I was reading an article on MDN and it said:
This might imply that RAF doesn't get called if the animation is offscreen. Meaning while timeouts and invertals might be throttled, RAF callback might not be called at all. |
That is true. It's browser dependent, but I think it was Chrome or Safari than when browser tab was not active, it throttle |
requestAnimationFrame() is much more efficient than
setTimeout()
orsetInterval()
, and the result is smoother and more accurate (just check the provided demo and the consumed CPU).