Skip to content

Fixing looping logic#58

Merged
iambalaam merged 2 commits into
mainfrom
fix-looping
May 20, 2026
Merged

Fixing looping logic#58
iambalaam merged 2 commits into
mainfrom
fix-looping

Conversation

@iambalaam

@iambalaam iambalaam commented May 20, 2026

Copy link
Copy Markdown
Contributor

Before this PR, looping videos without Sync enabled performs the seek ahead behaviour and waits at 1s each time.
Now the looping behaviour feels identical to the default HTML video loop

In this janky little video, I've got a normal looping video element in the top left

<video src="test-video.mp4" loop></video>

Underneath it covering the whole screen is a looping video from COGS that doesn't have sync on.
(they're not meant to be in time at all, but you can see when it loops the arrows pause for the same amount of time)
You can see that the arrows pause

20260519_111349.mp4

@iambalaam iambalaam requested a review from chetbox May 20, 2026 09:09
if (nextTemporalKeyframe?.[1]?.set?.t === 0) {
const timeRemaining = (mediaElement.duration - properties.t) / properties.rate;
const timeUntilKeyframe = nextTemporalKeyframe[0] - properties.t;
const timeRemaining = (mediaElement.duration * 1000 - properties.t) / properties.rate;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nooo 🤦🏼

@chetbox chetbox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a test for setting loop on a video element?

@chetbox chetbox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

@iambalaam iambalaam merged commit 81e2f8a into main May 20, 2026
5 checks passed
@iambalaam iambalaam deleted the fix-looping branch May 20, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants