search

youtube / js_mse_eme

js_mse_eme is an externally-published tool that is aimed to test the validity of a browser's HTML5 Media Source Extension and Encrypted Media Extension implementations
Apache License 2.0
91 stars 58 forks source link

Only measure time differences when the video has actually started playing #37

Closed eocanha closed 5 years ago

eocanha commented 5 years ago

On some browsers (eg: WebKit), a timeUpdate event is emitted as soon as video.play() is called, but that doesn't mean that the underlying media framework has actually started to play right at that point (a startup delay is expected on low end devices).

Therefore, the baseTimeDiff should be measured when video.currentTime has actually progressed (becomes greater than zero).

I don't think that modifying the web engine to avoiding that initial timeUpdate event is the right approach in this case, because that change breaks our layout tests. In this specific case, modifying the MSE test to start measuring baseTimeDiff in a better moment makes more sense. After all, baseTimeDiff is retroactive, in the sense that it subtracts currentTime so it doesn't matter if it's measured right at the begining of the playback or some moments later, when time goes forward for the first time.

eocanha commented 5 years ago

Any feedback about this pull request?

jiaqzhao commented 5 years ago

Hi, sorry for the late reply.

According to the spec, a timeupdate event is fired if "The current playback position changed as part of normal playback or in an especially interesting way, for example discontinuously."

FWIW, we could add a listener to a "canplay" event, and play the video when the "canplay" event is fired.

eocanha commented 5 years ago

Canplay wouldn't solve the problem in our case, as it's emitted before the first timeUpdate (with currentTime=0) event:

http://10.42.0.1/mstest/harness/main.js:119:16: CONSOLE LOG TestExecutor:  Test 26:SFRPausedAccuracy STARTED with timeout 30000 
http://10.42.0.1/mstest/media/conformanceTest.js:679:18: CONSOLE LOG ### currentTime: 0
http://10.42.0.1/mstest/media/conformanceTest.js:635:20: CONSOLE LOG ### onCanPlay: currentTime: 0
http://10.42.0.1/mstest/media/conformanceTest.js:639:20: CONSOLE LOG ### onTimeUpdate: currentTime: 0
http://10.42.0.1/mstest/media/conformanceTest.js:639:20: CONSOLE LOG ### onTimeUpdate: currentTime: 0.091950113
http://10.42.0.1/mstest/media/conformanceTest.js:639:20: CONSOLE LOG ### onTimeUpdate: currentTime: 0.293038548
...

It's true that Chrome and Firefox only emit timeUpdate events when currentTime actually changes from its previous value and that WebKit is the different one here. If the MSE tests have no room for our different implementation, we'll try to adapt our browser then.

jiaqzhao commented 5 years ago

Please do. Since the test doesn't conflict with the spec, we would leave it as it is.