Fix disabling of catchup (leave playback rate alone)

difosfor commented 5 years ago

Environment

[x] The MPD passes the DASH-IF Conformance Tool on https://conformance.dashif.org/
[x] The stream has correct Access-Control-Allow-Origin headers (CORS)
[x] There are no network errors such as 404s in the browser console when trying to play the stream
[x] The issue observed is not mentioned on https://github.com/Dash-Industry-Forum/dash.js/wiki/FAQ
[x] The issue occurs in the latest reference client on http://reference.dashif.org/dash.js/ and not just on my page
Link to playable MPD file: Any low latency stream, e.g: https://vm2.dashif.org/livesim/testpic_2s/Manifest.mpd
Dash.js version: 2.9.2
Browser name/version: Any browser
OS name/version: Any OS

Steps to reproduce

Create MediaPlayer: const player = MediaPlayer().create()
Enable low latency: player.setLowLatencyEnabled(true)
Disable catchup: player.setCatchUpPlaybackRate(0)
Initialize, e.g: player.initialize(media, sourceUrl, true)
Change playback rate, e.g: player.setPlaybackRate(1.1)

Observed behaviour

The playback rate is changed back to 1 even though catchup was disabled.

Console output

player.setPlaybackRate(1.1)
undefined
Debug.js:233 [194373][PlaybackController] Native video element event: ratechange:  1.1 
Debug.js:233 [194447][PlaybackController] Native video element event: ratechange:  1

Fix

I've forked dash.js and fixed it here: https://github.com/exmg/dash.js/commit/3397076c10b0669ec5bfad48c791a7a82f260723

If you like I can create a pull request, but I'll have to check if I can sign the Feedback Agreement etc. Please feel free to just copy my fix if that's easier.

epiclabsDASH commented 5 years ago

Hi @difosfor. Thanks for the fix! Unless there is any inconvenient, I always prefer creator of the fix to send the PR (you get the credits of the work you really did). Sending the Feedback Agreement shouldn't take too much time so, if you have that time I invite you to follow that process.

Anyway, if there is any inconvenient or you think following the contribution process will delay the release of this fix, let me know and I will send the PR myself (no problem at all, I can do it quickly).

difosfor commented 5 years ago

Alright, I've sent the agreement over: https://groups.google.com/forum/#!topic/dashjs/2uKAFnV38b4

And created a pull request: https://github.com/Dash-Industry-Forum/dash.js/pull/2899

Looking forward to working together!

epiclabsDASH commented 5 years ago

Many thanks!

difosfor commented 5 years ago

It looks like this issue is back in v3.0.0:

I think this line: https://github.com/Dash-Industry-Forum/dash.js/blob/1cc9d8074bab824f3b3846ad2a543eb42908057f/src/streaming/controllers/PlaybackController.js#L597

Should be: settings.get().streaming.liveCatchUpPlaybackRate > 0

How should I proceed? Create a new issue and PR?

Dash-Industry-Forum / dash.js