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

How should tambo deal with sounds attempting to play during sim construction? #181

Open
samreid opened this issue Jun 21, 2023 · 3 comments

Comments

@samreid
Copy link
Member

samreid commented Jun 21, 2023

From phetsims/center-and-variability#261, @matthew-blackman and I observed that if you clicked in the splash screen during startup, there were spurious sound effects (clipped audio, for instance) during startup. It seemed to happen more frequently with the dev tools closed. But never happened 100% of the time.

We were wondering if tambo should gracefully ignore attempts to play audio before the sim launcher is complete? Or alternatively, it could throw an assertion error if sound is attempted before sim launcher is complete, so we can catch this during development instead of testing.

@jbphet what do you think?

@jbphet
Copy link
Contributor

jbphet commented Jun 21, 2023

I just tried it a bunch of times and wasn't able to make it happen. I noticed that @marlitas said the same thing in phetsims/center-and-variability#261 (comment).

This probably has something to do with the auto-play policy. My guess is that some sounds are being set up as the sim is starting, but the associated gain nodes aren't being successfully turned down to zero until some user interaction happens, and that is happening in this case when clicking on the splash screen.

This doesn't seem like a huge priority to me, since I think it would be unusual for our users to click on the splash screen, it doesn't happen all the time even if they do, and it doesn't seem too disturbing even if it does. But that's just my two cents. If we decide that this is worth pursuing, here is what I would suggest:

  • Assign an issue to QA where they figure out what browsers and OSes this can be duplicated on. This matters because the auto-play policies vary from browser to browser.
  • Get QA to find a way to duplicate the issue consistently so that we can verify it's fixed. If this proves to be impossible, get some videos of it happening so that we can look for patterns.
  • See if it happens on other sims

Once this information is available, we would potentially know enough start addressing the problem and be in a position to test potential fixes. The first thing I'd be inclined to try is some sort of a timeout that doesn't attach the last portion of the audio path to the audio context "destination" until the audio context has been in the running state for some amount of time so that all the gain nodes have time to reach their target values.

Back to @samreid for next steps.

@jbphet jbphet assigned samreid and unassigned jbphet Jun 21, 2023
@samreid
Copy link
Member Author

samreid commented Jun 21, 2023

Were you testing with or without the sim-specific workarounds committed in phetsims/center-and-variability@15fccc5 ?

@samreid samreid assigned jbphet and unassigned samreid Jun 21, 2023
@jbphet
Copy link
Contributor

jbphet commented Jun 21, 2023

The answer to the question above is "with", since I was testing master (didn't know any better).

I just discussed this with @samreid over Zoom, and he was basically wondering if it would be super easy to add either an assertion for attempts to play sounds before startup was complete or a way to prevent sound until then. I think it could definitely be addressed, but not super easily, and I also think a change like this should be thoroughly regression tested. @samreid and I agreed that this probably isn't worth the time at this point.

I'll leave this issue open and deferred for now, and we can add it to the list of backlogged tambo issues that we may at some point bundle together into a task that is high enough priority to work on. If it languishes for more than, say, a year, I think we should just close it as "won't fix".

@jbphet jbphet removed their assignment Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants