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.

Sign up for GitHub

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

Jump to bottom

Linking to a particular time doesn't work #926

Open
KyleMaas opened this issue Dec 2, 2023 · 9 comments · May be fixed by #928
Open

Linking to a particular time doesn't work #926

KyleMaas opened this issue Dec 2, 2023 · 9 comments · May be fixed by #928
Assignees
@mgogoulos

Comments

@KyleMaas
Copy link
Contributor

KyleMaas commented Dec 2, 2023

Describe the issue
See this discussion:

#319

I've received user complaints and have tried this myself and confirm that it does not work. The UI elements for generating a timed link in the Share button work, but when you actually go to the link it doesn't work.

To Reproduce
Steps to reproduce the issue:

  1. Go to a video
  2. Skip partway through the video
  3. Click Share
  4. Check the "Start at XX:XX" checkbox and copy the link
  5. Paste the link into another tab or browser
  6. Video plays from the start instead of from the time specified

Expected behavior
This:

https://github.com/mediacms-io/mediacms/blob/main/docs/user_docs.md#using-timestamps-for-sharing

Environment (please complete the following information):

  • OS: Has been tested on both Gentoo Linux and Windows from the user side
  • Installation method: single server install
  • Browser, if applicable: Chromium and Firefox, I believe
@KyleMaas
Copy link
Contributor Author

KyleMaas commented Dec 2, 2023

I'm not even sure how this is possible, but setting breakpoints in the code, this entire function seems to not be running:

mediacms/frontend/src/static/js/components/video-player/VideoPlayer.jsx

Line 95 in c5047d8

function initPlayer() {

@KyleMaas
Copy link
Contributor Author

KyleMaas commented Dec 2, 2023

Sorry, wrong function. That one doesn't seem to be running, either, but this one is the more problematic one that's not running:

mediacms/frontend/src/static/js/components/video-player/VideoPlayer.jsx

Line 187 in c5047d8

useEffect(() => {

@KyleMaas
Copy link
Contributor Author

KyleMaas commented Dec 2, 2023

Nevermind. The compiled code is duplicated in two different places, and I was looking at the wrong one.

@KyleMaas KyleMaas mentioned this issue Dec 2, 2023
Open
@KyleMaas
Copy link
Contributor Author

KyleMaas commented Dec 2, 2023

So it is running, and it's reaching this within Video.js:

https://github.com/videojs/video.js/blob/3c110f0fb10961791c8b55025e8f9f4a15607306/src/js/player.js#L2510

But the problem seems to be that this happens before Video.js is finished initializing, so then this fires within Video.js:

https://github.com/videojs/video.js/blob/3c110f0fb10961791c8b55025e8f9f4a15607306/src/js/player.js#L2524

Which, because of the first call and the fact that the Video.js frontend thought it was ready and it cleared the cache init time, the second call sets the seek position to the start.

@KyleMaas
Copy link
Contributor Author

KyleMaas commented Dec 2, 2023

videojs/video.js#3495

@KyleMaas
Copy link
Contributor Author

KyleMaas commented Dec 2, 2023

videojs/video.js#1324

@KyleMaas KyleMaas linked a pull request Dec 2, 2023 that will close this issue
Open
@KyleMaas
Copy link
Contributor Author

KyleMaas commented Jan 3, 2024 via email

@segurancadecomputadores
Copy link

Even with this modification, this feature not work... i am using firefox as client. The server is Ubuntu 22, with this version:

https://github.com/KyleMaas/mediacms.git

@KyleMaas
Copy link
Contributor Author

@segurancadecomputadores That version doesn't contain the fix. You would need to use the code from this branch:

https://github.com/KyleMaas/mediacms/tree/fix-timed-links

But it still will not work until you rebuild the frontend. To make it easy to merge back into the project without conflicts, that branch only contains the changes to the source code for the frontend but does not contain the built files. To test it out, you would need to go into the frontend folder and npm build dist IIRC and then copy the built files from frontend/dist into static. It's not designed for direct use but for merging into the main project so that everyone can use it, so trying it out is a little more involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mgogoulos mgogoulos

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add a workaround for the start time race condition in Video.js KyleMaas/mediacms
3 participants
@mgogoulos @KyleMaas @segurancadecomputadores