Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: live edge. #7

Merged
merged 13 commits into from
Feb 22, 2023
Merged

Conversation

cjpillsbury
Copy link
Collaborator

@cjpillsbury cjpillsbury commented Feb 9, 2023

Live Edge Proposal

(closes #6)

@gkatsev
Copy link
Member

gkatsev commented Feb 9, 2023

If you rebase, feel free to delete the proposals/.gitkeep file.

proposals/0007-live-edge.md Outdated Show resolved Hide resolved
proposals/0007-live-edge.md Outdated Show resolved Hide resolved
proposals/0007-live-edge.md Outdated Show resolved Hide resolved
proposals/0007-live-edge.md Outdated Show resolved Hide resolved
Copy link
Member

@heff heff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So good. So thorough. 🎉

proposals/0007-live-edge.md Outdated Show resolved Hide resolved
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why this tries to clarify that seekable.end() means the "live sync point" in hls.js terminology, but it still feels a bit out of place here. It's like, we're on a road to the live edge, but there's an unexpected detour. I think it feels extra weird to me because my reading of seekable.end() is already this.

## Definitions

- __Advertised Live Edge__ - This is the latest available media time as represented in a manifest, playlist, or simiilar. This will not be exposed by any API defined in this proposal, but is defined here for reference in the recommended computations for Seekable Live Edge.
- __Seekable Live Edge__ - This is the latest available media time a player should attempt to seek to or play, typically some offset from the Advertised Live Edge, determined in part by things like the `HOLD-BACK` or the `suggestedPresentationDelay`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this mention that this is reflected via video.seekable.end(video.seekable.length-1)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to the "Live Edge Window" and "Live Seekable Window" definitions, but I'll add here as well.

proposals/0007-live-edge.md Outdated Show resolved Hide resolved
proposals/0007-live-edge.md Outdated Show resolved Hide resolved
@cjpillsbury
Copy link
Collaborator Author

cjpillsbury commented Feb 16, 2023

@gkatsev

I understand why this tries to clarify that seekable.end() means the "live sync point" in hls.js terminology, but it still feels a bit out of place here. It's like, we're on a road to the live edge, but there's an unexpected detour. I think it feels extra weird to me because my reading of seekable.end() is already this.

While many native/MSE-based players/playback engines do implement this, it is not explicitly defined anywhere (hence the hls.js deviation, though there may be others that we don't yet know of). Because of that, and since this proposal explicitly depends on that constraint, I think this needs to be in a media-ui-extension proposal somewhere. I'm definitely amenable to it being broken out into a separate proposal/PR if folks would prefer that (though that means that proposal needs to be settled before this one is accepted).

@heff @luwes thoughts/concerns/preferences here?

@heff
Copy link
Member

heff commented Feb 18, 2023

I'm not totally following the concern. Is it that defining seekable.end doesn't belong, or that it's a lot before getting to the actual new API being proposed?

If it's just reordering the changes in the proposal, that seems appropriate to me. Is an option to reframe the seekable.end definition as assumed-to-be-true, but then call out the actual and potential deviations from it?

It seems clear that we can't ignore the seekable.end detail in this proposal all together. I feel fine leaving it in. If we think we're going to be pushing the seekable.end defintion on its own to various groups, it could be worth its own extension, when that happens.

@cjpillsbury
Copy link
Collaborator Author

If it's just reordering the changes in the proposal, that seems appropriate to me.

👍 I don't see any concern here other than the fact that liveEdgeStart refers to the constrained seekable.end() definition, which isn't enough reason to push back on this ask imo.

Is an option to reframe the seekable.end definition as assumed-to-be-true, but then call out the actual and potential deviations from it?

I'd rather keep the definition as is, since it currently defines the constraint based on what it is expected to be and not based on an ad hoc list of deviations from that expectation.

@gkatsev gkatsev merged commit bff9d68 into video-dev:main Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Live Edge Window
4 participants