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

BW estimator is sometimes called with missing stats for LL-HLS #3578

Closed
4 of 5 tasks
kanongil opened this issue Mar 6, 2021 · 2 comments · Fixed by #4825
Closed
4 of 5 tasks

BW estimator is sometimes called with missing stats for LL-HLS #3578

kanongil opened this issue Mar 6, 2021 · 2 comments · Fixed by #4825

Comments

@kanongil
Copy link
Contributor

kanongil commented Mar 6, 2021

There is a problem with the consistency of the bandwidth reporting.

The issue seems to be related to level switches (maybe due to aborts??).

What version of Hls.js are you using?

1.0.0-rc3

What browser and OS are you using?

macOS Chrome

Test stream:

https://stream.sob.m-dn.net/live/sb1-ll/index.m3u8

Checklist

  • The stream has correct Access-Control-Allow-Origin headers (CORS)
  • There are no network errors such as 404s in the browser console when trying to play the stream

Steps to reproduce

  1. Observe numBytes value passed to estimator here:
    sample(durationMs: number, numBytes: number) {
  2. Play stream
  3. Force level switch a few times

Expected behavior

All passed sample contain actual data, and not zeroes.

Actual behavior

Sometimes samples contain numBytes = 0 and seriously wrong durationMs (ex. 629418.165).

Example of a LoadStats used to compute the supplied values:
Screenshot 2021-03-06 at 16 45 37

@robwalch robwalch added this to the 1.0.0 milestone Mar 6, 2021
@robwalch robwalch removed this from the 1.0.0 milestone Mar 8, 2021
@kanongil
Copy link
Contributor Author

kanongil commented Mar 9, 2021

This is still an issue in RC4. I suspect it is related to the parser blocking here:

this.hls.trigger(Events.FRAG_BUFFERED, {

If you look at the traffic, you will see that ABR controller onFragParsed is not triggered for all transfers when performing a LL-HLS switch to a non-indendent part. If this is by design (which I suspect it is), a fix could be to move the logic back to the more direct onFragLoaded event (and use stats.loading.end instead of stats.parsing.end to compute the transfer duration).

It is unclear what the reasoning was to include the parsing time, from the original commit.

@robwalch robwalch added this to the 1.0.0 milestone Mar 9, 2021
@robwalch robwalch added Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. and removed cannot reproduce labels Mar 9, 2021
@robwalch robwalch removed this from the 1.0.0 milestone Apr 1, 2021
@robwalch robwalch added cannot reproduce and removed Needs Triage If there is a suspected stream issue, apply this label to triage if it is something we should fix. labels Apr 1, 2021
@robwalch
Copy link
Collaborator

robwalch commented Apr 1, 2021

@kanongil let me know if you are still seeing this in the latest release. I have not been able to reproduce this recently.

Adding a console.assert to the code in question may help me encounter the problem during development, and would be a welcome contribution.

robwalch added a commit that referenced this issue Aug 4, 2022
…ents and higher TTFB

Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)
robwalch added a commit that referenced this issue Aug 4, 2022
…ents and higher TTFB

Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)
@robwalch robwalch added the Verify Fixed An unreleased bug fix has been merged and should be verified before closing. label Aug 4, 2022
robwalch added a commit that referenced this issue Aug 5, 2022
…ents and higher TTFB

Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)
robwalch added a commit that referenced this issue Nov 28, 2022
…ents and higher TTFB

Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)
@robwalch robwalch added this to the 1.4.0 milestone Dec 1, 2022
silltho pushed a commit to silltho/hls.js that referenced this issue Dec 2, 2022
…ents and higher TTFB

Fixes video-dev#3578 (special thanks to @Oleksandr0xB for submitting video-dev#4283)
Fixes video-dev#3563 and Closes video-dev#3595 (special thanks to @kanongil)
robwalch added a commit that referenced this issue Jan 9, 2023
…ents and higher TTFB

Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)
robwalch added a commit that referenced this issue Jan 12, 2023
…ents and higher TTFB

Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)
robwalch added a commit that referenced this issue Jan 18, 2023
* Improve bandwidth estimation and adaptive switching with smaller segments and higher TTFB
Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)

* Load rate and loaded delay calculation fixes

* Convert ttfbEstimate to seconds

* Include main variant init segments in TTFB sampling

* Use ttfb estimate in abandon rules down-switch timing
robwalch added a commit that referenced this issue Jan 26, 2023
* Improve bandwidth estimation and adaptive switching with smaller segments and higher TTFB
Fixes #3578 (special thanks to @Oleksandr0xB for submitting #4283)
Fixes #3563 and Closes #3595 (special thanks to @kanongil)

* Load rate and loaded delay calculation fixes

* Convert ttfbEstimate to seconds

* Include main variant init segments in TTFB sampling

* Use ttfb estimate in abandon rules down-switch timing
@robwalch robwalch removed the Verify Fixed An unreleased bug fix has been merged and should be verified before closing. label Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

2 participants