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

Blinking time indicator on second recording #46

Closed
alsar opened this issue Jan 20, 2016 · 20 comments
Closed

Blinking time indicator on second recording #46

alsar opened this issue Jan 20, 2016 · 20 comments
Labels

Comments

@alsar
Copy link
Collaborator

alsar commented Jan 20, 2016

If you record a video for the second time, the time indicator starts to blink.
Also the recording time is wrong. If the length of the recorded video is 10s the recorder shows 8s. This also happens on the second run.

@thijstriemstra
Copy link
Member

@alsar is this something you noticed in v1.1.0 or was it already present?

@alsar
Copy link
Collaborator Author

alsar commented Jan 21, 2016

It was already present in 1.0.3

@thijstriemstra
Copy link
Member

@alsar what browser are you using? It is supposed to blink (but perhaps it only does it the second time in your browser).

@thijstriemstra
Copy link
Member

Oh you mean the time indicator, not the record indicator of course, my bad. The time indicator starts to blink, can you make a gif or something of this, I'm not seeing this or misunderstanding you. This would also help with figuring out the recording time issue.

@alsar
Copy link
Collaborator Author

alsar commented Jan 25, 2016

@thijstriemstra Here is a video: https://youtu.be/g_QsgwQRqO4

As already mentioned, this happens only on the second run (and every next).

@vojka
Copy link

vojka commented Feb 1, 2016

I'm also having the same problem.

@thijstriemstra
Copy link
Member

It seems this only happens on slow machines, I can't reproduce it on this early 2015 Macbook Pro.

@alsar
Copy link
Collaborator Author

alsar commented Feb 4, 2016

Tried on multiple i5 machines with 8-16gb ram. Same problem.

@thijstriemstra
Copy link
Member

There is someting in video.js that resets the current time every x milliseconds when no valid source was found. This is what makes the time indicator 'blink'. Please look through the video.js source (probably for timeupdate event) and figure out how to disable this, I won't have time the next few days.

@thijstriemstra
Copy link
Member

For example, on https://github.com/videojs/video.js/blob/master/src/js/player.js#L2792:

/**
 * Fired when the current playback position has changed *
 * During playback this is fired every 15-250 milliseconds, depending on the
 * playback technology in use.
 *
 * @event timeupdate
 */

This is the 15-250 ms time indicator reset issue I think.

@thijstriemstra
Copy link
Member

And then in tech.js there's trackCurrentTime. Can you check if that method is called and resets the time to 0?

@thijstriemstra
Copy link
Member

When the videojs-record plugin is first intialized, the timeupdate event is disabled. Then during playback it's enabled again. If you then start recording again, the timeupdate handler is disabled again. I suspect this is where something's wrong..

@thijstriemstra
Copy link
Member

@alsar can you upgrade video.js to 5.8.0 or newer and check if this issue is fixed (related commit: videojs/video.js@0490728)?

@alsar
Copy link
Collaborator Author

alsar commented Mar 16, 2016

@thijstriemstra it's still the same with 5.8.3

@zang
Copy link
Contributor

zang commented Mar 23, 2016

Got the same issue. Time still updates, but the leading 0 seems to blink. (e.g. 0:12 to 00:12, then to 0:13)

@zang
Copy link
Contributor

zang commented Mar 24, 2016

it works fine if I change line 1547 to the following:
m = ((h && m < 10) ? '0' + m : m) + ':';

I am not unsure what's the gm for, so didn't create PR.

@thijstriemstra
Copy link
Member

@zang feel free to make a PR so I can check it out. videojs-wavesurfer also uses the same code, I'm guessing it's the same problem there? https://collab-project.github.io/videojs-wavesurfer

@zang
Copy link
Contributor

zang commented Mar 24, 2016

#65

@thijstriemstra
Copy link
Member

This is the original code btw: https://github.com/videojs/video.js/blob/master/src/js/utils/format-time.js

videojs-record adds support for M:SS:MMM.

@thijstriemstra
Copy link
Member

Thanks for the PR @zang, merged it and will release a new version now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants