Skip to content
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

Set the next frame time at the project level. #32

Open
Ross-Esmond opened this issue Aug 10, 2022 · 1 comment
Open

Set the next frame time at the project level. #32

Ross-Esmond opened this issue Aug 10, 2022 · 1 comment
Labels
a-core Relates to the core package b-enhancement New feature or request

Comments

@Ross-Esmond
Copy link
Contributor

Description
Currently, each thread controller computes the time of the next frame individually given the project time and framerate. I believe it would be advantageous to set the "target time" at the project level to be used by the flow functions. There are two main advantages to this.

The first is performance. If the target time was set at the project, the target time could change dynamically whenever the play speed was adjusted. At the moment, (as far as I recall), switching to a x8 slowdown causes a recomputation that requires eight times as many frames to be computed. For most statements this isn't noticeably slower, but tweens will cause far more computations.

There is another effect on performance, however. If the target time can change without a recomputation, then the performance of seeking can be boosted by using a lower frame rate during a seek and switching to the correct frame rate once the requested frame arrives. In fact, the seek could run at no frame rate. I'll explain the two parts of this.

A seek would set the target time as the time selected by the user. If that time is 10 seconds in, the project would set the current time to 0, as usual when seeking backwards, and set the target time to 10, as if the frame-rate was 1/10th frames per second.

Tweens that take less than a frame run twice, once at value=0, and once at value=1. This cleanly extends to a ten-second frame. A tween that starts at t=1 and ends at t=3 will see that the "next frame" is beyond it, and will run twice. A tween that starts at t=9 and ends at t=11 will see that the next frame is inside it, and will also run twice, at 0 and 0.5. All tweens would only run twice during a seek, rather than running all their frames as usual.

The time to seek would then be a multiple of the number of tweens and other statements rather than the number of frames. For heavily animated videos this would be a lot faster.

The second advantage of this change is better synchronization with the audio. Currently, if the thread runner gets out of sync with the audio, the audio time is adjusted, but that just turns stuttery video into stuttery audio. With this change the project could adjust the frame rate in real-time to correct mismatch, such that the audio is perfect and the video actually corrects for errors in real-time. My guess is that the browser works very hard to keep audio from stuttering in most cases, so the video speed might become more stable, and if the adjustments are always small, the animations should only become smoother.

Disadvantage
If any imperative code uses a value that is adjusted by a tween then that code will break while seeking, as the tweens will not run as usual. Granted, this is already bad practice, as it causes the frame rate to effect the outcome, but it could be a gotcha for some people. There could be an option to turn this adjustment off, but users won't understand it well enough to know when to use it.

A note on performance (for Jacob)
You've mentioned in the past that the seek performance is acceptable in your use case, and that is a valid reason not to make this change, however, it is possible that some users will attempt to use Motion-Canvas as more of an animation library rather than more of an instructional library. There is massive overlap (the Astortion devlog uses animation, of course), but if many users find themselves using 20x as many tweens in a scene, it's possible this change could become necessary. This ticket can serve as a ripcord to pull if the performance needs a boost.

@Ross-Esmond Ross-Esmond added b-enhancement New feature or request a-core Relates to the core package labels Aug 10, 2022
@aarthificial
Copy link
Contributor

switching to a x8 slowdown causes a recomputation that requires eight times as many frames to be computed.

There's no x8 slowdown, max is x4. But the recalculation that happens seems to be a leftover from before https://github.com/motion-canvas/core/issues/57
The goal of recalculation is to compute the new starting and ending frames which are currently independent of the speed. So I believe recalculation, in this case, could simply be omitted.

Currently, if the thread runner gets out of sync with the audio, the audio time is adjusted

That's not entirely true. If present, the audio serves as a master clock.
If the animation is ahead, it will wait until the next requestAnimationFrame:

// Do nothing if paused or is ahead of the audio.
else if (
state.paused ||
(state.speed === 1 &&
this.audio.isReady() &&
this.audio.isInRange(this.project.time) &&
this.audio.getTime() < this.project.time)
) {
this.request();
return;
}

If the animation is behind, it will try to catch on:

// Seek to synchronize animation with audio.
else if (
this.audio.isReady() &&
state.speed === 1 &&
this.audio.isInRange(this.project.time) &&
this.project.time < this.audio.getTime() - MAX_AUDIO_DESYNC
) {
const seekFrame = this.project.secondsToFrames(this.audio.getTime());
state.finished = await this.project.seek(seekFrame);
}

So the framerate is variable, but it's controlled by the requestAnimationFrame loop and not the project itself.
The audio is only adjusted if the speed is different than 1 for obvious reasons.

When it comes to seeking/recalculation optimization, I played around with this idea when working on https://github.com/motion-canvas/core/issues/57 since the implementation was already there. If you set the speed to a high number, say 10, you'll see the performance gains you're talking about. But the disadvantage you've mentioned was the reason I dropped it.

Personally, I wouldn't say that accessing a value during a tween is bad practice.
It would only be a problem if you were to use this value to modify the timing. Pass it to waitFor or something like that.
Otherwise, the worst it could do is to produce some visual artifacts/discrepancies which I think would be an acceptable trade-off.

Anyway, as you've mentioned in the note, let's leave this as a ripcord for now.

@aarthificial aarthificial removed their assignment Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-core Relates to the core package b-enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants