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

add support for aspectRatio constraint in videoResolution #64

Merged
merged 1 commit into from
Oct 28, 2021

Conversation

lukasIO
Copy link
Contributor

@lukasIO lukasIO commented Oct 27, 2021

No description provided.

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

Is this needed when both width and height are required? What is the use case?

@lukasIO
Copy link
Contributor Author

lukasIO commented Oct 27, 2021

setting aspectRatio explicitly can have an influence on the order/importance of applied constraints.
An example would be

const constraints = {
  width: { max: 1280 },
  height: { max: 720 },
  aspectRatio: { exact: 1.5 }
} 

in order to get any video smaller than 1280x720 that has a 1.5 aspect ratio.

Side note: apparently constraints are applied in order, if all of them are set as a plain number/value instead of specifing min/max/exact.

The prioritization of the properties is simple: if two properties' requested values are mutually exclusive, then the one listed first in the constraint set will be used.
https://developer.mozilla.org/en-US/docs/Web/API/Media_Streams_API/Constraints#requesting_a_specific_value_for_a_setting

So setting

const constraints = {
  width: 1280,
  height:  720,
  aspectRatio: 1.5
} 

would potentially have a different result than

const constraints = {
  aspectRatio: 1.5,
  width: 1280,
  height:  720
} 

Copy link
Member

@davidzhao davidzhao left a comment

Choose a reason for hiding this comment

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

ah interesting.. that makes sense.. lg.

@davidzhao davidzhao merged commit a2db4d3 into livekit:main Oct 28, 2021
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