feat: export period combiner #5324
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
baconz
force-pushed
the
export_period_combiner
branch
from
5d2b59c to
63533da
Compare
baconz
changed the title
|
Incremental code coverage: 100.00% |
|
@baconz, long time, no see! Thanks for the PR. Looks good to me. |
|
JFYI, it has been proposed to trash this class in v5 and go back to modeling periods explicitly in the manifest structure. We haven't committed to it, but it is a breaking change we're considering in the next ~12 months. |
|
Thanks for getting this out so quickly! Interesting about going back to the old manifest format. What is the reasoning for switching back? |
|
Primarily that period combiner is complicated and uses more memory than other approaches might. |
When we exported `PeriodCombiner` in #5324 we added an `@export` to `PeriodCombiner.Period`, and since then we've been testing in our dogfood builds using shaka-player.compiled.debug, because we like to get logs from dogfood. Everything was working great. When we went to switch over to the production build, we realized that `@export` on a `typedef` doesn't really work because the type gets minified internally!! This moves `Period` over to `extern` so that it does not get minified and can be used externally.
github-actions
bot
added
the
status: archived
See #5307