phetsims / resistance-in-a-wire

"Resistance in a Wire" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/resistance-in-a-wire
GNU General Public License v3.0
1 stars 4 forks source link

Small sound when muted #205

Closed KatieWoe closed 5 years ago

KatieWoe commented 5 years ago

Test device: Dell Operating System: Win 10 Browser: Edge (Not reproducible on chrome or firefox that I've seen) Problem description: For https://github.com/phetsims/QA/issues/247 It is possible to hear a tiny fraction of the ding sound when moving a slider with keyboard nav just after muting the sound. Steps to reproduce:

  1. Make sure sound is on
  2. Use keyboard nav shift to put one of the sliders in a value slightly above a whole number (A=8.02 for example)
  3. Use keyboard nav to mute sound
  4. Navigate back to the slider from (2) using keyboard nav
  5. Adjust the slider to the nearest whole number using keyboard nav (hit the down arrow key to make A=8 from the example above)

Troubleshooting information (do not edit):

Name: ‪Resistance in a Wire‬ URL: https://phet-dev.colorado.edu/html/resistance-in-a-wire/1.6.0-rc.3/phet/resistance-in-a-wire_en_phet.html Version: 1.6.0-rc.3 2018-12-28 18:35:19 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36 Edge/18.17763 Language: en-US Window: 1536x760 Pixel Ratio: 2.5/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 Vendor: Microsoft (Microsoft Edge) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"928741cf","branch":"HEAD"},"axon":{"sha":"de77d4b5","branch":"HEAD"},"brand":{"sha":"1fd6682e","branch":"HEAD"},"chipper":{"sha":"fa2fbadf","branch":"HEAD"},"dot":{"sha":"bbbd8526","branch":"HEAD"},"joist":{"sha":"4352e866","branch":"HEAD"},"kite":{"sha":"380cef53","branch":"HEAD"},"phet-core":{"sha":"1b90ac2f","branch":"HEAD"},"phet-io":{"sha":"38d7b161","branch":"HEAD"},"phet-io-wrapper-classroom-activity":{"sha":"246085c1","branch":"HEAD"},"phet-io-wrapper-hookes-law-energy":{"sha":"7479b0ec","branch":"HEAD"},"phet-io-wrapper-lab-book":{"sha":"c46f7839","branch":"HEAD"},"phet-io-wrappers":{"sha":"a6bc62ca","branch":"HEAD"},"phetcommon":{"sha":"cd63d89a","branch":"HEAD"},"query-string-machine":{"sha":"06ed6276","branch":"HEAD"},"resistance-in-a-wire":{"sha":"9ec241e6","branch":"HEAD"},"scenery":{"sha":"9953c5f7","branch":"HEAD"},"scenery-phet":{"sha":"7d0ce8b6","branch":"HEAD"},"sherpa":{"sha":"2cd50500","branch":"HEAD"},"sun":{"sha":"3c28faa7","branch":"HEAD"},"tambo":{"sha":"65315b32","branch":"HEAD"},"tandem":{"sha":"ed8f8f1d","branch":"HEAD"}}
jbphet commented 5 years ago

I'm able to duplicate on my Win 10 machine running Edge version 42.17134.1.0. I appreciate the thorough testing that uncovered this. In my opinion, this is a super minor issue - it's an unlikely use case on a lightly used browser that produces a hardly noticeable sound. I'll spend a few minutes investigating, but I don't feel that it warrants much more than that.

jbphet commented 5 years ago

In the course of investigating this, I added some code to the soundManager object that can log the gain value for the master gain node at every animation frame for a length of time specified by the client. The method looked like this:

    // TODO: temp for debug
    logGain: function( duration ){
      duration = duration || 1;
      var startTime = phetAudioContext.currentTime;
      console.log( '------- start of gain logging -----' );
      var self = this;
      function logGain(){
        var time = phetAudioContext.currentTime - startTime;
        console.log( 'Time: ' + time.toFixed( 6 ) + ', Gain Value: ' + self.masterGainNode.gain.value );
        if ( phetAudioContext.currentTime - startTime < duration ){
          window.requestAnimationFrame( logGain );
        }
      }
      logGain();
    }

I made this method globally available, then invoked it whenever the sound enable/disable setting changed and whenever a sound was played. This led me to determine that Edge seems to delay the change to the gain value until a sound actually comes through the node. I tested this same case on Chrome, and Chrome executes the change to the gain value right away. I tried testing on Firefox, but Firefox doesn't appear to support reading of gain values at all - they are always 1. But the thing that really surprised me was that Safari does the same thing that Edge does, i.e. it doesn't make the change to the gain until a sound is played. This is making me suspect that there is something that is ambiguous or undefined in the Web Audio spec about how these changes are supposed to take place, and perhaps some way they can be forced to occur. This may also help to explain some other audio oddities that we are seeing, such as those described in https://github.com/phetsims/friction/issues/159.

jbphet commented 5 years ago

I have something that may fix this issue. The master gain level issue currently being set using the setTargetAtTime method. When I use that method, the behavior is as I mentioned before, which is that the gain level in Edge and Safari doesn't change until a sound goes through the node, and then it changes based on the provided time constant. I just tried using the method linearRampToValueAtTime instead, and it still doesn't change the level immediately, but it makes the change immediately when a sound is played. I'm not sure why it doesn't do the linear ramp at that point - it seems like that would be more consistent - but the behavior is the same on both Edge and Safari, so I think it's worth a try.

jbphet commented 5 years ago

So that it isn't missed by anyone who is tracking this issue (or myself in the future), I want to be clear about something: Safari has this problem too, including the audible "small sound". My testing was done on the MacBook Air named "Jordan", OS version 10.13.6, Safari version 12.0.2.

jbphet commented 5 years ago

I've published two dev versions that log console output that shows the differences in handling gain changes between browsers. https://phet-dev.colorado.edu/html/resistance-in-a-wire/1.7.0-dev.2/phet/resistance-in-a-wire_en_phet.html shows the behavior when using setTargetAtTime for setting the master gain (which is how muting works), and https://phet-dev.colorado.edu/html/resistance-in-a-wire/1.7.0-dev.1/phet/resistance-in-a-wire_en_phet.html shows how it behaves when using linearRampToValueAtTime.

I'll back out the debug code shortly, but I thought it would be useful to have these in the future since I think the browsers may eventually converge in their behavior.

jbphet commented 5 years ago

This issue can be resolved by using linearRampToValueAtTime instead of setTargetAtTime. As I mentioned above, I'm not absolutely certain why this works better, but it does.

I ended up adding a bunch of debug code in the process of trying this out, so the best way to fix it in a branch is probably to make the changes manually in tambo.

jbphet commented 5 years ago

I've propagated the fix to the branch of tambo that is used by the RIAW 1.6 branch.

jbphet commented 5 years ago

Here's another thing that I'd like to be clear about in case we reference this issue in the future: It doesn't seem to me that setTargetAtTime should behave the way it does on Edge and Safari, which is that it doesn't start changing its value until some signal is sent through the gain node. For some reason, linearRampToValue behaves differently - it also doesn't seem to execute the operation right away, but it responds more quickly once a signal starts going through the gain stage, and the end result is that we can mute without getting "small sounds" afterwards.

I don't think that this was the intent of the Web Audio specification designers, but it's impossible to say for certain. I fully expect that at some point in the future, the behavior of the browsers will become more consistent, and my money would be on the Chrome behavior. It just seems more logical. If an when that happens, it may be possible to go back to using setTargetAtTime if we want to.

ariel-phet commented 5 years ago

Removing my assignment for now since there is a pending fix.

KatieWoe commented 5 years ago

Looks good 1.6.0-rc.4

emily-phet commented 5 years ago

assigning to myself - want to refer to this later when creating helpful resources for the broader community. This issue seems like something relatively general that others would benefit from knowing about.