From 922778a5ebd2d58ca0c1e804745ca40cda1228bc Mon Sep 17 00:00:00 2001 From: Casey Occhialini <1508707+littlespex@users.noreply.github.com> Date: Wed, 12 Jan 2022 00:40:04 -0500 Subject: [PATCH] fix: Fix CMCD top bitrate reporting (#3852) The tb (top bitrate) property should honor bitrate/size ABR constraints and only report the top playable bitrate. The tb property should only report the bandwidth for that segments type (audio, video, muxed). Fix incorrect top bitrate CMCD reporting by: Honoring the player's current bitrate/size ABR constraints. Only reporting the bandwidth for that segments type (audio, video, muxed). Fixes #3851 Co-authored-by: Dan Sparacio --- lib/player.js | 2 +- lib/util/cmcd_manager.js | 45 +++++++++++++++++++++------------- test/util/cmcd_manager_unit.js | 24 ++++++++++++------ 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/lib/player.js b/lib/player.js index 964c67c609..634b381cda 100644 --- a/lib/player.js +++ b/lib/player.js @@ -2824,7 +2824,7 @@ shaka.Player = class extends shaka.util.FakeEventTarget { getBandwidthEstimate: () => abr ? abr.getBandwidthEstimate() : NaN, getBufferedInfo: () => this.getBufferedInfo(), getCurrentTime: () => this.video_ ? this.video_.currentTime : 0, - getManifest: () => this.manifest_, + getVariantTracks: () => this.getVariantTracks(), getPlaybackRate: () => this.getPlaybackRate(), isLive: () => this.isLive(), }; diff --git a/lib/util/cmcd_manager.js b/lib/util/cmcd_manager.js index 894e15650e..4caa230465 100644 --- a/lib/util/cmcd_manager.js +++ b/lib/util/cmcd_manager.js @@ -118,10 +118,11 @@ shaka.util.CmcdManager = class { data.ot = this.getObjectType_(segmentInfo); - const isMedia = data.ot === shaka.util.CmcdManager.ObjectType.VIDEO || - data.ot === shaka.util.CmcdManager.ObjectType.AUDIO || - data.ot === shaka.util.CmcdManager.ObjectType.MUXED || - data.ot === shaka.util.CmcdManager.ObjectType.TIMED_TEXT; + const ObjectType = shaka.util.CmcdManager.ObjectType; + const isMedia = data.ot === ObjectType.VIDEO || + data.ot === ObjectType.AUDIO || + data.ot === ObjectType.MUXED || + data.ot === ObjectType.TIMED_TEXT; if (isMedia) { data.bl = this.getBufferLength_(segmentInfo.type); @@ -131,7 +132,9 @@ shaka.util.CmcdManager = class { data.br = segmentInfo.bandwidth / 1000; } - data.tb = this.getTopBandwidth_(segmentInfo.type) / 1000; + if (isMedia && data.ot !== ObjectType.TIMED_TEXT) { + data.tb = this.getTopBandwidth_(data.ot) / 1000; + } this.apply_(request, data); } catch (error) { @@ -387,23 +390,31 @@ shaka.util.CmcdManager = class { * @private */ getTopBandwidth_(type) { - const manifest = this.playerInterface_.getManifest(); - if (!manifest) { + const variants = this.playerInterface_.getVariantTracks(); + if (!variants.length) { return NaN; } - const variants = (type === 'text') ? - manifest.textStreams : manifest.variants; - let top = variants[0][type] || variants[0]; + let top = variants[0]; for (const variant of variants) { - const stream = variant[type] || variant; - if (stream.bandwidth > top.bandwidth) { - top = stream; + if (variant.type === 'variant' && variant.bandwidth > top.bandwidth) { + top = variant; } } - return top.bandwidth; + const ObjectType = shaka.util.CmcdManager.ObjectType; + + switch (type) { + case ObjectType.VIDEO: + return top.videoBandwidth || NaN; + + case ObjectType.AUDIO: + return top.audioBandwidth || NaN; + + default: + return top.bandwidth; + } } /** @@ -553,7 +564,7 @@ shaka.util.CmcdManager = class { * getBandwidthEstimate: function():number, * getBufferedInfo: function():shaka.extern.BufferedInfo, * getCurrentTime: function():number, - * getManifest: function():shaka.extern.Manifest, + * getVariantTracks: function():Array., * getPlaybackRate: function():number, * isLive: function():boolean * }} @@ -564,8 +575,8 @@ shaka.util.CmcdManager = class { * Get information about what the player has buffered. * @property {function():number} getCurrentTime * Get the current time - * @property {function():shaka.extern.Manifest} getManifest - * Get the manifest + * @property {function():Array.} getVariantTracks + * Get the variant tracks * @property {function():number} getPlaybackRate * Get the playback rate * @property {function():boolean} isLive diff --git a/test/util/cmcd_manager_unit.js b/test/util/cmcd_manager_unit.js index 3dd20fe581..196c5901a0 100644 --- a/test/util/cmcd_manager_unit.js +++ b/test/util/cmcd_manager_unit.js @@ -85,12 +85,20 @@ describe('CmcdManager', () => { {start: 35, end: 40}, ], }), - getManifest: () => /** @type {shaka.extern.Manifest} */({ - variants: [ - {video: {bandwidth: 50000}}, - {video: {bandwidth: 5000000}}, - ], - }), + getVariantTracks: () => /** @type {Array.} */([ + { + type: 'variant', + bandwidth: 50000, + videoBandwidth: 40000, + audioBandWidth: 10000, + }, + { + type: 'variant', + bandwidth: 5000000, + videoBandwidth: 4000000, + audioBandWidth: 1000000, + }, + ]), getPlaybackRate: () => 1, getCurrentTime: () => 10, }; @@ -177,7 +185,7 @@ describe('CmcdManager', () => { const uri = 'https://test.com/test.mpd?CMCD=bl%3D21200%2Cbr%3D5234%2Ccid%3D%22' + 'testing%22%2Cd%3D3330%2Cmtp%3D10000%2Cot%3Dv%2Csf%3Dd%2C' + 'sid%3D%222ed2d1cd-970b-48f2-bfb3-50a79e87cfa3%22%2Cst%3Dv%2Csu%2C' + - 'tb%3D5000'; + 'tb%3D4000'; expect(r.uris[0]).toBe(uri); }); @@ -214,7 +222,7 @@ describe('CmcdManager', () => { cmcdManager.applySegmentData(r, segmentInfo); expect(r.headers).toEqual({ 'testing': '1234', - 'CMCD-Object': 'br=5234,d=3330,ot=v,tb=5000', + 'CMCD-Object': 'br=5234,d=3330,ot=v,tb=4000', 'CMCD-Request': 'bl=21200,mtp=10000,su', 'CMCD-Session': 'cid="testing",sf=d,' + 'sid="2ed2d1cd-970b-48f2-bfb3-50a79e87cfa3",st=v',