Skip to content

Commit 4e75ec6

Browse files
authored
fix(cast): Reduce size of Cast update messages (#4644)
Our Cast API sends update messages from receiver to sender, and we have observed before that there is a hidden limit on the size of those messages. A test was written long ago to catch unexpected increases in message sizes, and it has recently started failing. Our original limit on individual messages was set to 6kB, but later raised to 7kB to silence test failures. Our original goal was to keep messages well under 8kB. We also had a limit on the average message size of 3kB (over 50 messages). This change greatly reduces the sizes of individual messages by splitting out updates to certain getters into their own messages. These are the getters that produce the most data: getConfiguration(), getStats(), getVariantTracks(), and getTextTracks(). In testing, getConfiguration() alone is nearly 4kB with defaults, so this is a signficant chunk of the test limit of 7kB. With this change, the max message size seen in tests was reduced from ~7kB to ~4kB, and the average message size was reduced from ~2kB to ~1kB. With this, we are lowering the thresholds in tests back to 6kB (max) and 2kB (average). This also adds new versions of these message size tests for clear content. Although DRM content will generate larger messages, I had to do some of the work on this change while my internet connection was out, and I found it very useful to be able to run a version of these tests that did not require an internet connection (for a DRM license).
1 parent 216bdd7 commit 4e75ec6

File tree

7 files changed

+154
-41
lines changed

7 files changed

+154
-41
lines changed

lib/cast/cast_proxy.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,8 @@ shaka.cast.CastProxy = class extends shaka.util.FakeEventTarget {
705705
// If we are casting, but the first update has not come in yet, use local
706706
// getters, but not local methods.
707707
if (this.sender_.isCasting() && !this.sender_.hasRemoteProperties()) {
708-
if (shaka.cast.CastUtils.PlayerGetterMethods[name]) {
708+
if (shaka.cast.CastUtils.PlayerGetterMethods[name] ||
709+
shaka.cast.CastUtils.LargePlayerGetterMethods[name]) {
709710
const value = /** @type {Object} */(this.localPlayer_)[name];
710711
goog.asserts.assert(typeof value == 'function',
711712
'only methods on Player');

lib/cast/cast_receiver.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -518,18 +518,32 @@ shaka.cast.CastReceiver = class extends shaka.util.FakeEventTarget {
518518
update['video']['muted'] = systemVolume.muted;
519519
}
520520

521+
this.sendMessage_({
522+
'type': 'update',
523+
'update': update,
524+
}, this.shakaBus_);
525+
526+
// Getters with large outputs each get sent in their own update message.
527+
for (const name in shaka.cast.CastUtils.LargePlayerGetterMethods) {
528+
const frequency = shaka.cast.CastUtils.LargePlayerGetterMethods[name];
529+
if (this.updateNumber_ % frequency == 0) {
530+
const update = {'player': {}};
531+
update['player'][name] = /** @type {Object} */ (this.player_)[name]();
532+
533+
this.sendMessage_({
534+
'type': 'update',
535+
'update': update,
536+
}, this.shakaBus_);
537+
}
538+
}
539+
521540
// Only start progressing the update number once data is loaded,
522541
// just in case any of the "rarely changing" properties with less frequent
523542
// update messages changes significantly during the loading process.
524543
if (this.startUpdatingUpdateNumber_) {
525544
this.updateNumber_ += 1;
526545
}
527546

528-
this.sendMessage_({
529-
'type': 'update',
530-
'update': update,
531-
}, this.shakaBus_);
532-
533547
this.maybeSendMediaInfoMessage_();
534548
}
535549

lib/cast/cast_sender.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ shaka.cast.CastSender = class {
370370
return (...args) =>
371371
this.remoteAsyncCall_(targetName, property, ...args);
372372
}
373-
if (CastUtils.PlayerGetterMethods[property]) {
373+
if (CastUtils.PlayerGetterMethods[property] ||
374+
CastUtils.LargePlayerGetterMethods[property]) {
374375
return () => this.propertyGetter_(targetName, property);
375376
}
376377
}

lib/cast/cast_utils.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,6 @@ shaka.cast.CastUtils.PlayerGetterMethods = {
307307
'getAudioLanguagesAndRoles': 4,
308308
'getBufferFullness': 1,
309309
'getBufferedInfo': 2,
310-
// NOTE: The 'getSharedConfiguration' property is not proxied as it would
311-
// not be possible to share a reference.
312-
'getConfiguration': 4,
313310
'getExpiration': 2,
314311
'getKeyStatuses': 2,
315312
// NOTE: The 'getManifest' property is not proxied, as it is very large.
@@ -318,9 +315,6 @@ shaka.cast.CastUtils.PlayerGetterMethods = {
318315
'getPlaybackRate': 2,
319316
'getTextLanguages': 4,
320317
'getTextLanguagesAndRoles': 4,
321-
'getTextTracks': 2,
322-
'getStats': 5,
323-
'getVariantTracks': 2,
324318
'getImageTracks': 2,
325319
'getThumbnails': 2,
326320
'isAudioOnly': 10,
@@ -334,6 +328,22 @@ shaka.cast.CastUtils.PlayerGetterMethods = {
334328
};
335329

336330

331+
/**
332+
* Player getter methods with data large enough to be sent in their own update
333+
* messages, to reduce the size of each message. The format of this is
334+
* identical to PlayerGetterMethods.
335+
* @const {!Object.<string, number>}
336+
*/
337+
shaka.cast.CastUtils.LargePlayerGetterMethods = {
338+
// NOTE: The 'getSharedConfiguration' property is not proxied as it would
339+
// not be possible to share a reference.
340+
'getConfiguration': 4,
341+
'getStats': 5,
342+
'getTextTracks': 2,
343+
'getVariantTracks': 2,
344+
};
345+
346+
337347
/**
338348
* Player getter methods that are proxied while casting, but only when casting
339349
* a livestream.

test/cast/cast_receiver_integration.js

Lines changed: 97 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
3434

3535
/** @type {shaka.util.PublicPromise} */
3636
let messageWaitPromise;
37+
/** @type {Array.<string>} */
38+
let pendingMessages = null;
3739

3840
/** @type {!Array.<function()>} */
3941
let toRestore;
@@ -91,6 +93,9 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
9193
toRestore = [];
9294
pendingWaitWrapperCalls = 0;
9395

96+
messageWaitPromise = null;
97+
pendingMessages = null;
98+
9499
fakeInitState = {
95100
player: {
96101
configure: {},
@@ -118,6 +123,12 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
118123
player = null;
119124
video = null;
120125
receiver = null;
126+
127+
if (messageWaitPromise) {
128+
messageWaitPromise.resolve([]);
129+
}
130+
messageWaitPromise = null;
131+
pendingMessages = null;
121132
});
122133

123134
afterAll(() => {
@@ -128,6 +139,66 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
128139
}
129140
});
130141

142+
describe('without drm', () => {
143+
it('sends reasonably-sized updates', async () => {
144+
// Use an unencrypted asset.
145+
fakeInitState.manifest = 'test:sintel';
146+
147+
const p = waitForLoadedData();
148+
149+
// Start the process of loading by sending a fake init message.
150+
fakeConnectedSenders(1);
151+
fakeIncomingMessage({
152+
type: 'init',
153+
initState: fakeInitState,
154+
appData: {},
155+
}, mockShakaMessageBus);
156+
157+
await p;
158+
// Wait for an update message.
159+
const messages = await waitForUpdateMessages();
160+
for (const message of messages) {
161+
// Check that the update message is of a reasonable size. From previous
162+
// testing we found that the socket would silently reject data that got
163+
// too big. 6KB is safely below the limit.
164+
expect(message.length).toBeLessThan(6000);
165+
}
166+
});
167+
168+
it('has reasonable average message size', async () => {
169+
// Use an unencrypted asset.
170+
fakeInitState.manifest = 'test:sintel';
171+
172+
const p = waitForLoadedData();
173+
174+
// Start the process of loading by sending a fake init message.
175+
fakeConnectedSenders(1);
176+
fakeIncomingMessage({
177+
type: 'init',
178+
initState: fakeInitState,
179+
appData: {},
180+
}, mockShakaMessageBus);
181+
182+
await p;
183+
184+
// Collect messages over 50 update cycles, and average their length.
185+
// Not all properties are passed along on every update message, so
186+
// the average length is expected to be lower than the length of the first
187+
// update message.
188+
let totalLength = 0;
189+
let totalMessages = 0;
190+
for (let i = 0; i < 50; i++) {
191+
// eslint-disable-next-line no-await-in-loop
192+
const messages = await waitForUpdateMessages();
193+
for (const message of messages) {
194+
totalLength += message.length;
195+
totalMessages += 1;
196+
}
197+
}
198+
expect(totalLength / totalMessages).toBeLessThan(2000);
199+
});
200+
});
201+
131202
filterDescribe('with drm', () => support['com.widevine.alpha'], () => {
132203
drmIt('sends reasonably-sized updates', async () => {
133204
// Use an encrypted asset, to make sure DRM info doesn't balloon the size.
@@ -150,11 +221,13 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
150221

151222
await p;
152223
// Wait for an update message.
153-
const message = await waitForUpdateMessage();
154-
// Check that the update message is of a reasonable size. From previous
155-
// testing we found that the socket would silently reject data that got
156-
// too big. 6KB is safely below the limit.
157-
expect(message.length).toBeLessThan(7 * 1024);
224+
const messages = await waitForUpdateMessages();
225+
for (const message of messages) {
226+
// Check that the update message is of a reasonable size. From previous
227+
// testing we found that the socket would silently reject data that got
228+
// too big. 6KB is safely below the limit.
229+
expect(message.length).toBeLessThan(6000);
230+
}
158231
});
159232

160233
drmIt('has reasonable average message size', async () => {
@@ -177,17 +250,22 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
177250
}, mockShakaMessageBus);
178251

179252
await p;
180-
// Collect 50 update messages, and average their length.
253+
254+
// Collect messages over 50 update cycles, and average their length.
181255
// Not all properties are passed along on every update message, so
182256
// the average length is expected to be lower than the length of the first
183257
// update message.
184258
let totalLength = 0;
259+
let totalMessages = 0;
185260
for (let i = 0; i < 50; i++) {
186261
// eslint-disable-next-line no-await-in-loop
187-
const message = await waitForUpdateMessage();
188-
totalLength += message.length;
262+
const messages = await waitForUpdateMessages();
263+
for (const message of messages) {
264+
totalLength += message.length;
265+
totalMessages += 1;
266+
}
189267
}
190-
expect(totalLength / 50).toBeLessThan(3000);
268+
expect(totalLength / totalMessages).toBeLessThan(2000);
191269
});
192270
});
193271

@@ -227,12 +305,12 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
227305
expect(pendingWaitWrapperCalls).toBe(0);
228306

229307
// Wait for a final update message before proceeding.
230-
await waitForUpdateMessage();
308+
await waitForUpdateMessages();
231309
});
232310

233311
/**
234312
* Creates a wrapper around a method on a given prototype, which makes it
235-
* wait on waitForUpdateMessage before returning, and registers that wrapper
313+
* wait on waitForUpdateMessages before returning, and registers that wrapper
236314
* to be uninstalled afterwards.
237315
* The replaced method is expected to be a method that returns a promise.
238316
* @param {!Object} prototype
@@ -249,7 +327,7 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
249327
'Waiting for update message before calling ' +
250328
name + '.' + methodName + '...');
251329
const originalArguments = Array.from(arguments);
252-
await waitForUpdateMessage();
330+
await waitForUpdateMessages();
253331
// eslint-disable-next-line no-restricted-syntax
254332
return original.apply(this, originalArguments);
255333
};
@@ -265,7 +343,8 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
265343
});
266344
}
267345

268-
function waitForUpdateMessage() {
346+
function waitForUpdateMessages() {
347+
pendingMessages = [];
269348
messageWaitPromise = new shaka.util.PublicPromise();
270349
return messageWaitPromise;
271350
}
@@ -316,10 +395,12 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
316395
bus.messages.push(CastUtils.deserialize(message));
317396
// Check to see if it's an update message.
318397
const parsed = CastUtils.deserialize(message);
319-
if (parsed.type == 'update' && messageWaitPromise) {
398+
if (parsed.type == 'update' && pendingMessages) {
320399
shaka.log.debug('Received update message. Proceeding...');
321-
messageWaitPromise.resolve(message);
322-
messageWaitPromise = null;
400+
// The code waiting on this Promise will get an array of all of the
401+
// messages processed within this tick.
402+
pendingMessages.push(message);
403+
messageWaitPromise.resolve(pendingMessages);
323404
}
324405
});
325406
const channel = {

test/cast/cast_receiver_unit.js

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -245,18 +245,20 @@ filterDescribe('CastReceiver', castReceiverSupport, () => {
245245
const fakeEvent = {type: 'timeupdate'};
246246
mockVideo.on['timeupdate'](fakeEvent);
247247

248-
// There are now "update" and "event" messages, in that order.
249-
expect(mockShakaMessageBus.messages).toEqual([
250-
{
251-
type: 'update',
252-
update: jasmine.any(Object),
253-
},
254-
{
255-
type: 'event',
256-
targetName: 'video',
257-
event: jasmine.objectContaining(fakeEvent),
258-
},
259-
]);
248+
// There are now some number of "update" and "event" messages, in that
249+
// order.
250+
expect(mockShakaMessageBus.messages).toContain({
251+
type: 'update',
252+
update: jasmine.any(Object),
253+
});
254+
expect(mockShakaMessageBus.messages).toContain({
255+
type: 'event',
256+
targetName: 'video',
257+
event: jasmine.objectContaining(fakeEvent),
258+
});
259+
const eventIndex = mockShakaMessageBus.messages.findIndex(
260+
(message) => message.type == 'event');
261+
expect(eventIndex).toBe(mockShakaMessageBus.messages.length - 1);
260262
});
261263
});
262264

@@ -1105,6 +1107,9 @@ filterDescribe('CastReceiver', castReceiverSupport, () => {
11051107
for (const name in CastUtils.PlayerGetterMethods) {
11061108
player[name] = jasmine.createSpy(name);
11071109
}
1110+
for (const name in CastUtils.LargePlayerGetterMethods) {
1111+
player[name] = jasmine.createSpy(name);
1112+
}
11081113
for (const name in CastUtils.PlayerGetterMethodsThatRequireLive) {
11091114
player[name] = jasmine.createSpy(name);
11101115
}

test/cast/cast_utils_unit.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ describe('CastUtils', () => {
3434
const castMembers = CastUtils.PlayerVoidMethods
3535
.concat(CastUtils.PlayerPromiseMethods)
3636
.concat(Object.keys(CastUtils.PlayerGetterMethods))
37+
.concat(Object.keys(CastUtils.LargePlayerGetterMethods))
3738
.concat(Object.keys(CastUtils.PlayerGetterMethodsThatRequireLive));
3839
// eslint-disable-next-line no-restricted-syntax
3940
const allPlayerMembers = Object.getOwnPropertyNames(shaka.Player.prototype);

0 commit comments

Comments
 (0)