feat(cmcdv2): implement rtp, lb, dl, pb, tpb, ttfb and ttlb#7895
Conversation
littlespex
left a comment
There was a problem hiding this comment.
All of these changes look good for request/response based reports, but event based reports would not get these new values, or any values set in applyFragmentData for that matter. Ideally these fields would be set via reporter.update so that the values would persist for all reports. However, this would be a much larger change because the persistent values would need to be properly flagged with object type tokens. For now, I think this work leaves the feature in a better place than it was, so we can merge it.
I've been playing around with how we could workaround this and enable these keys in event mode, and here's an implementation I've been drafting for this PR: Persists those keys in the reporter's shared state via Each key is updated from the hls.js event that provides the most accurate context for it:
The behavior of request reports is unchanged — Known limitations (pre-existing or inherent to the approach):
|
| private getLowestBandwidth(fragment: Fragment | MediaFragment) { | ||
| let bitrate: number = Infinity; | ||
| let levels; | ||
| const hls = this.hls; | ||
|
|
||
| if (fragment.type === 'audio') { | ||
| levels = hls.audioTracks; | ||
| } else { | ||
| const max = hls.maxAutoLevel; | ||
| const len = max > -1 ? max + 1 : hls.levels.length; | ||
| levels = hls.levels.slice(0, len); | ||
| } | ||
|
|
||
| levels.forEach((level) => { | ||
| if (level.bitrate < bitrate) { | ||
| bitrate = level.bitrate; | ||
| } | ||
| }); | ||
|
|
||
| return bitrate < Infinity ? bitrate : NaN; | ||
| } |
There was a problem hiding this comment.
Audio segments do not have bitrate. HLS variant ("level") bitrates are the combined bitrate of audio and video.
There was a problem hiding this comment.
Same goes for getTopBandwidth but it looks like I let that slip.
There was a problem hiding this comment.
If you wanted to use level.bitrate to calculate video bitrate, you'd need to subtract an estimate of the audio bitrate. audio bitrate can be guessed, but not measured until audio fragments are loaded.
Co-authored-by: Rob Walch <robwalch@users.noreply.github.com>
|
All comments have been addressed 😄. Let me know if there's anything else we can improve. We can also discuss the feasibility of enabling these keys for event reports on this PR |
|
Following up on the known limitation around event-mode reports, I've been thinking about a scoped approach that could resolve it without the larger per-object-type refactor. The idea is to gate all For non-muxed streams (alternate audio renditions), we'd skip the update entirely rather than report without Wanted to float this before going further, does this direction seem good to you? |
|
We still wouldn't want to include |
This PR will...
Complete the implementation of several CMCD v2 request and response keys in CMCDController that were previously missing.
rtp(Requested Throughput): segment bitrate ×rtpSafetyFactor(default 5), rounded to nearest 100 kbpslb(Lowest Bitrate): lowest bitrate within the currentmaxAutoLevelrangepb(Playhead Bitrate): bitrate of the level currently at the playhead, tracked viaFRAG_CHANGEDtpb(Top Playhead Bitrate): bitrate of the highest level allowed bymaxAutoLeveldl(Deadline): buffer length divided by playback rate, rounded to nearest 100 msttfb/ttlb: derived from fragment loader response timing by wrapping theonSuccesscallback to callreporter.recordResponseReceived()Adds a new config option
rtpSafetyFactor(default:5) to let operators tune the headroom multiplier used forrtp.Why is this Pull Request needed?
These keys are part of the CMCD v2 spec and were previously missing. Without them, CDN servers cannot use CMCD v2 signals to make informed caching and bitrate decisions.
Are there any points in the code the reviewer needs to double check?
playheadLeveltracking relies onFRAG_CHANGED— verify this is the right event to determine which level is currently at the playhead vs. being buffered ahead.tpbuseshls.maxAutoLevel, which may return-1when unconstrained; the code guards this but worth a second look.onSuccesswrapper in the fragment loader spreadscallbacks— verify there are no edge cases where this wrapping could affect existing loader error handling.Resolves issues:
#6090
Checklist