-
Notifications
You must be signed in to change notification settings - Fork 19
Validate resolution in UI #209
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
Conversation
yondonfu
left a comment
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 agree that with the comment that the pipeline should not automatically adjust the output resolution.
I think the UX can be a bit more intuitive here by immediately auto-adjusting the values instead of waiting until the stream starts:
- Clicking +/- can move the value to the next valid value for height/width.
- If the user directly inputs a value that is invalid "snap" it to the closest valid value
This way when the user starts the stream the values they see are the values that are used.
|
Can we also add validation in the pipeline to error out early if an invalid height/width are used? This would be useful to users that call the pipeline directly. |
We can change it, but I find it less intuitive TBH, because:
resolution2.mp4Compare it to the previous demo I shared in which you're hinted what's wrong and what will happen. resolution.mp4 |
Good idea, added. |
406256c to
7b02c4e
Compare
Signed-off-by: Rafal Leszko <rafal@livepeer.org>
3236680 to
1e8602c
Compare
yondonfu
left a comment
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
Note: Currently, the factor is hardcoded to
16in the frontend, but after the "defaults' changes from @ryanontheinside we should take these values from the backend.fix #194