-
Notifications
You must be signed in to change notification settings - Fork 156
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
feat: add scaleBehavior for subscriber and publisher view #474
base: main
Are you sure you want to change the base?
feat: add scaleBehavior for subscriber and publisher view #474
Conversation
const sanitizeProperties = (properties) => { | ||
if (typeof properties !== 'object') { | ||
return { | ||
scaleBehavior: 'fill', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having this as the default breaks it. I couldn't get it to work unless I changed this value here. I assume this is because the subscriber loads in a bit after the view is rendered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On which platform / or how does it break for you? 😯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both. The subscriber scaleBehavior is always fill
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM |
@abdulajet what do you think should be done with this PR? |
Contributing checklist
Solves issue(s)
Closes #327.
This PR introduces a new property
scaleBehavior: "fill" | "fit"
for OTSubscriber and OTPublisher which translates to the OTVideoViewScaleBehavior on iOS, respectively BaseVideoRenderer.STYLE_VIDEO_* on Android.By default, the value is "fill" which will scale the video to fill the entire view, with cropping as needed. This was the only behavior available so far, thus, no code changes in present apps are required. The new value "fit" will shrink the video (pillarboxing), so that the entire video is contained in the view.