Fixes demo graphs for improved bandwidth estimation

[silltho]

Why is this Pull Request needed?

The demo calculates the bandwidth for each fragment separately from the abr-controller. Therefore it needs to be updated to ensure that the same values are used for the bandwidth estimation and the bandwidth graph on the demo page.

Are there any points in the code the reviewer needs to double check?

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

[itsjamie]

I assume this check was based on this code.

// Only count non-alt-audio frags which were actually buffered in our BW calculations
    if (frag.type !== PlaylistLevelType.MAIN || frag.sn === 'initSegment') {
      return;
    }

This is to only use the larger segments to get a more accurate line speed in our EWMA sample. However, this data should have all segments so I'm not sure this is necessary.

[silltho]

I assume this check was based on this code.

Exactly.

This is to only use the larger segments to get a more accurate line speed in our EWMA sample. However, this data should have all segments so I'm not sure this is necessary.

That's a good question. I used the demo to verify the changes made to the bw estimation and found it counterintuitive that the graph shows different data than the bw estimator uses internally. Adding console.logs instead to get a better understanding of what is happening.

However I can see that the graph might show the bandwidth of every request regarding what is being selected for estimation.

[itsjamie]

I think making the change so the bitrate is calculated the same is a good change and should definitely be made.

What do you think about simply grabbing the calculated EWMA and showing it on the metrics page? That way you would get the exact data, and we wouldn't need to re-calculate the demo page if we ever changed the way the metric was calculated.

[itsjamie]

I believe I understand that you want the bandwidth (bw) value calculated in the event to accurate represent what hls.js would have sampled into the EWMA class, is that accurate?

If that's true, then I think the direction you have with the calculation for processingMs and using stats.loaded is on track.

Some of the early exits are changing which fragments will be stored as data on the demo page, which I don't think is ideal.

[silltho]

Thanks for the review i will work through your feedback and update the PR next week.

[silltho]

Some of the early exits are changing which fragments will be stored as data on the demo page, which I don't think is ideal.

Agree. What do you think would be good solution? Adding a new graph comes to my mind.

[itsjamie]

@silltho I think you might need to rebase against feature/estimate-ttfb-fix-part-stats-improve-abr to clean up some of the conflicts.

[itsjamie]

Agree. What do you think would be good solution? Adding a new graph comes to my mind.

Yeah, rather than an entirely new graph, what do you think about directly getting the EWMA value and add it directly on the bitrate graph as a new value? Leaving the bitrate graph showing all requests, and the moving bitrate. This would allow you to easily see the difference between the raw bitrate/s average required, versus what the EWMA is reporting in code for selection.

[silltho]

@itsjamie I've added the estimated bandwidth as yellow line to the bitrate graph. I had to to increase the height of the graph (not sure if this has an impact on something)

[itsjamie]

Just did a rebase and fixed the conflicts, looks like the core changes have all been merged already, so nice finding those @silltho. I'm running the demo changes locally while the CI runs.

[itsjamie]

Looks great! Thank you.