fix(cast): Reduce size of Cast update messages #4644
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
|
Incremental code coverage: 100.00% |
avelad
added
type: bug
avelad
approved these changes
Nov 2, 2022
joeyparrish
added a commit
that referenced
this pull request
Nov 8, 2022
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).
joeyparrish
added a commit
that referenced
this pull request
Nov 8, 2022
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).
joeyparrish
added a commit
that referenced
this pull request
Nov 8, 2022
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).
joeyparrish
added a commit
that referenced
this pull request
Nov 8, 2022
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).
github-actions
bot
added
the
status: archived
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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).