Skip to content

Conversation

@mjwilson-google
Copy link
Contributor

@mjwilson-google mjwilson-google commented Mar 19, 2025

This affects #2435, but may not fix everything in that issue.

We discussed at TPAC 2024 that, because device enumeration is asynchronous, it will be easier for implementers and also more robust to use onerror to signal problems with the device at construction time.

This PR does the following:

  • Move the sink validation to before system resources are acquired
  • Change the sink validation algorithm to return boolean values
  • Update both uses of the sink validation algorithm to use the new return value

Preview | Diff

@mjwilson-google mjwilson-google added the Agenda+ Should be discussed at the next WG meeting. https://speced.github.io/spec-maintenance/about/ label Mar 19, 2025
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

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

I think this is what we discussed offline, but I am curious if there are places we might need to spot check,

@mjwilson-google
Copy link
Contributor Author

Thank you for the quick review! I'm not sure if this is the best way to word this change, so any feedback or additional checking is welcome.

Copy link
Member

@padenot padenot left a comment

Choose a reason for hiding this comment

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

One small non-fonctional comment.

@mjwilson-google mjwilson-google removed the Agenda+ Should be discussed at the next WG meeting. https://speced.github.io/spec-maintenance/about/ label Mar 20, 2025
@mjwilson-google
Copy link
Contributor Author

After the working group meeting this PR needs some rework, I will comment again when it is ready for review.

@mjwilson-google mjwilson-google marked this pull request as draft March 20, 2025 21:55
@mjwilson-google mjwilson-google marked this pull request as ready for review March 20, 2025 22:43
@mjwilson-google
Copy link
Contributor Author

I think the current patch covers what we talked about at the meeting:

  • Add private internal slot for sinkid, to hold the constructor parameter until we are sure the sinkid is usable
  • Then we can update the javascript-visible slot with the valid ID once it has been opened
  • If validation fails, fall back to default device, fire onerror, and do not transition to running
  • For acquisition failure, always fire onerror, and we can remove the speaker-selection step since it will be handled in the validation step

I'm not sure about the naming of the private slot, and we also have two things called sending a control message to start processing (one in the constructor and one for setsinkid) but hopefully it's clear from context which one is which.

@hoch hoch merged commit 27add2c into WebAudio:main Mar 24, 2025
1 check passed
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.

3 participants