issue/3619 CMCD #3662

littlespex · 2021-09-24T17:39:08Z

Description

Add support for including Common Media Client Data (CMCD) in outgoing requests.

Fixes #3619.

NOTE: The following fields have not been implemented: rtp, nrr, nor, dl

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to
not work as expected)
This change requires a documentation update

Checklist:

I have signed the Google CLA https://cla.developers.google.com
My code follows the style guidelines of this project
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
I have verified my change on multiple browsers on different platforms
I have run ./build/all.py and the build passes
I have run ./build/test.py and all tests pass

…haka-player into issue/3619-cmcd # Conflicts: # externs/shaka/player.js

Visual Studio Code has different formatting rules than the Shaka project. This can be overridden by adding custom settings via the .vscode/settings.json file. This folder should not be committed.

google-cla · 2021-09-24T17:40:34Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

joeyparrish

Looks good overall. Just a few comments. Thanks!

joeyparrish · 2021-10-08T20:26:33Z

externs/shaka/player.js

+ *   A GUID identifying the current playback session. A playback session
+ *   typically ties together segments belonging to a single media asset.
+ *   Maximum length is 64 characters. It is RECOMMENDED to conform to the UUID
+ *   specification.


Can we please add a note that by default, we will generate a session ID for you on each load() call?

The doc for the sessionId config property have been updated.

joeyparrish · 2021-10-08T20:30:15Z

lib/hls/hls_parser.js

@@ -1488,7 +1493,7 @@ shaka.hls.HlsParser = class {
      pixelAspectRatio: undefined,
      width: undefined,
      height: undefined,
-      bandwidth: undefined,
+      bandwidth: bandwidth,


Bandwidth in HLS is really only known at the Variant level, not for individual streams.

This change has been reverted.

joeyparrish · 2021-10-08T20:31:38Z

lib/hls/hls_parser.js

+        fullRequest,
+        type,
+        reference instanceof shaka.media.InitSegmentReference,
+        reference.endTime - reference.startTime,


If I'm not mistaken, InitSegmentReference doesn't have start and end times. Is it okay for this argument to modifySegmentReference to be NaN?

Yes, the serialization function will ignore NaN, null, undefined and '' values.

joeyparrish · 2021-10-08T20:35:08Z

lib/player.js

@@ -7,6 +7,7 @@
 goog.provide('shaka.Player');

 goog.require('goog.asserts');
+goog.require('shaka.util.CmcdManager');


nit: Please alphabetize these requirements

alphabetized

joeyparrish · 2021-10-08T20:40:37Z

lib/util/cmcd_manager.js

+   */
+  configure(config) {
+    this.config_ = config;
+    this.sid_ = config.sessionId || window.crypto.randomUUID();


In other components, configure() is something that can be called multiple times during the life of the component, to update the config. Here, CMCD config is locked in once playback starts in load().

It isn't necessarily a problem that the config is locked in for playback, but if that's the case, I suggest you move the config to the constructor. That way, someone doesn't later call this component's configure() method from Player's configure() to update the config, triggering changes to the session ID mid-playback.

Alternately, if you write a unit test on Player that proves that configure() of CmcdManager is only called once during load(), and not during subsequent configure() calls, that would suffice to prevent mistakes, I think.

The cmcd configuration has been moved into the constructor

joeyparrish · 2021-10-08T20:55:36Z

lib/util/cmcd_manager.js

+   */
+  getObjectType_(type, init, mimeType, codecs) {
+    if (type == 'text' && mimeType === 'text/vtt') {
+      return shaka.util.CmcdManager.ObjectType.CAPTION;


Is "caption" really spec'd to mean "VTT" in CMCD?

I thought the meaning of caption was something like CC/CEA608 for the deaf, where as everything else was subtitles (for the hearing, but in another language).

The spec defines two text related object types:

c = caption or subtitle

tt = ISOBMFF timed text track

Then it sounds like this condition is completely unnecessary. You already have a check for type == 'text' below. This extra condition at the top is redundant with that one, and would also suppress INIT type for the init segments of text streams embedded in MP4.

joeyparrish · 2021-10-08T20:56:21Z

lib/util/cmcd_manager.js

+      if (mimeType === 'application/mp4') {
+        return shaka.util.CmcdManager.ObjectType.TIMED_TEXT;


If "TIMED_TEXT" corresponds to TTML, then this seems wrong. Both VTT and TTML can appear in mp4 containers.

It is my understanding that TIMED_TEXT refers to text based content delivered via a container, and CAPTION refers to text based content delivered via a text file.

joeyparrish · 2021-10-08T20:57:06Z

lib/util/cmcd_manager.js

+      case 'video/webm':
+      case 'video/mp4':
+        return shaka.util.CmcdManager.ObjectType.MUXED;


What is meant by "muxed"? Does that imply audio & video together in the stream?

What about TS containers?

MUXED refers to a segment that contains both audio and video. This function is only called when src is being used to load the video, which would be mp4 and web files, and hls manifests in Safari. Are there any others that are missing?

joeyparrish · 2021-10-08T20:58:08Z

lib/util/cmcd_manager.js

+    for (const range of ranges) {
+      if (range.start < start) {
+        start = range.start;
+      }
+      if (range.end > end) {
+        end = range.end;
+      }
+    }


I believe the ranges are sorted and disjoint. So if the list is non-empty, you should be able to do start = ranges[0].start and end = ranges.pop().end

@wilaw Is the value of bl supposed to represent just the forward buffer, or the entire buffer length?

Yes, forward buffer only. And only the immediate contiguous range in front of the current playhead, in the case that you have a multi-part forward buffer.

The getBufferLength_ function has been updated.

joeyparrish · 2021-10-08T20:59:05Z

lib/util/cmcd_manager.js

+      const stream = variant[type] || variant;
+      if (stream.bandwidth > top.bandwidth) {
+        top = stream;
+      }


For HLS compatibility, I believe you should be using variant.bandwidth instead of stream.bandwidth.

We originally used the variant bandwidth. This was flagged as an error during the spec compliance testing because the top bitrate didn't match any of the values in the manifest (because the variant bandwidth is an aggregate). This is why we first look for the specific value from variant.video.bandwidth and variant.audio.bandwidth, then fallback to variant.bandwidth.

…rward buffer

lib/util/cmcd_manager.js

joeyparrish · 2021-10-18T20:20:27Z

lib/util/cmcd_manager.js

+   */
+  getObjectType_(type, init, mimeType, codecs) {
+    if (type == 'text' && mimeType === 'text/vtt') {
+      return shaka.util.CmcdManager.ObjectType.CAPTION;


Then it sounds like this condition is completely unnecessary. You already have a check for type == 'text' below. This extra condition at the top is redundant with that one, and would also suppress INIT type for the init segments of text streams embedded in MP4.

joeyparrish · 2021-10-18T20:23:16Z

lib/util/cmcd_manager.js

+      return NaN;
+    }
+
+    return (range.end - start) * 1000;


We have a utility for this that you might want to use instead: shaka.media.TimeRangesUtils

return shaka.media.TimeRangesUtils.bufferedAheadOf(video.buffered, video.currentTime);

It wouldn't cover text, though, so feel free to keep this implementation if you like.

I left the getBufferInfo_ calculation as is so that text buffers are covered. The redundant conditional in getObjectType_ has been removed.

joeyparrish · 2021-10-20T00:00:27Z

Looks good, but there seems to be a merge conflict. Can you take a look?

littlespex · 2021-10-20T01:01:51Z

Resolved.

shaka-bot · 2021-10-20T02:23:12Z

All tests passed!

joeyparrish · 2021-10-20T19:22:49Z

Thank you for your contribution!

ppaneksamsung · 2021-10-25T06:54:23Z

When I tried to run Shaka Player tests via HTTP with this feature, all lot of them (~100) fail on:

TypeError: window.crypto.randomUUID is not a function in lib/util/cmcd_manager.js (line 4531)
_class@lib/util/cmcd_manager.js:34:51 <- lib/util/cmcd_manager.js:4531:104
...

Previously, there was no such issue. window.crypto.randomUUID is only supported in secure contexts.

@joeyparrish, is it a regression or expected behaviour? Should it be possible to run tests via HTTP? Or maybe HTTPS is the only supported protocol?

joeyparrish · 2021-10-26T00:47:59Z

It should be possible to run tests over HTTP. There should be two paths to making this work:

Using localhost, which counts as a secure context without HTTPS
Via the polyfill for randomUUID, which should be installed by default: https://github.com/google/shaka-player/blob/master/lib/polyfill/random_uuid.js

If the polyfill is not working on plain HTTP pages, please file a bug and we can investigate. Thanks!

ppaneksamsung · 2021-11-02T12:27:17Z

I tried the first path (localhost), but it didn't work. I guess that my browser (Tizen TV) does not support randomUUID API so switching to a secure context does not help.

It seems that polyfill is not installed properly, see my patch: ppaneksamsung@b0d760e.

dsparacio · 2022-01-07T16:44:08Z

@joeyparrish, Congratulations on the latest release!
We noticed that v3.2.2 may not have carried this ticket (not seeing it in the release notes) and cant seem to find out info on when you plan of did push it to prod. Any info on what is needed from us or if we missed something and it is already deployed would be awesome. Sorry to bother, Happy New Year

joeyparrish · 2022-01-07T18:28:59Z

In semantic versioning, new features require a minor release, which would put CMCD support in v3.3. With updates pushed to all existing release branches yesterday, I'm working on the new v3.3.x branch and v3.3.0 release today (though the process may spill into next week).

dsparacio · 2022-01-10T19:45:56Z

@joeyparrish Thank you very much!! Extremely appreciated - ViacomCBS team.

dsparacio and others added 29 commits October 29, 2020 16:06

Merge remote-tracking branch 'upstream/master'

90b4527

Merge branch 'master' of https://github.com/google/shaka-player

49d611f

Merge remote-tracking branch 'upstream/master'

93fdcd3

Merge branch 'google:master' into master

0b7ac5b

Update AUTHORS and CONTRIBUTORS

63791ed

Add CMCD configuration

1ccca06

Add CMCD configuration

e38f313

Merge branch 'issue/3619-cmcd' of https://github.com/cbsinteractive/s…

c2e9fff

…haka-player into issue/3619-cmcd # Conflicts: # externs/shaka/player.js

Merge branch 'google:master' into master

6db0e3c

Add vscode settings folder to gitignore

53b4164

Visual Studio Code has different formatting rules than the Shaka project. This can be overridden by adding custom settings via the .vscode/settings.json file. This folder should not be committed.

Merge branch 'google:master' into master

efaebee

Add missing CMCD property to typedef

efdbe59

Add basic CMCD data to segment requests

43b8bb2

Move CMCD types and object to shaka.cmcd namespace

c57c250

Add basic CMCD data to manifest requests

26e6b87

Fix manifest tests

136e30e

Decouple streaming_engine and cmcd_manager

6e53bdc

Add new cmcd config fields to demo

1717bfa

Add unit test for CmcdManager

aef9356

Merge branch 'google:master' into master

7fed0d0

Merge branch 'master' into issue/3619-cmcd

f9f7032

Fix lint issues

3d9b2f5

Fix doc formatting

af67fc2

Add code comments

eb5e216

Fix incorrect default value for CmcdConfiguration useHeaders

baeeaca

Fix issue where custom fields were not being passed as headers

9949fea

Add support for su and bs parameters

abdbef8

Add CMCD data to files loaded via src=

af7a001

Improve test coverage

f18d21f

joeyparrish requested changes Oct 8, 2021

View reviewed changes

littlespex added 10 commits October 8, 2021 15:01

Add note about session id being auto-generated at load

f13588b

revert hls_parser bandwidth change

a168e3d

Alphabetize imports

340908d

Rename initialized_ to playbackStarted_

131fed7

Refactor applyManifestData to use StreamFormat enum

8f4cfe8

Add missing return types

8a13831

Move cmcd configuration into constructor

660a4e5

Consolidate applySegmentData arguments

1957b95

Optimize getBufferLength function

b8ec43a

Update getBufferLength to meet spec requirements of only reporting fo…

f847d3e

…rward buffer

joeyparrish requested changes Oct 18, 2021

View reviewed changes

Remove redundant conditional

860ae15

joeyparrish approved these changes Oct 19, 2021

View reviewed changes

Merge branch 'master' into issue/3619-cmcd

11ac1a1

joeyparrish approved these changes Oct 20, 2021

View reviewed changes

joeyparrish merged commit 221637a into shaka-project:master Oct 20, 2021

littlespex deleted the issue/3619-cmcd branch November 17, 2022 17:28

github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023

		if (mimeType === 'application/mp4') {
		return shaka.util.CmcdManager.ObjectType.TIMED_TEXT;

issue/3619 CMCD #3662

issue/3619 CMCD #3662

Conversation

littlespex commented Sep 24, 2021

Description

Type of change

Checklist:

google-cla bot commented Sep 24, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

joeyparrish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littlespex Oct 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littlespex Oct 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeyparrish commented Oct 20, 2021

littlespex commented Oct 20, 2021

shaka-bot commented Oct 20, 2021

joeyparrish commented Oct 20, 2021

ppaneksamsung commented Oct 25, 2021

joeyparrish commented Oct 26, 2021

ppaneksamsung commented Nov 2, 2021

dsparacio commented Jan 7, 2022

joeyparrish commented Jan 7, 2022

dsparacio commented Jan 10, 2022

littlespex Oct 8, 2021 •

edited

Loading

littlespex Oct 9, 2021 •

edited

Loading