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

bugfix: Don't sync to live when media is in paused #4402

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

jony89
Copy link
Contributor

@jony89 jony89 commented Oct 27, 2021

This PR will...

fix issue when the video is in paused state yet still hls.js updating the video element current time.

this was fixed and merged in old versions (0.1.x) and probably got missed when refactoring base-stream-controller

see #2417

I was able to reproduce it in the demo page:

https://hls-js.netlify.app/demo/

choose Low Latency HLS Sample of Big Buck.. (or go directly to https://hls-js.netlify.app/demo/?src=https%3A%2F%2Fstream.mux.com%2Fv69RSHhFelSm4701snP22dYz2jICy4E4FUyk02rW4gxRM.m3u8&demoConfig=eyJlbmFibGVTdHJlYW1pbmciOnRydWUsImF1dG9SZWNvdmVyRXJyb3IiOnRydWUsInN0b3BPblN0YWxsIjpmYWxzZSwiZHVtcGZNUDQiOmZhbHNlLCJsZXZlbENhcHBpbmciOi0xLCJsaW1pdE1ldHJpY3MiOi0xfQ== )

and the following configuration :

{
  "debug": true,
  "enableWorker": true,
  "lowLatencyMode": true,
  "backBufferLength": 90,
  "liveSyncDurationCount": 3,
  "liveMaxLatencyDurationCount": 6
}

now click the pause button and see how the video frame is changing every 6 seconds.

Resolves issues:

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

question

the above fix works for me. but I am wondering whether _trySkipBufferHole and _tryNudgeBuffer in gap-controller need this check as well?

this was fixed and merged in old versions (0.1.x) and probably got missed when refactoring base-stream-controller

see video-dev#2417
@jony89
Copy link
Contributor Author

jony89 commented Nov 7, 2021

@robwalch ?

@jony89
Copy link
Contributor Author

jony89 commented Dec 6, 2021

@lpommers any help here?

@lpommers
Copy link
Contributor

lpommers commented Dec 8, 2021

@jony89 I'm not sure i'll be of much help here (I'm not really a maintainer or expert of hls.js).

I think you might want to fill out a bug report with a config, log, and test stream (if possible - can be hard with live). Testing one of my own live streams I wasn't able to reproduce the issue of the paused frame of the stream being changed.

You might also find these PRs/issues helpful for context:

@jony89
Copy link
Contributor Author

jony89 commented Dec 8, 2021

@robwalch not sure why the downvote? this PR is identical and the same issue as #2417

yet the condition was removed while refactoring.

@lpommers
Copy link
Contributor

lpommers commented Dec 8, 2021

I think this is considered "works as expected" because v1 follows the pattern for native hls playback from the <video /> element in Safari.

My understanding is that live streams will continue to buffer up to specified amount. Eventually, the browser will remove media from the buffer and the player will need to seek to a buffered range, even if paused. If we were to take a live native <video /> in Safari, pause it and have the browser dump the whole buffer, it would have to seek the currentTime of the media to somewhere in the sliding playlist. Which is what hls.js seems to do as well...

(I agree though from UX/user perspective, it would be cool if browsers were able to keep the tiniest chunk of the buffer that is being visibly displayed, so that the frame does not visibly move)

@jony89
Copy link
Contributor Author

jony89 commented Jan 9, 2022

so if the user click Pause, during Live streaming, it's sounds ok to you to change the frame every few seconds?

I was able to reproduce it in the demo page:

https://hls-js.netlify.app/demo/

choose Low Latency HLS Sample of Big Buck.. (or go directly to https://hls-js.netlify.app/demo/?src=https%3A%2F%2Fstream.mux.com%2Fv69RSHhFelSm4701snP22dYz2jICy4E4FUyk02rW4gxRM.m3u8&demoConfig=eyJlbmFibGVTdHJlYW1pbmciOnRydWUsImF1dG9SZWNvdmVyRXJyb3IiOnRydWUsInN0b3BPblN0YWxsIjpmYWxzZSwiZHVtcGZNUDQiOmZhbHNlLCJsZXZlbENhcHBpbmciOi0xLCJsaW1pdE1ldHJpY3MiOi0xfQ== )

and the following configuration :

{
  "debug": true,
  "enableWorker": true,
  "lowLatencyMode": true,
  "backBufferLength": 90,
  "liveSyncDurationCount": 3,
  "liveMaxLatencyDurationCount": 6
}

now click the pause button and see how the video frame is changing every 6 seconds.

@robwalch I can't see how this is work as expected behavior ?

@phillipseamore
Copy link
Contributor

This happens because of liveMaxLatencyDurationCount being set to other than Infinity. You are telling hls.js that the maximum allowed delay from live is 6 segment durations.

You could possibly change liveMaxLatencyDurationCount to Infinity on pause and back again on play.

@jony89
Copy link
Contributor Author

jony89 commented Jan 9, 2022

@phillipseamore thanks, indeed that is one workaround, that one can use for now, yet changing configurations just for pause to actually pause the video seems a bit hacky to me. so I keep this PR open and wait for more responses, unless you think this change is wrong?

@phillipseamore
Copy link
Contributor

I see nothing wrong with the patch.

Personally I do feel pausing a live stream without a DVR window should behave more like stopping it completely (e.g. no more fetching playlists, chunks or updating time or the buffer). For streams with a window shorter than 1 minute I just tear down hls.js and throw a play button overlay over the video element (this could be an image grab of the last frame in the video element as well) and build it up on the rare occasion a user hits play again, saving me a little bit in server resources and bandwidth.

@stale
Copy link

stale bot commented Apr 27, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the Stale label Apr 27, 2022
@stale stale bot removed the Stale label Apr 27, 2022
@jony89
Copy link
Contributor Author

jony89 commented Jul 19, 2022

@robwalch can you please help me here, why did we merge #2417 yet this PR is not acceptable? they both are the same fix..

@robwalch
Copy link
Collaborator

@robwalch can you please help me here, why did we merge #2417 yet this PR is not acceptable? they both are the same fix..

Hi @jony89,

Even when playback is paused, HLS.js continues to buffer media, refresh live playlists, and adhere to the live sync logic as configured in the player API for the active HLS stream. In this case it's liveMaxLatencyDurationCount: 6 that triggers a seek to maintain a distance of 6 target durations or less.

If you would like to stop activity when paused so that the frame does not update, call hls.stopLoad() when the media element is paused, and hls.startLoad() when unpaused.

@robwalch robwalch added Enhancement Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. and removed Works as expected labels Jul 19, 2022
@robwalch robwalch added this to Low-Latency Improvements in Release Planning and Backlog Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Awaiting Triage
Release Planning and Backlog
  
Low-Latency Improvements
Development

Successfully merging this pull request may close these issues.

None yet

4 participants