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

fix: multiple seeks called on same time #793

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

abdelaziz-mahdy
Copy link
Member

This fix was originally part of PR #791 but was separated due to failing tests and the need for additional validation to ensure it works as expected. This specific change addresses an issue where holding the arrow keys during video seeking caused repeated seek operations to the same position—up to 27 times in one case. This behavior resulted in the video player being stuck in a loading state, as each completed seek triggered another to the same position. This refactor aims to optimize the seek functionality and prevent redundant operations to enhance the viewing experience.

@default-anton
Copy link
Contributor

seek relative shouldn't have this problem and can be useful to seek -N seconds/+N seconds. How do you feel about extending the seek method interface to allow relative seeking? For example, seek(duration, relative: true). A positive duration seeks forward, while a negative duration seeks backward.

@abdelaziz-mahdy
Copy link
Member Author

The other pr was closed which was the relative seek, since it's not accurate, this pr is only to fix the problem of seeking to same time calls are stuck waiting for each other

As Alex mentioned if you want to use relative in your app, use the set property

@default-anton
Copy link
Contributor

Sorry, forgot to add exact, which is accurate seek relative+exact

I see, +- N seconds is so common in every player. Maybe it’s worth extending the interface? Or adding skipForward, skipBackward methods? Given that it solves the problem of pressing the +N seconds multiple times

@alexmercerind wdyt?

@abdelaziz-mahdy
Copy link
Member Author

lets think feature wise what we will gain?

instead of getting the position and adding the time, mpv will do it internally

and we will have to make the same feature on web and any other platform we add.

from my point of view if its not faster then there is no pros to it.

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.

None yet

2 participants