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

Fixes calling start() on a remote track multiple times #393

Merged
merged 6 commits into from
Aug 22, 2022

Conversation

wcarle
Copy link
Contributor

@wcarle wcarle commented Aug 11, 2022

Prevent multiple redundant monitors from being started if start is called multiple times on a RemoteTrack

Fixes issue: #392

@changeset-bot
Copy link

changeset-bot bot commented Aug 11, 2022

🦋 Changeset detected

Latest commit: 515fb77

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
livekit-client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Aug 11, 2022

CLA assistant check
All committers have signed the CLA.

@lukasIO
Copy link
Contributor

lukasIO commented Aug 12, 2022

hi @wcarle,
thanks for the PR!
Looking at the code (not particularly your changes), I'm thinking it might make sense to base the monitor on an interval, rather than timeouts.
Then a reference to the interval could be used to track if monitoring has been started already and it wouldn't need the extra variable to track if monitoring has been started.

@wcarle
Copy link
Contributor Author

wcarle commented Aug 17, 2022

@lukasIO Yeah that's a good call, it's much cleaner that way. Just pushed that change

@lukasIO
Copy link
Contributor

lukasIO commented Aug 18, 2022

great, thank you!
I think there's only one thing left to do, which is clearing the interval at the top of the stop function on the RemoteTrack

@wcarle
Copy link
Contributor Author

wcarle commented Aug 19, 2022

@lukasIO Done!

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.

None yet

3 participants