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 sound #166

Closed
zepumph opened this issue Jul 2, 2019 · 17 comments
Closed

Add sound #166

zepumph opened this issue Jul 2, 2019 · 17 comments

Comments

@zepumph
Copy link
Member

zepumph commented Jul 2, 2019

It was brought to my attention by @jbphet and @emily-phet that sound isn't implemented yet in this repo. This needs to be done before publication.

@terracoda
Copy link

Thanks for posting this @zepumph. I just noticed yesterday that Molarity's repo does have sound, and I was going to ask why this repo doesn't.

@jbphet
Copy link
Contributor

jbphet commented Jul 9, 2019

The status of this was discussed in the 7/9/2019 sound design meeting, and the sound design document is not really finished. The prototype sound design at the link below should be used as the specification.

https://phet-dev.colorado.edu/html/phet-io-wrapper-sonification/1.0.0-dev.112/phet-io-wrapper-sonification/gravity-force-lab-basics/html/gravity-force-lab-basics-sonification.html?sonificationFile=sonificationOptions&massSound=0&forceSound=0&massBoundaryLimitSound=2&accessibility

jbphet added a commit that referenced this issue Jul 9, 2019
@zepumph
Copy link
Member Author

zepumph commented Jul 10, 2019

The addition of the resetInProgressProperty broke the phet-io version of this sim over in #167. In addition, I had a few questions about the addition:

  1. Why does this need to be instrumented?
  2. Why is this not implemented in the ResetAllButton or ResetButton itself?
  3. To me it feels like this pattern could easily be duplicated wherever there is a reset that needs listening.
  4. Isn't this the same logic as ResetAllButton.isFiringProperty? I see that is @private, but don't know why we couldn't make it @public (read-only) for just these sorts of purposes.

@jbphet
Copy link
Contributor

jbphet commented Jul 10, 2019

The initial cut of sound has been added to the sim. @Ashton-Morris and @emily-phet, please review (see dev link below). I matched the volume levels of the prototype, so it might not need much mixing, but if needed the levels can be easily adjusted.

If you notice anything through Friday July 12th, please assign this back to me, and after that time assign it to @zepumph.

Link: https://phet-dev.colorado.edu/html/gravity-force-lab-basics/1.0.0-dev.41/phet/gravity-force-lab-basics_en_phet.html.

@jbphet jbphet assigned Ashton-Morris and emily-phet and unassigned jbphet Jul 10, 2019
jbphet added a commit that referenced this issue Jul 10, 2019
@terracoda
Copy link

terracoda commented Jul 12, 2019

Great work @jbphet.

I feel that all the sim-specific sounds need to be louder, more on the same level as the UI sounds.

I have my sound on max and with a screen reader on I do not hear the Las stop boomp at all, and the force sound is difficult to hear when changing mass.

@jbphet
Copy link
Contributor

jbphet commented Jul 12, 2019

Thanks @terracoda. Once @Ashton-Morris has added his input, this issue should be assigned to @zepumph for followup. I'm going to be out for a while, and he has been briefed on how to make volume adjustments.

@Ashton-Morris
Copy link

I agree with @terracoda. I think that the reset stands out as much louder but the checkbox isn't too loud in comparison to the rest.

@zepumph Can we bring up all sim specific sounds 10% so they are closer to the reset volume.

Also, the audio file for mass has a popping sound each time I play it, in Safari and Chrome.

And on Safari only, the force sine wave sound has a popping sound most of the time I let go after the changing distance.

@zepumph
Copy link
Member Author

zepumph commented Jul 23, 2019

Also, the audio file for mass has a popping sound each time I play it, in Safari and Chrome.

And on Safari only, the force sine wave sound has a popping sound most of the time I let go after the changing distance.

After discussing with @emily-phet, we decided that it isn't worth my time in investigating these since it would largely be discovering knowledge that @jbphet already has. As a result I'm not going to work on any of this. I'm going to assign this back over to @jbphet for when he returns.

@zepumph zepumph removed their assignment Jul 23, 2019
@jbphet
Copy link
Contributor

jbphet commented Jul 30, 2019

In the 7/30/2019 sound design meeting, we decided to turn up all of the sim-specific sounds based on @terracoda's feedback and our own review.

jbphet added a commit that referenced this issue Jul 30, 2019
jbphet added a commit that referenced this issue Jul 31, 2019
jbphet added a commit that referenced this issue Jul 31, 2019
jbphet added a commit that referenced this issue Jul 31, 2019
@jbphet
Copy link
Contributor

jbphet commented Jul 31, 2019

I ended up making quite a number of changes to the sound files (i.e. the mp3 and wav files), the common code library, and the sim-specific sound generation code in order to get rid of the glitches and improve the volume balance. See the commits listed above for details.

I've published a dev version with these changes for review. @Ashton-Morris and @zepumph - please review the sound behavior in this version and let me know if further changes are needed and, if so, what you'd recommend.

link: https://phet-dev.colorado.edu/html/gravity-force-lab-basics/1.0.0-dev.43/phet/gravity-force-lab-basics_en_phet.html

@jbphet jbphet assigned zepumph and unassigned jbphet Jul 31, 2019
@terracoda
Copy link

@jbphet, I had a quick listen (with headphones) in Safari 12.1.2 on macOS High Sierra 10.13.6, MacBook Pro.

The levels sounded much better to me. I listened with and without a screen reader and could easily make out all the sounds.

I noticed a little crackle only if I listened hard, so that seems reduced, too - at least with my setup.

Nice work everyone!

@Ashton-Morris
Copy link

@jbphet The levels sound much better. Also I barely heard any clicking when I was changing the mass.

I did however hear some slight clicking when the force loop while in use. I don't think its something the average person will hear but I'll attach an image just so you can see what I mean.

Screen Shot 2019-08-01 at 2 12 47 PM

These lines are where the clicking is occurring. ^ But the distance between them is the same ever though the duration of the loop is changing due to the playback rate changing. So not sure what that is.

If anyone else notices it, it might be worth looking into. But I'm not sure they will.

@jbphet
Copy link
Contributor

jbphet commented Aug 1, 2019

I did however hear some slight clicking when the force loop while in use.

@Ashton-Morris - did you only hear it on Safari? If so, I think I know what it is, and I was hearing it too. For some reason, Safari doesn't seem to handle loops as well as Chrome, and it inserts a small but audible discontinuity when starting the loop over. We don't notices this in cases like the John Travoltage "charges in the body" sound because we made sure that the sound file wasn't producing sound at the very beginning or end. Because this is a constant volume wave, we can't really get away with that in this case.

I'm inclined to not worry about it. If we decide that we do need to address it, I think some possible ways to approach it are:

  • get as close as we can to duplicating the sound using oscillators instead of a loop
  • change the loop to have a bit of low frequency amplitude modulation and then leave a space at the end so that there won't be an audible glitch.

@Ashton-Morris
Copy link

That spectrogram was taken off of a recording when I was using Chrome. I did hear it in safari a little as well.

@jbphet
Copy link
Contributor

jbphet commented Aug 8, 2019

We discussed the remaining sound artifact issue in the 8/6/2019 sound design meeting and decided that it's barely perceptible and there's not much we can do about it, so we're okay with it as is.

@zepumph
Copy link
Member Author

zepumph commented Aug 8, 2019

Sounds good to me. Is this issue ready to close then?

@zepumph zepumph assigned jbphet and unassigned zepumph Aug 8, 2019
@jbphet
Copy link
Contributor

jbphet commented Aug 9, 2019

Yep.

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

5 participants