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

Make setMicrophoneActive and setCameraActive return promises. #312

Merged

Conversation

youennf
Copy link
Contributor

@youennf youennf commented Jan 10, 2024

Define steps for each of these methods.
Mention the possibility to mute/unmute tracks based on setMicrophoneActive/setCameraActive calls. Mention the possibility for the user agent to deny the mutation of capture states via setMicrophoneActive/setCameraActive calls.


Preview | Diff

@youennf
Copy link
Contributor Author

youennf commented Jan 10, 2024

@jan-ivar, @steimelchrome, PTAL.

Copy link
Contributor

@steimelchrome steimelchrome left a comment

Choose a reason for hiding this comment

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

This looks good generally though I don't really know much about MediaStreamTrack

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

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

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

Thanks for this! Some nits.

I've only commented on setMicrophoneActive(false) but they apply to setCameraActive(false) as well.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
index.bs Outdated
Comment on lines 85 to 90
urlPrefix: https://www.w3.org/TR/mediacapture-streams/; spec: mediacapture-main
type: dfn
text: MediaStreamTrack; url:#mediastreamtrack
text: MediaStreamTrack muted state; url:#track-muted
text: set MediaStreamTrack muted state; url:#set-track-muted
Copy link
Member

Choose a reason for hiding this comment

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

Can we get these exported properly from the other spec if needed?

Copy link
Member

Choose a reason for hiding this comment

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

How does w3c/mediacapture-main#986 look?

index.bs Outdated 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
index.bs Outdated Show resolved Hide resolved
Copy link
Contributor

@beaufortfrancois beaufortfrancois left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

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

Forgot to request changes with my earlier comments.

@youennf
Copy link
Contributor Author

youennf commented Feb 2, 2024

You may to update https://w3c.github.io/mediasession/#example-microphone-camera-hangup as well

Let's do that in a follow-up, given the examples are still ok, at least in Chrome.

Define steps for each of these methods.
Mention the possibility to mute/unmute tracks based on setMicrophoneActive/setCameraActive calls.
Mention the possibility for the user agent to deny the mutation of capture states via setMicrophoneActive/setCameraActive calls.
@youennf youennf force-pushed the enable-asynchronous-errors-insetActive-methods branch from 79293d8 to 772f0ec Compare February 2, 2024 07:57
index.bs Outdated 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
index.bs Show resolved Hide resolved
index.bs Outdated Show resolved Hide resolved
youennf and others added 4 commits February 2, 2024 17:58
Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>
Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>
Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>
index.bs Outdated 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
index.bs Outdated Show resolved Hide resolved
Copy link
Member

@jan-ivar jan-ivar left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

index.bs Show resolved Hide resolved
Copy link
Contributor

@steimelchrome steimelchrome left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@youennf youennf merged commit fac2b9a into w3c:main Feb 15, 2024
2 checks passed
github-actions bot added a commit that referenced this pull request Feb 15, 2024
SHA: fac2b9a
Reason: push, by youennf

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

5 participants