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

Increase default adaptiveStream pixelDensity on high-density screens #735

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

davidzhao
Copy link
Member

Mobile devices typically use high density screens (2.5-3+). On these platforms, using default CSS dimensions tends to produce less than ideal streaming quality when using adaptiveStream defaults.

Mobile devices typically use high density screens (2.5-3+). On these
platforms, using default CSS dimensions tends to produce less than ideal
streaming quality when using adaptiveStream defaults.
@changeset-bot
Copy link

changeset-bot bot commented Jun 10, 2023

🦋 Changeset detected

Latest commit: 44a5024

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 Minor

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

@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2023

size-limit report 📦

Path Size
dist/livekit-client.esm.mjs 83.88 KB (+0.03% 🔺)
dist/livekit-client.umd.js 87 KB (+0.02% 🔺)

@davidliu
Copy link
Contributor

Can we switch to screen pixel density by default? Or does that not work for mobile browser cases?

@davidzhao
Copy link
Member Author

Can we switch to screen pixel density by default? Or does that not work for mobile browser cases?

I'm trying to avoid a change in behavior on desktop systems. If we defaulted to screen, it will increase the bandwidth consumption on most displays today. On mobile, it'll mean fetching 720p by default for a video tile that takes about 200 css pixels.

Maybe this is a change we make for 2.0? as more efficient codecs are used.

Copy link
Contributor

@davidliu davidliu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@lukasIO lukasIO left a comment

Choose a reason for hiding this comment

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

that makes sense!
we should probably do this with a minor changeset, as it is changing current default behaviour on mobile

@davidzhao davidzhao merged commit 691e8b2 into main Jun 13, 2023
3 checks passed
@davidzhao davidzhao deleted the dz-increase-default-density branch June 13, 2023 02:00
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