Refactor CMCD controller and tests to use the common media library utilities #5903

littlespex · 2023-10-13T06:19:01Z

This PR will...

Replace the utilities used to append CMCD data to outgoing requests with the versions provided by @svta/common-media-library

Why is this Pull Request needed?

The CMCD utilities have been manually implemented in multiple player libraries. When bugs need to be fixed, or features added, then need to be done across all of these projects. The common media library provides a single place for these utilities to live and be maintained.

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

As a result of the refactor, the static functions provided by the CMCDController class are no longer needed. The functions have been removed, but this technically changes the public API. I don't think these functions are commonly used, but if this is considered a breaking change, we can restore the functions and replace the function bodies with calls to the common media library's version.
A change to the build script was needed to transpile the common media library because it is only published in ES2017. The change is limited to the @svta folder, but any changes to the build script should be scrutinized.

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

robwalch

Package lock file changes should be limited to adding @svta/common-media-library and any peer dependencies. It looks like you may have performed an npm install or update and inadvertently updated other dependencies which we let renovate[bot] manage.

Please reset package-lock.json and only include changes from npm add @svta/common-media-library.

littlespex · 2023-10-17T23:27:25Z

Done

robwalch

0 differences in light build ✅
hls.min.js ~1 KB larger with this library (looks mostly like unminified enums and additional uuid generation fallback) 🆗

robwalch · 2023-10-18T21:34:10Z

@littlespex,

Is v1.6.0 OK for this change? Do you expect to make changes or need updates or changes included with the library sooner rather than later?

I'm assuming this change has no functional impact at this stage (no new params or output fixes other than maybe the UUID fallback).

v1.5.0 is still pre-beta, but a little overdue. I don't see this change as a risk, but it would be easier for me to push it to the following minor.

littlespex · 2023-10-18T21:35:13Z

No rush. This can wait until 1.6

…s.js into task/svta-cmcd

api-extractor/report/hls.js.api.md

src/controller/cmcd-controller.ts

tsconfig.json

Co-authored-by: Rob Walch <[email protected]>

tsconfig.json

cjpillsbury · 2023-11-16T21:56:13Z

build-config.js

