-
Notifications
You must be signed in to change notification settings - Fork 45
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
Skip initial silence #1262
Skip initial silence #1262
Conversation
410f772
to
3251366
Compare
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.
Works smoothly!
Before merging it to dev, I would like to make sure: we do rebuild the database container using "/docs/static/tum-live-starter.sql" for each deployment, right? |
I'm afraid I don't quite follow. But we can always get someone who is more familiar with the system in here if need be. |
I thought it would make sense to commit these changes as well for future testing in this area, but we can also exclude this file if it's cluttering the db. |
Yes, I think changes to the sql file should be pushed anyway, but they need to be handled more carefully when deployed. I'd like to hear from someone more familiar with the project on this issue. |
@alexanderstephan or @joschahenningsen what are your thoughts on the matter? |
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.
tum-live-starter.sql
Is just for local testing and can be modified without worries :)
Co-authored-by: Joscha Henningsen <[email protected]>
Co-authored-by: Joscha Henningsen <[email protected]>
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.
🍻
* added api endpoints for user setting * fixed API endpoints * watchPageData sets progress to end of first silence when user setting is set * GetAutoSkipEnabled wraps result in json * added front-end component * added error handling when getting user setting * make sure stream is not selfstream before skipping silence * imported logger * gofumpt everything * updated db starter for better testing * use correct logger Co-authored-by: Joscha Henningsen <[email protected]> * use correct logger Co-authored-by: Joscha Henningsen <[email protected]> --------- Co-authored-by: Joscha Henningsen <[email protected]>
Motivation and Context
Closes #1188
Instead of opening a VOD, waiting for it to load, clicking skip, and waiting for it to load again there should be a user setting which allows the player to immediately skip past the first period of silence.
Description
A new user setting to allow automatic skipping was added, this can be modified in the users settings page.
When this setting is enabled, the watch page sets the progress of the stream VOD to the maximum of the watch progress in the database and the end of the first period of silence.
This only happens if the stream is not a self-stream (lecture_hall_id != 0). This is because the progress is converted into a timestamp using the JS videoplayer duration, not the duration calculated by subtracting the stream end from stream beginning. I think it's safe to assume these two duration values will be the same when it is not a self-stream, as the recording just runs on in those cases.
Steps for Testing
Prerequisites:
Screenshots