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

some type fixes + deduplicate setTargetBitrate #280

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mildsunrise
Copy link
Contributor

this PR first makes setTargetBitrateAsync consistent with setTargetBitrate,
and then factors out most of the logic (common to both) to a new private function called __setTargetBitrate.

to do this, we move the call to getActiveLayers (or getActiveLayersAsync) up in the function, which could have small side effects. please see the commit message for more info

also, make setTargetBitrate and setTargetBitrateAsync
consistent before deduplication
make both functions consistent, applying the changes
that 2951d62 did to
setTargetBitrate also to setTargetBitrateAsync
@murillo128
Copy link
Member

how about removing the non async version completely instead?

@mildsunrise
Copy link
Contributor Author

oh, but we're still using it in StreamView, aren't we? and I thought we wanted to avoid blocking the event loop so frequently

@murillo128
Copy link
Member

I mean to keep only the async version

@mildsunrise
Copy link
Contributor Author

ah sorry, misread. I'm perfectly okay with it, I'll replace the last commit

@mildsunrise
Copy link
Contributor Author

it is done

@mildsunrise
Copy link
Contributor Author

mildsunrise commented Nov 5, 2024

tests are failing after swapping setTargetBitrate for await setTargetBitrateAsync, and I'm a little puzzled why...

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