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

fix: Manually order key for decodingInfo cache #4795

Merged
merged 2 commits into from
Dec 7, 2022
+44 −6
Merged

fix: Manually order key for decodingInfo cache #4795

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b446633
fix: Manually order key for decodingInfo cache
theodab Dec 6, 2022
8805169
fix: Even better method for generating keys.
theodab Dec 7, 2022
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
43 changes: 39 additions & 4 deletions lib/util/stream_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,10 +510,45 @@ shaka.util.StreamUtils = class {
* @private
*/
static async getDecodingInfosForVariant_(variant, decodingConfig) {
// Construct a key for that decoding config. By using this instead of plain
Copy link
Member

Choose a reason for hiding this comment

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

I don't love that this is brittle against future changes to MediaCapabilities, and the failure mode is worse now. We would have to add new fields here or else end up with potentially bogus results (where cache hits ignore certain fields).

We can defer this part, IMO. Iteration order is spec'd as the order the fields were created in, and that should hold well enough for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I just thought of a better approach for this that should be less brittle. Let me give it a shot...

// JSON.stringify, we ensure that the keys are always in a consistent order.
const cacheKeyAr = [decodingConfig.type];
if (decodingConfig.video) {
cacheKeyAr.push(decodingConfig.video.contentType);
cacheKeyAr.push(decodingConfig.video.width);
cacheKeyAr.push(decodingConfig.video.height);
cacheKeyAr.push(decodingConfig.video.bitrate);
cacheKeyAr.push(decodingConfig.video.framerate);
cacheKeyAr.push(decodingConfig.video.transferFunction);
}
if (decodingConfig.audio) {
cacheKeyAr.push(decodingConfig.audio.contentType);
cacheKeyAr.push(decodingConfig.audio.channels);
cacheKeyAr.push(decodingConfig.audio.bitrate);
cacheKeyAr.push(decodingConfig.audio.samplerate);
cacheKeyAr.push(decodingConfig.audio.spatialRendering);
}
if (decodingConfig.keySystemConfiguration) {
const keyConfig = decodingConfig.keySystemConfiguration;
cacheKeyAr.push(keyConfig.keySystem);
cacheKeyAr.push(keyConfig.initDataType);
cacheKeyAr.push(keyConfig.persistentState);
cacheKeyAr.push(keyConfig.distinctiveIdentifier);
cacheKeyAr.push(keyConfig.sessionTypes.join(','));
if (keyConfig.audio) {
cacheKeyAr.push(keyConfig.audio.robustness);
}
if (keyConfig.video) {
cacheKeyAr.push(keyConfig.video.robustness);
}
}
const cacheKey = cacheKeyAr.map((term) => `{${term}}`).join('');

try {
const cacheKey = JSON.stringify(decodingConfig);
const cache = shaka.util.StreamUtils.decodingConfigCache;
const cache = shaka.util.StreamUtils.decodingConfigCache_;
if (cache[cacheKey]) {
shaka.log.v2('Using cached results of mediaCapabilities.decodingInfo',
'for key', cacheKey);
variant.decodingInfos.push(cache[cacheKey]);
} else {
const result =
Expand Down Expand Up @@ -1601,9 +1636,9 @@ shaka.util.StreamUtils = class {
* (stringified) decodingConfig.
*
* @type {Object.<(!string), (!MediaCapabilitiesDecodingInfo)>}
* @export
* @private
*/
shaka.util.StreamUtils.decodingConfigCache = {};
shaka.util.StreamUtils.decodingConfigCache_ = {};


/** @private {number} */
Expand Down
4 changes: 2 additions & 2 deletions test/test/boot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,8 @@ function configureJasmineEnvironment() {
}

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

// Code in karma-jasmine's adapter will malform test failures when the
Expand Down