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

seekTo end frame jumps to 0:01 #591

Closed
rijk opened this issue Mar 11, 2019 · 6 comments
Closed

seekTo end frame jumps to 0:01 #591

rijk opened this issue Mar 11, 2019 · 6 comments

Comments

@rijk
Copy link
Contributor

rijk commented Mar 11, 2019

Hi, I'm having some issues with the changes made to seekTo as a result of #75.

  1. Seeking to the end of the video (seekTo(1)) makes the player jump to 0:01. To reproduce: open https://cookpete.com/react-player/, load mp4 and seek to end. Expected: video shows last frame. Actual: video jumps to 0:01.
  2. I do have to be able to skip to 0.5 seconds ("I did consider this when implementing but figured the chance of someone ever wanting to seek to 0.x seconds was pretty much zero."), because I am developing a tool where users can give feedback on exact frames of a video.

So, while I love the clean and concise API, I have to say I agree with @waynebloss here:

Overloading the seekTo method to do 2 separate functions by looking for fractions between '0' and '1' seems to go against best practices for making a clear and concise programming interface.

I understand we now face backwards compatibility issues as well. Maybe we can add a second parameter to the function: type: AUTODETECT (default) / SECONDS / FRACTION?

@cookpete
Copy link
Owner

Overloading the seekTo method to do 2 separate functions by looking for fractions between '0' and '1' seems to go against best practices for making a clear and concise programming interface.

Using fraction to seek is simply legacy behaviour from back when duration was not returned consistently between different players. The method wasn't deliberately overloaded, but supporting both seemed better than a breaking change or adding another almost identical method for (what seemed like) a very rare edge case.

Maybe we can add a second parameter to the function: type: AUTODETECT (default) / SECONDS / FRACTION?

Perhaps the seconds parameter should be just a forceSeconds boolean? I don't see why you would ever need to specify FRACTION.

@rijk
Copy link
Contributor Author

rijk commented Mar 11, 2019

Thanks for the quick reply and background info! As the seek bar control returns fractions, and the default for onProgress is fraction as well, I assumed fractions was the defacto standard, and so I standardised my app on fractions (for context: when a comment is posted, I link it to the current frame of the video).

I don't see why you would ever need to specify FRACTION.

Well, in this case I would need it to prevent seekTo(1) jumping to 0:01 (you can reproduce it in the demo player and it really feels like a bug).


To work around it, I'm now trying something like

if ( played === 1 ) played *= this.player.getDuration()

but am running into a different issue where Safari seems to jump too far, and I'm not seeing anything anymore (although it works in Chrome).

@cookpete
Copy link
Owner

Well, in this case I would need it to prevent seekTo(1) jumping to 0:01 (you can reproduce it in the demo player and it really feels like a bug).

Yeah, fair enough. I guess a hacky workaround for now could be seekTo(0.999999)?

@rijk
Copy link
Contributor Author

rijk commented Mar 11, 2019

Yep, that would work as a workaround, except I am now running into the Safari issue (happens with seekTo(0.999999) as well). The video is an mp4 of 0:42 seconds.

I've tested and found that in this case 0.999 works, and 0.9999 doesn't (player is blank). I can settle on 0.999 but it feels tricky (I suspect it will be related to the video length as well). Should I create a separate issue for this?

@cookpete
Copy link
Owner

A new issue makes sense, but this seems like a problem with Safari rather than ReactPlayer.

I can't reproduce the issue myself. For me it just shows the last frame of the video: https://jsfiddle.net/8s1xjrgf/

@rijk
Copy link
Contributor Author

rijk commented Mar 24, 2019

Thanks! ✌️

albanqoku added a commit to albanqoku/react-player that referenced this issue Feb 24, 2021
Webmaster1116 added a commit to Webmaster1116/video-player that referenced this issue May 20, 2021
webmiraclepro added a commit to webmiraclepro/video-player that referenced this issue Sep 9, 2022
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

No branches or pull requests

2 participants