fix: Fix CMCD top bitrate reporting (shaka-project#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 shaka-project#3851 Co-authored-by: Dan Sparacio <[email protected]>
joeyparrish · Jan 12, 2022 · 922778a · 922778a
1 parent d4ab3ca
commit 922778a
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 26 deletions.
diff --git 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
@@ -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.<shaka.extern.Track>,
  *   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.<shaka.extern.Track>} 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
@@ -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.<shaka.extern.Track>} */([
+        {
+          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',