Skip to content

Commit

Permalink
fix(cast): Reduce size of Cast update messages (#4644)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
joeyparrish committed Nov 8, 2022
1 parent d927cec commit 5e0e942
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 41 deletions.
3 changes: 2 additions & 1 deletion lib/cast/cast_proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,8 @@ shaka.cast.CastProxy = class extends shaka.util.FakeEventTarget {
// If we are casting, but the first update has not come in yet, use local
// getters, but not local methods.
if (this.sender_.isCasting() && !this.sender_.hasRemoteProperties()) {
if (shaka.cast.CastUtils.PlayerGetterMethods[name]) {
if (shaka.cast.CastUtils.PlayerGetterMethods[name] ||
shaka.cast.CastUtils.LargePlayerGetterMethods[name]) {
const value = /** @type {Object} */(this.localPlayer_)[name];
goog.asserts.assert(typeof value == 'function',
'only methods on Player');
Expand Down
24 changes: 19 additions & 5 deletions lib/cast/cast_receiver.js
Original file line number Diff line number Diff line change
Expand Up @@ -518,18 +518,32 @@ shaka.cast.CastReceiver = class extends shaka.util.FakeEventTarget {
update['video']['muted'] = systemVolume.muted;
}

this.sendMessage_({
'type': 'update',
'update': update,
}, this.shakaBus_);

// Getters with large outputs each get sent in their own update message.
for (const name in shaka.cast.CastUtils.LargePlayerGetterMethods) {
const frequency = shaka.cast.CastUtils.LargePlayerGetterMethods[name];
if (this.updateNumber_ % frequency == 0) {
const update = {'player': {}};
update['player'][name] = /** @type {Object} */ (this.player_)[name]();

this.sendMessage_({
'type': 'update',
'update': update,
}, this.shakaBus_);
}
}

// Only start progressing the update number once data is loaded,
// just in case any of the "rarely changing" properties with less frequent
// update messages changes significantly during the loading process.
if (this.startUpdatingUpdateNumber_) {
this.updateNumber_ += 1;
}

this.sendMessage_({
'type': 'update',
'update': update,
}, this.shakaBus_);

this.maybeSendMediaInfoMessage_();
}

Expand Down
3 changes: 2 additions & 1 deletion lib/cast/cast_sender.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ shaka.cast.CastSender = class {
return (...args) =>
this.remoteAsyncCall_(targetName, property, ...args);
}
if (CastUtils.PlayerGetterMethods[property]) {
if (CastUtils.PlayerGetterMethods[property] ||
CastUtils.LargePlayerGetterMethods[property]) {
return () => this.propertyGetter_(targetName, property);
}
}
Expand Down
22 changes: 16 additions & 6 deletions lib/cast/cast_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,6 @@ shaka.cast.CastUtils.PlayerGetterMethods = {
'getAudioLanguagesAndRoles': 4,
'getBufferFullness': 1,
'getBufferedInfo': 2,
// NOTE: The 'getSharedConfiguration' property is not proxied as it would
// not be possible to share a reference.
'getConfiguration': 4,
'getExpiration': 2,
'getKeyStatuses': 2,
// NOTE: The 'getManifest' property is not proxied, as it is very large.
Expand All @@ -318,9 +315,6 @@ shaka.cast.CastUtils.PlayerGetterMethods = {
'getPlaybackRate': 2,
'getTextLanguages': 4,
'getTextLanguagesAndRoles': 4,
'getTextTracks': 2,
'getStats': 5,
'getVariantTracks': 2,
'getImageTracks': 2,
'getThumbnails': 2,
'isAudioOnly': 10,
Expand All @@ -334,6 +328,22 @@ shaka.cast.CastUtils.PlayerGetterMethods = {
};


/**
* Player getter methods with data large enough to be sent in their own update
* messages, to reduce the size of each message. The format of this is
* identical to PlayerGetterMethods.
* @const {!Object.<string, number>}
*/
shaka.cast.CastUtils.LargePlayerGetterMethods = {
// NOTE: The 'getSharedConfiguration' property is not proxied as it would
// not be possible to share a reference.
'getConfiguration': 4,
'getStats': 5,
'getTextTracks': 2,
'getVariantTracks': 2,
};


/**
* Player getter methods that are proxied while casting, but only when casting
* a livestream.
Expand Down
113 changes: 97 additions & 16 deletions test/cast/cast_receiver_integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {

/** @type {shaka.util.PublicPromise} */
let messageWaitPromise;
/** @type {Array.<string>} */
let pendingMessages = null;

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

messageWaitPromise = null;
pendingMessages = null;

fakeInitState = {
player: {
configure: {},
Expand Down Expand Up @@ -118,6 +123,12 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
player = null;
video = null;
receiver = null;

if (messageWaitPromise) {
messageWaitPromise.resolve([]);
}
messageWaitPromise = null;
pendingMessages = null;
});

afterAll(() => {
Expand All @@ -128,6 +139,66 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
}
});

describe('without drm', () => {
it('sends reasonably-sized updates', async () => {
// Use an unencrypted asset.
fakeInitState.manifest = 'test:sintel';

const p = waitForLoadedData();

// Start the process of loading by sending a fake init message.
fakeConnectedSenders(1);
fakeIncomingMessage({
type: 'init',
initState: fakeInitState,
appData: {},
}, mockShakaMessageBus);

await p;
// Wait for an update message.
const messages = await waitForUpdateMessages();
for (const message of messages) {
// Check that the update message is of a reasonable size. From previous
// testing we found that the socket would silently reject data that got
// too big. 6KB is safely below the limit.
expect(message.length).toBeLessThan(6000);
}
});

it('has reasonable average message size', async () => {
// Use an unencrypted asset.
fakeInitState.manifest = 'test:sintel';

const p = waitForLoadedData();

// Start the process of loading by sending a fake init message.
fakeConnectedSenders(1);
fakeIncomingMessage({
type: 'init',
initState: fakeInitState,
appData: {},
}, mockShakaMessageBus);

await p;

// Collect messages over 50 update cycles, and average their length.
// Not all properties are passed along on every update message, so
// the average length is expected to be lower than the length of the first
// update message.
let totalLength = 0;
let totalMessages = 0;
for (let i = 0; i < 50; i++) {
// eslint-disable-next-line no-await-in-loop
const messages = await waitForUpdateMessages();
for (const message of messages) {
totalLength += message.length;
totalMessages += 1;
}
}
expect(totalLength / totalMessages).toBeLessThan(2000);
});
});

filterDescribe('with drm', () => support['com.widevine.alpha'], () => {
drmIt('sends reasonably-sized updates', async () => {
// Use an encrypted asset, to make sure DRM info doesn't balloon the size.
Expand All @@ -150,11 +221,13 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {

await p;
// Wait for an update message.
const message = await waitForUpdateMessage();
// Check that the update message is of a reasonable size. From previous
// testing we found that the socket would silently reject data that got
// too big. 6KB is safely below the limit.
expect(message.length).toBeLessThan(7 * 1024);
const messages = await waitForUpdateMessages();
for (const message of messages) {
// Check that the update message is of a reasonable size. From previous
// testing we found that the socket would silently reject data that got
// too big. 6KB is safely below the limit.
expect(message.length).toBeLessThan(6000);
}
});

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

await p;
// Collect 50 update messages, and average their length.

// Collect messages over 50 update cycles, and average their length.
// Not all properties are passed along on every update message, so
// the average length is expected to be lower than the length of the first
// update message.
let totalLength = 0;
let totalMessages = 0;
for (let i = 0; i < 50; i++) {
// eslint-disable-next-line no-await-in-loop
const message = await waitForUpdateMessage();
totalLength += message.length;
const messages = await waitForUpdateMessages();
for (const message of messages) {
totalLength += message.length;
totalMessages += 1;
}
}
expect(totalLength / 50).toBeLessThan(3000);
expect(totalLength / totalMessages).toBeLessThan(2000);
});
});

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

// Wait for a final update message before proceeding.
await waitForUpdateMessage();
await waitForUpdateMessages();
});

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

function waitForUpdateMessage() {
function waitForUpdateMessages() {
pendingMessages = [];
messageWaitPromise = new shaka.util.PublicPromise();
return messageWaitPromise;
}
Expand Down Expand Up @@ -316,10 +395,12 @@ filterDescribe('CastReceiver', castReceiverIntegrationSupport, () => {
bus.messages.push(CastUtils.deserialize(message));
// Check to see if it's an update message.
const parsed = CastUtils.deserialize(message);
if (parsed.type == 'update' && messageWaitPromise) {
if (parsed.type == 'update' && pendingMessages) {
shaka.log.debug('Received update message. Proceeding...');
messageWaitPromise.resolve(message);
messageWaitPromise = null;
// The code waiting on this Promise will get an array of all of the
// messages processed within this tick.
pendingMessages.push(message);
messageWaitPromise.resolve(pendingMessages);
}
});
const channel = {
Expand Down
29 changes: 17 additions & 12 deletions test/cast/cast_receiver_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,18 +245,20 @@ filterDescribe('CastReceiver', castReceiverSupport, () => {
const fakeEvent = {type: 'timeupdate'};
mockVideo.on['timeupdate'](fakeEvent);

// There are now "update" and "event" messages, in that order.
expect(mockShakaMessageBus.messages).toEqual([
{
type: 'update',
update: jasmine.any(Object),
},
{
type: 'event',
targetName: 'video',
event: jasmine.objectContaining(fakeEvent),
},
]);
// There are now some number of "update" and "event" messages, in that
// order.
expect(mockShakaMessageBus.messages).toContain({
type: 'update',
update: jasmine.any(Object),
});
expect(mockShakaMessageBus.messages).toContain({
type: 'event',
targetName: 'video',
event: jasmine.objectContaining(fakeEvent),
});
const eventIndex = mockShakaMessageBus.messages.findIndex(
(message) => message.type == 'event');
expect(eventIndex).toBe(mockShakaMessageBus.messages.length - 1);
});
});

Expand Down Expand Up @@ -1105,6 +1107,9 @@ filterDescribe('CastReceiver', castReceiverSupport, () => {
for (const name in CastUtils.PlayerGetterMethods) {
player[name] = jasmine.createSpy(name);
}
for (const name in CastUtils.LargePlayerGetterMethods) {
player[name] = jasmine.createSpy(name);
}
for (const name in CastUtils.PlayerGetterMethodsThatRequireLive) {
player[name] = jasmine.createSpy(name);
}
Expand Down
1 change: 1 addition & 0 deletions test/cast/cast_utils_unit.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('CastUtils', () => {
const castMembers = CastUtils.PlayerVoidMethods
.concat(CastUtils.PlayerPromiseMethods)
.concat(Object.keys(CastUtils.PlayerGetterMethods))
.concat(Object.keys(CastUtils.LargePlayerGetterMethods))
.concat(Object.keys(CastUtils.PlayerGetterMethodsThatRequireLive));
// eslint-disable-next-line no-restricted-syntax
const allPlayerMembers = Object.getOwnPropertyNames(shaka.Player.prototype);
Expand Down

0 comments on commit 5e0e942

Please sign in to comment.