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

Add an API for a "keyframe has been requested" event #215

Merged
merged 9 commits into from
Dec 12, 2023
Merged

Conversation

alvestrand
Copy link
Contributor

@alvestrand alvestrand commented Nov 28, 2023

This is split from #207 - in the October interim discussion, the overall congestion control API was seen as requiring more discussion, but the keyframe proposal was thought reasonable to pursue on its own.

I preserved the split into "ScriptSource" and "ScriptSink" interfaces - I think that can be valuable for documenting which interfaces are facing in which direction on the transformer.

Comments welcome.


Preview | Diff

@alvestrand
Copy link
Contributor Author

Fixes #210

index.bs Outdated

## Events ## {#RTCRtpScriptTransformer-events}

The following events fire on an {{RTCRtpScriptTransformer}}:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should rewrite as "when the UA decides that a key frame has been requested, the UA will queue a task to fire an event...."

@jan-ivar
Copy link
Member

LGTM if @youennf agrees.

@alvestrand
Copy link
Contributor Author

Waiting for @youennf

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
Copy link
Collaborator

@youennf youennf left a comment

Choose a reason for hiding this comment

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

Let's first introduce onkeyframerequest without introducing the mixins, we can do that as a follow-up.

@alvestrand
Copy link
Contributor Author

Mixins removed as agreed during the editors meeting.
I also tightened up the definition of what to do when the keyframerequest event fires.
Should be ready to merge now - hope to merge it before the WG meeting Tuesday, so that we can say "it is done".

index.bs Outdated Show resolved Hide resolved
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
@alvestrand
Copy link
Contributor Author

Despite bikeshed refs --for=Event --text='canceled flag' returning a value (spec=dom), I'm unable to make {{Event/canceled flag}} autolink. @dontcallmedom help?

@dontcallmedom
Copy link
Member

dontcallmedom commented Dec 12, 2023

the correct syntax would be [=Event/canceled flag=] - the {{ }} syntax is only for WebIDL terms, when "canceled flag" is a regular definition (see relevant Bikeshed doc)

index.bs Outdated Show resolved Hide resolved
Co-authored-by: Dominique Hazael-Massieux <[email protected]>
@alvestrand
Copy link
Contributor Author

the correct syntax would be [=Event/canceled flag=] - the {{ }} syntax is only for WebIDL terms, when "canceled flag" is a regular definition (see relevant Bikeshed doc)

Thanks - that worked. I could have sworn that I tried that before!

@alvestrand alvestrand merged commit 678a265 into main Dec 12, 2023
2 checks passed
github-actions bot added a commit that referenced this pull request Dec 12, 2023
SHA: 678a265
Reason: push, by alvestrand

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@dontcallmedom-bot
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants