Skip to content

fixes seek bar not updating and video loading for infinite time when keyboard controls are used#3267

Merged
TheModMaker merged 8 commits intoshaka-project:masterfrom
surajkumar-sk:seekbar
Mar 24, 2021
Merged

fixes seek bar not updating and video loading for infinite time when keyboard controls are used#3267
TheModMaker merged 8 commits intoshaka-project:masterfrom
surajkumar-sk:seekbar

Conversation

@surajkumar-sk
Copy link
Contributor

@surajkumar-sk surajkumar-sk commented Mar 22, 2021

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

changeto() function in range_element.js file this.isChanging_ was set to true but never set to false which caused updateTimeAndseekRange() in controls.js to fail. this fixes the bug

the second bug was caused because the video.currenttime was set to this.bar.max. I don't know why but the video doesn't get loaded at its max time and after starting to buffer at its max time it never stops buffering. I don't know why it doesn't buffer at its max time but instead of setting video.currenttime to the max value, we could set it to (max-0.125) this wouldn't make any difference because the time difference is very small but it does prevent that bug and video being stuck at buffer state.

Fixes # (issue)
#3234

Screenshots (optional)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • New and existing unit tests pass locally with my changes

@surajkumar-sk surajkumar-sk changed the title fixes seek bar not updating when keyboard controls are used fixes seek bar not updating and video loading for infinite time when keyboard controls are used Mar 22, 2021
@TheModMaker
Copy link
Contributor

Please revert the changes to the end time and file a new bug with reproduction steps using the latest nightly site. The other changes LGTM but changing the end time doesn't seem like the correct fix.

@surajkumar-sk
Copy link
Contributor Author

@TheModMaker removed the changes for max value, the bug of video not loading happens because the video element gets a value beyond or equal to its max time when arrow keys are used that stalls the loading of video and audio. I cannot see a way around without reducing the max time. I know you are busy with lot of work, but any initial clue on how you would approach it would help a lot. It doesn't feel good leaving the bug unsolved.

@surajkumar-sk
Copy link
Contributor Author

surajkumar-sk commented Mar 24, 2021

@TheModMaker I feel there isn't a better solution than reducing the max value. reducing by 0.125 looks a bit glitchy but if the value is reduced by 0.025 it will be completely unnoticeable and the bug will be fixed. I say this is better because,

. the video doesn't load and gets stuck at loading because the video's current time gets set to max, which somehow doesn't exist and the video keeps loading.
. the video's current time is set in seekTimer_ which runs when the onchange() function is called, which is called in changeto() after setting the current time value to this.bar.value. we cannot set this.bar.value to max because that is causing the bug.
so, a proper fix would be somehow ending the video when the seek bar gets the value greater than or equal to max.
. in controls.js and event listener is added which listen to the video ended and runs a function to pause the video. An ended event is only fired by the browser when the video's current time reaches its max value. so the video should be assigned the max value to trigger ended event.
.This kind of creates a paradox, max cannot be set without causing the bug so an ended event should be triggered without assigning the max value but the ended event cannot be triggered without assigning the max value.

I don't think there is anything we could do to prevent the bug without reducing the max value or complicating the code.

I browsed Youtube to get a clue on how they handled this error, I realized even they did something similar to reducing max value. The below video show youtube handling this bug.

screen-capture.mp4

Reducing by 0.125 looks a bit glitchy but if the value is reduced by 0.025 it will be completely unnoticeable and the bug will be fixed.
I seriously need to work on giving a small explanation. 😳

@shaka-bot
Copy link
Collaborator

All tests passed!

@TheModMaker
Copy link
Contributor

Thanks for the contribution. We should have things to shift the time backwards if you seek near the end. I think it is better to handle it in the library instead of the UI.

@TheModMaker TheModMaker merged commit e933c36 into shaka-project:master Mar 24, 2021
@surajkumar-sk
Copy link
Contributor Author

@TheModMaker ok , I'll create a new issue showing the bug.

vasujain314 added a commit to vasujain314/shaka-player that referenced this pull request Mar 26, 2021
fix(ui): Fix holding keyboard controls (shaka-project#3267)
joeyparrish pushed a commit that referenced this pull request Apr 25, 2021
Backported to v2.5.x

Change-Id: Ib195e95e7703f4f9a5c5ef1d0ff22be46b4d27b3
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

status: archived Archived and locked; will not be updated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants