feat: inferred stream type #592
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/playback-core/src/index.ts
Outdated
| .then((mediaPlaylistStr) => { | ||
| const typeLine = mediaPlaylistStr.split('\n').find((line) => line.startsWith('#EXT-X-PLAYLIST-TYPE')) ?? ''; | ||
|
|
||
| const playlistType = typeLine.split(':')[1]?.trim() as HlsPlaylistTypes; |
There was a problem hiding this comment.
there is a block of identical code here and in the hls.js use case. Break into a shared function?
| @@ -2,78 +2,65 @@ | |||
| { | |||
| "playback-id": "a4nOgmxGWg6gULfcBbAa00gXyfcwPnAFldF8RdsNyk8M", | |||
| "description": "Landscape VOD", | |||
| "stream-type": "on-demand", | |||
There was a problem hiding this comment.
Removed the explicit stream type for testing inferred behavior, but we may want to consider keeping these values removed.
There was a problem hiding this comment.
Removed in this PR.
| @@ -700,24 +707,22 @@ class MuxPlayerElement extends VideoApiElement { | |||
| * we aren't an audio player and the stream-type isn't live. | |||
| */ | |||
| get storyboard() { | |||
| // If the storyboardSrc has been explicitly set, assume it should be used | |||
There was a problem hiding this comment.
Intentionally changed the rules here a bit to always prefer the storyboardSrc if it's explicitly set to unlock e.g. audio themes that have storyboards.
cjpillsbury
force-pushed
the
feat/inferred-stream-type
branch
from
115ef88 to
06c38c1
Compare
Co-authored-by: Wesley Luyten <[email protected]>
There was a problem hiding this comment.
LGTM!
one thing I also have to remind myself of is keep the PR small enough.
ideally something like half the size of this, sometimes it's difficult but this one could have been split up in a few I think. would also help with merge conflicts. faster reviews / merges
| @@ -0,0 +1,23 @@ | |||
| # Deprecated Stream Type | |||
There was a problem hiding this comment.
(non-blocking) -- open to other opinions.
Can we put this on a branch for safe keeping? Feels weird to have it in the repo if it's never linked.
Might also want to put the commented code on a branch. Feels a little sloppy to have commented out code making it into main
Implementing inferred stream types and related (
targetLiveWindow,liveEdgeStart).This PR removes the requirement for setting stream type, which can now be inferred directly from the media. Note that, since this process is asynchronous and does not begin until the media metadata is requested (aka will not immediately begin if
preload="none"), this means that, for our current UI designs, folks will see an "on demand" UI until thestreamTypeand related has been inferred (at least by default).Additionally, for this iteration, we will continue to support/translate "invalid" stream types to their appropriate corresponding properties/attributes. See https://github.com/cjpillsbury/elements/blob/feat/inferred-stream-type/errors/deprecated-stream-type.md for how to migrate now-deprecated stream types.
Finally, there is now a distinct
default-stream-type/defaultStreamTypeattribute/property for cases where we want a different UI to show by default. I've treated any corresponding "default-target-live-window" attr/prop as out of scope for this iteration, as there is no corresponding implementation within Media Chrome.