@@ -96,7 +96,7 @@ const babelTsWithPresetEnvTargets = ({ targets, stripConsole }) =>
  babel({
    extensions,
    babelHelpers: 'bundled',
-    exclude: 'node_modules/**',
+    exclude: /node_modules\/(?!(@svta)\/).*/,


That is an unfortunate side effect of rollup's babel plugin config rules:

When relying on Babel configuration files you cannot include files already excluded there.

https://github.com/rollup/plugins/blob/2c58b01a45d31e0abd72c9c83a86c826c898cd24/packages/babel/types/index.d.ts#L8

Not excluding it ensures that babel does the es conversion correct? We have es-check verifying that the .js output is es5 w/o modules so I think we're good.

Correct. CML is targeted at ESNEXT and needs top be transpiled. It would just be clearer if we could "include CML" via the include config property instead of "exclude everything but CML".

cjpillsbury

LGTM! @littlespex we should probably move any CMCD version identifiers to common media library, since it effectively "owns" versioning information/compliance.

littlespex · 2023-11-16T22:15:21Z

@cjpillsbury What version identifiers? The client is responsible for reporting which version of the CMCD spec it adheres to, which is done here:

https://github.com/cbsinteractive/hls.js/blob/fe8579a1cf4abd7bfab139bec0300bdcb4fa2a2c/src/controller/cmcd-controller.ts#L135

Is there somewhere else you are referring to?

cjpillsbury · 2023-11-16T22:24:15Z

@cjpillsbury What version identifiers? The client is responsible for reporting which version of the CMCD spec it adheres to, which is done here:

https://github.com/cbsinteractive/hls.js/blob/fe8579a1cf4abd7bfab139bec0300bdcb4fa2a2c/src/controller/cmcd-controller.ts#L135

Is there somewhere else you are referring to?

Nope that's what I meant. I was just thinking that the value should be a const of some sort defined in CMCD.

@svta

* chore(deps): update dependency chromedriver to v118 (video-dev#5919) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency lint-staged to v15 (video-dev#5920) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update tjenkinson/gh-action-auto-merge-dependency-updates action to v1.3.5 (video-dev#5922) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency lint-staged to v15.0.1 * chore(deps): update dependency lint-staged to v15.0.2 * chore(deps): update dependency chromedriver to v118.0.1 * chore(deps): update dependency @rollup/plugin-replace to v5.0.4 * chore(deps): update dependency wrangler to v3.13.2 * chore(deps): update dependency wrangler to v3.14.0 * chore(deps): update dependency @types/chai to v4.3.9 * chore(deps): update dependency @rollup/plugin-commonjs to v25.0.7 * chore(deps): update typescript-eslint monorepo to v6.8.0 * chore(deps): update typescript-eslint monorepo to v6.9.0 * chore(deps): update dependency @types/mocha to v10.0.3 * chore(deps): update dependency @types/chart.js to v2.9.39 * chore(deps): update dependency @types/sinon-chai to v3.2.11 * chore(deps): update dependency sinon to v16.1.1 * chore(deps): update dependency eslint-plugin-import to v2.29.0 * chore(deps): update dependency sinon to v16.1.3 * chore(deps): update dependency eslint to v8.52.0 * chore(deps): update dependency wrangler to v3.15.0 * chore(deps): update dependency @rollup/plugin-replace to v5.0.5 * Add named exports for classes and enums to ESM output Resolves video-dev#5630 * chore(deps): update dependency @microsoft/api-documenter to v7.23.10 * chore(deps): update dependency @microsoft/api-extractor to v7.38.1 * chore(deps): update dependency @microsoft/api-documenter to v7.23.11 * chore(deps): update dependency @microsoft/api-extractor to v7.38.2 * chore(deps): update typescript-eslint monorepo to v6.9.1 * chore(deps): update typescript-eslint monorepo to v6.10.0 * chore(deps): update dependency eslint to v8.53.0 * Update README.md * Update README.md * Update README.md * chore(deps): update dependency @types/mocha to v10.0.4 * chore(deps): update dependency selenium-webdriver to v4.15.0 * chore(deps): update dependency @types/chart.js to v2.9.40 * chore(deps): update dependency @types/chai to v4.3.10 * chore(deps): update dependency @types/sinon-chai to v3.2.12 * Fix detach attach behavior dropping one of two SourceBuffers * Use Content Steering Pathways to manage Redundant Streams (video-dev#5970) * Use Content Steering Pathways to manage Redundant Streams and resolve their errors * Ensure correct Pathway penalization on Playlist loading errors * Do not reload Content Steering manifest while media is ended or detached * chore(deps): update babel monorepo to v7.23.3 * Remove use of `self` from `enableLogger` (video-dev#5936) Fixes video-dev#5905 * Refactor CMCD controller and tests to use the common media library utilities (video-dev#5903) * refactor CMCD controller and test to use common media library * update build script to transpile the @svta package * add ability to specify cmcd keys * chore(deps): update dependency lint-staged to v15.1.0 * chore(deps): update dependency @microsoft/api-extractor to v7.38.3 * chore(deps): update typescript-eslint monorepo to v6.11.0 * chore(deps): update typescript-eslint monorepo to v6.12.0 * chore(deps): update dependency @microsoft/api-documenter to v7.23.12 * Fix regression introduced with video-dev#5689 Lazy init CEA608 parsers found in video-dev#5953 (video-dev#5986) * Fix issues with long cea608 captions. video-dev#5952 In mp4-tools.ts * Fixed parsing for sei_message, by always consuming the entire message, before parsing the message according to payload_type * Fixed payloadType / payloadSize parsing to ensure they never exceed 255, as the field is restricted to 8 bytes. * chore(deps): update dependency node to v20 (video-dev#5928) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency sinon to v17 (video-dev#5944) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update actions/setup-node action to v4 (video-dev#5948) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency chromedriver to v119 [security] (video-dev#5965) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency prettier to v3.1.0 (video-dev#5983) * chore(deps): update dependency prettier to v3.1.0 * Run prettier --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Tom Jenkinson <[email protected]> * Configure typescript, eslint and prettier caches (video-dev#5990) * Rollup 4 (video-dev#5886) * Update karma-rollup-preprocessor to version that works in watch mode (video-dev#5991) * chore(deps): update dependency @svta/common-media-library to v0.5.1 * chore(deps): update dependency wrangler to v3.16.0 * chore(deps): update dependency wrangler to v3.17.1 * chore(deps): update dependency rollup to v4.5.2 * chore(deps): update dependency eslint to v8.54.0 * API enhancements for audio and subtitle selection Resolves video-dev#5532 * Remove note about "canplay" that references code removed from the example (related to autoplay policy) Closes video-dev#3153 * Fix issues parsing sei_messages In mp4-tools.ts * Fixed parsing for sei_message to remove the incorrect masking to 8 bits for the sei message size. * Remove SEI payload type masking * chore(deps): update dependency @types/chart.js to v2.9.41 * chore(deps): update dependency @types/chai to v4.3.11 * chore(deps): update dependency @types/mocha to v10.0.5 * chore(deps): update dependency @types/mocha to v10.0.6 * Expand isSupported check to test other codecs Resolves video-dev#6004 * Add `videoPreference` config option for HDR/SDR VIDEO-RANGE selection and priority Resolves video-dev#2489 * Recover from media error after MediaSource ended following SourceBuffer update error event * Fix exception on 2019 Tizen where MediaCapabilities is undefined * Add `isMSESupported` check Add named exports for and expose statically: `isSupported`, `isMSESupported`, and `getMediaSource` * chore(deps): update dependency @rollup/plugin-alias to v5.1.0 * chore(deps): update dependency rollup to v4.6.0 * chore(deps): update dependency rollup to v4.6.1 * chore(deps): update typescript-eslint monorepo to v6.13.0 * chore(deps): update typescript-eslint monorepo to v6.13.2 * chore(deps): update babel monorepo to v7.23.5 * Remove use of deprecated WebKitDataCue and hand Cue instantiation and custom property setting errors Fixes video-dev#6020 * Add polyfill for isSafeInteger * Fix esds box parsing for for usac audio * Update README Compatibility section * chore(deps): update dependency @svta/common-media-library to v0.6.0 * chore(deps): update dependency wrangler to v3.18.0 * chore(deps): update dependency wrangler to v3.19.0 * chore(deps): update dependency eslint to v8.55.0 * chore(deps): update dependency eslint-config-prettier to v9.1.0 * chore(deps): update dependency lint-staged to v15.2.0 * chore(deps): update dependency @microsoft/api-documenter to v7.23.13 * chore(deps): update dependency @microsoft/api-extractor to v7.38.4 * chore(deps): update dependency @microsoft/api-extractor to v7.38.5 * chore(deps): update dependency @microsoft/api-documenter to v7.23.14 * Use addEventListener for MediaKeySession events video-dev#6034 * chore(deps): update dependency selenium-webdriver to v4.16.0 * fix(latency-controller): only sync live stream * chore(deps): update actions/github-script action to v7 (video-dev#5996) * Ignore #EXT-X-INDEPENDENT-SEGMENTS so that it is not added to Fragment tagList * Fix handling of the DATERANGE END-ON-NEXT attribute * chore(deps): update dependency rollup to v4.7.0 * chore(deps): update dependency rollup to v4.9.0 * Store deployments in json file, and generate md and txt file from that (video-dev#6044) * Fix deployment branch update commit messages Just noticed this has been broken for a while * Fix path to deployment readme script * Fix path to script again * Add the final `/` at the end of deployment url * Remove tab at end of deployments readme Which is causing the list to be really spaced out for some reason * chore(deps): update dependency typescript to v5.3.3 (video-dev#5999) * chore(deps): update dependency typescript to v5.3.3 * Include api extractor changes --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Tom Jenkinson <[email protected]> * Exclude PS4 from TextDecoder use On PS4 TextDecoder is defined but it is partially implemented and does not function properly. This will force manual decoding approach for PS4 platform. * Ignore #EXT-X-INDEPENDENT-SEGMENTS (video-dev#6047) Fixes video-dev#6039 * chore(deps): update babel monorepo to v7.23.6 * chore(deps): update dependency prettier to v3.1.1 * chore(deps): update typescript-eslint monorepo to v6.14.0 * chore(deps): update typescript-eslint monorepo to v6.15.0 * chore(deps): update dependency wrangler to v3.20.0 * chore(deps): update dependency wrangler to v3.22.0 * Fix base-stream-controller onHandlerDestroying callback evocation Remove circular references left after destroying player * chore(deps): update dependency eslint-plugin-import to v2.29.1 * chore(deps): update dependency eslint to v8.56.0 * chore(deps): update dependency @svta/common-media-library to v0.6.1 * chore(deps): update dependency rollup to v4.9.1 * chore(deps): update dependency @microsoft/api-documenter to v7.23.15 * chore(deps): update dependency @microsoft/api-extractor to v7.39.0 * chore(deps): update dependency chromedriver to v120 (video-dev#6052) * chore(deps): update github/codeql-action action to v3 (video-dev#6058) * chore(deps): update dependency wrangler to v3.22.1 * Abort fetch loader as long as loading has not ended Fixes video-dev#6054 * chore(deps): update dependency chromedriver to v120.0.1 * chore(deps): update typescript-eslint monorepo to v6.16.0 * chore(deps): update typescript-eslint monorepo to v6.17.0 * chore(deps): update babel monorepo to v7.23.7 * chore(deps): update dependency rollup to v4.9.2 * chore(deps): update dependency rollup to v4.9.4 * Fix codec parsing for AVC streams (video-dev#6077) * Force auto level on emergency switch down (video-dev#6082) Update estimates on frag load timeout Do not abort request in _abandonRulesCheck Remove two segment forward buffer length limit in _abandonRulesCheck Reset estimate when candidate bitrate is lower than adjusted estimate Resolves video-dev#6079 * chore(deps): update dependency wrangler to v3.22.2 * chore(deps): update dependency wrangler to v3.22.4 * chore(deps): update dependency @microsoft/api-documenter to v7.23.16 * chore(deps): update dependency @microsoft/api-extractor to v7.39.1 * Null CMCD callbacks on destroy (video-dev#6098) * Fix regression where subtitle options with AUTOSELECT and FORCED are enabled at start (video-dev#6094) * Do not enable subtitle options with AUTOSELECT=YES attribute * Update and add initial selection tests for subtitle-controller * Only pick forced subtitle option if it is the only one Add default field to audio and subtitle selection options and forced field to subtitle selection option * Address TextTrack change event overriding subtitle preference Fix _TRACKS_UPDATED and _TRACK_SWITCH event order when preference is selected * Do not auto select subtitle options with FORCED=YES attribute * Update artifact actions (video-dev#6099) * Update functional tests to run on Safari using MacOS 13 (video-dev#6101) * Update functional tests to run on Safari using MacOS 13 * Skip smooth switch test in Safari on streams with overlapping appends * Omit VOD "ended" event tests with overlapping appends from Safari * chore(deps): update dependency chai to v4.4.0 * chore(deps): update dependency chai to v4.4.1 * chore(deps): update typescript-eslint monorepo to v6.18.0 * chore(deps): update typescript-eslint monorepo to v6.18.1 * Use AAC SBR (HE-AAC) workaround on Pale Moon (video-dev#6111) --------- Co-authored-by: hlsjs-ci <[email protected]> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Rob Walch <[email protected]> Co-authored-by: Rob Walch <[email protected]> Co-authored-by: Casey Occhialini <[email protected]> Co-authored-by: Joey Ekstrom <[email protected]> Co-authored-by: Tom Jenkinson <[email protected]> Co-authored-by: Tom Jenkinson <[email protected]> Co-authored-by: Evan Burton <[email protected]> Co-authored-by: 曾智锋 <[email protected]> Co-authored-by: Agajan Jumakuliyev <[email protected]> Co-authored-by: Jakub Perżyło <[email protected]> Co-authored-by: Pat Nafarrete <[email protected]>

littlespex added 4 commits October 12, 2023 22:40

refactor CMCD controller and test to use common media library

822da21

update build script to transpile the @svta package

648c0ac

fix lint errors

ae10e6b

fix lint errors

724c638

littlespex marked this pull request as ready for review October 13, 2023 16:30

littlespex added 3 commits October 16, 2023 22:42

Merge branch 'master' into task/svta-cmcd

239d6d7

no message

2be2f84

no message

073567a

robwalch requested changes Oct 17, 2023

View reviewed changes

littlespex added 2 commits October 17, 2023 16:25

no message

8ec64bc

Merge branch 'master' into task/svta-cmcd

c1c9303

robwalch self-requested a review October 18, 2023 21:07

robwalch previously approved these changes Oct 18, 2023

View reviewed changes

robwalch added this to the 1.6.0 milestone Oct 18, 2023

add ability to specify cmcd keys

b2d3924

littlespex dismissed robwalch’s stale review via b2d3924 October 23, 2023 19:56

littlespex added 3 commits October 23, 2023 13:04

Merge branch 'master' into task/svta-cmcd

39da823

update api report

25a579d

Merge branch 'task/svta-cmcd' of https://github.com/cbsinteractive/hl…

9280396

…s.js into task/svta-cmcd

littlespex mentioned this pull request Oct 27, 2023

Refactor hls.js's CMCD integration to use CML streaming-video-technology-alliance/common-media-library#37

Closed

littlespex commented Nov 15, 2023

View reviewed changes

api-extractor/report/hls.js.api.md Show resolved Hide resolved

Merge branch 'master' into task/svta-cmcd

aa01f47

robwalch reviewed Nov 15, 2023

View reviewed changes

src/controller/cmcd-controller.ts Outdated Show resolved Hide resolved

tsconfig.json Outdated Show resolved Hide resolved

robwalch modified the milestones: 1.6.0, 1.5.0 Nov 15, 2023

Update src/controller/cmcd-controller.ts

db568a5

Co-authored-by: Rob Walch <[email protected]>

littlespex commented Nov 15, 2023

View reviewed changes

tsconfig.json Outdated Show resolved Hide resolved

Update tsconfig.json

fe8579a

robwalch approved these changes Nov 16, 2023

View reviewed changes

cjpillsbury reviewed Nov 16, 2023

View reviewed changes

cjpillsbury approved these changes Nov 16, 2023

View reviewed changes

robwalch enabled auto-merge (squash) November 16, 2023 22:36

robwalch disabled auto-merge November 16, 2023 22:39

robwalch merged commit f943d31 into video-dev:master Nov 16, 2023
12 checks passed

robwalch added the CMCD label Apr 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor CMCD controller and tests to use the common media library utilities #5903

Refactor CMCD controller and tests to use the common media library utilities #5903

littlespex commented Oct 13, 2023 •

edited

Loading

robwalch left a comment

littlespex commented Oct 17, 2023

robwalch left a comment

robwalch commented Oct 18, 2023

littlespex commented Oct 18, 2023

cjpillsbury Nov 16, 2023

littlespex Nov 16, 2023

robwalch Nov 16, 2023

littlespex Nov 16, 2023

cjpillsbury left a comment

littlespex commented Nov 16, 2023 •

edited

Loading

cjpillsbury commented Nov 16, 2023

Refactor CMCD controller and tests to use the common media library utilities #5903

Refactor CMCD controller and tests to use the common media library utilities #5903

Conversation

littlespex commented Oct 13, 2023 • edited Loading

This PR will...

Why is this Pull Request needed?

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

Checklist

robwalch left a comment

Choose a reason for hiding this comment

littlespex commented Oct 17, 2023

robwalch left a comment

Choose a reason for hiding this comment

robwalch commented Oct 18, 2023

littlespex commented Oct 18, 2023

cjpillsbury Nov 16, 2023

Choose a reason for hiding this comment

littlespex Nov 16, 2023

Choose a reason for hiding this comment

robwalch Nov 16, 2023

Choose a reason for hiding this comment

littlespex Nov 16, 2023

Choose a reason for hiding this comment

cjpillsbury left a comment

Choose a reason for hiding this comment

littlespex commented Nov 16, 2023 • edited Loading

cjpillsbury commented Nov 16, 2023

littlespex commented Oct 13, 2023 •

edited

Loading

littlespex commented Nov 16, 2023 •

edited

Loading