Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Cache mediaCapabilities.decodingInfo results #4789

Merged
merged 1 commit into from
Dec 6, 2022
Merged

feat: Cache mediaCapabilities.decodingInfo results #4789

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions lib/util/stream_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,33 @@ shaka.util.StreamUtils = class {
}


/**
* Queries mediaCapabilities for the decoding info for that decoding config,
* and assigns it to the given variant.
* If that query has been done before, instead return a cached result.
* @param {!shaka.extern.Variant} variant
* @param {!MediaDecodingConfiguration} decodingConfig
* @private
*/
static async getDecodingInfosForVariant_(variant, decodingConfig) {
try {
const cacheKey = JSON.stringify(decodingConfig);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a problematic key. If the fields are serialized in a different order, this will fail to match.

It's better than no caching, but this needs work. Please file an issue to follow-up on this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(There's an entire ToTT episode on this subject, BTW, in context of comparing objects in tests using JSON.stringify. But it still applies here.)

const cache = shaka.util.StreamUtils.decodingConfigCache;
if (cache[cacheKey]) {
variant.decodingInfos.push(cache[cacheKey]);
} else {
const result =
await navigator.mediaCapabilities.decodingInfo(decodingConfig);
cache[cacheKey] = result;
variant.decodingInfos.push(result);
}
} catch (e) {
shaka.log.info('MediaCapabilities.decodingInfo() failed.',
JSON.stringify(decodingConfig), e);
}
}


/**
* Get the decodingInfo results of the variants via MediaCapabilities.
* This should be called after the DrmEngine is created and configured, and
Expand All @@ -518,18 +545,6 @@ shaka.util.StreamUtils = class {
return;
}

const mediaCapabilities = navigator.mediaCapabilities;

const getVariantDecodingInfos = (async (variant, decodingConfig) => {
try {
const result = await mediaCapabilities.decodingInfo(decodingConfig);
variant.decodingInfos.push(result);
} catch (e) {
shaka.log.info('MediaCapabilities.decodingInfo() failed.',
JSON.stringify(decodingConfig), e);
}
});

for (const variant of variants) {
/** @type {!Array.<!MediaDecodingConfiguration>} */
const decodingConfigs = shaka.util.StreamUtils.getDecodingConfigs_(
Expand All @@ -540,7 +555,8 @@ shaka.util.StreamUtils = class {
// https://github.com/shaka-project/shaka-player/pull/4708#discussion_r1022581178
for (const config of decodingConfigs) {
// eslint-disable-next-line no-await-in-loop
await getVariantDecodingInfos(variant, config);
await shaka.util.StreamUtils.getDecodingInfosForVariant_(
variant, config);
}
}
}
Expand Down Expand Up @@ -1578,6 +1594,16 @@ shaka.util.StreamUtils = class {
};


/**
* A cache of results from mediaCapabilities.decodingInfo, indexed by the
* (stringified) decodingConfig.
*
* @type {Object.<(!string), (!MediaCapabilitiesDecodingInfo)>}
* @export
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this exported? I don't think we want this exposed to the application and subject to backward compatibility guarantees.

If you just need it for unit testing, use @suppress in a test helper instead.

*/
shaka.util.StreamUtils.decodingConfigCache = {};


/** @private {number} */
shaka.util.StreamUtils.nextTrackId_ = 0;

Expand Down
5 changes: 5 additions & 0 deletions test/test/boot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ function configureJasmineEnvironment() {
});
}

// Reset decoding config cache after each test.
afterEach(() => {
shaka.util.StreamUtils.decodingConfigCache = {};
});

// Code in karma-jasmine's adapter will malform test failures when the
// expectation message contains a stack trace, losing the failure message and
// mixing up the stack trace of the failure. To avoid this, we modify
Expand Down