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

Harsh Sound in TamTam Micro activity in keyboard mode on Firefox and Safari #564

Closed
s-mag opened this issue Jan 16, 2020 · 18 comments
Closed
Labels
bug to be release Fixed, to be release
Milestone

Comments

@s-mag
Copy link
Contributor

s-mag commented Jan 16, 2020

To reproduce : Change any instrument and play 2 sounds together..
You will hear distorted sounds throughout the activity.
@llaske Can you reproduce this bug...

@llaske
Copy link
Owner

llaske commented Jan 18, 2020

I'm not able to reproduce it. I'm trying with guitar for example.

@s-mag
Copy link
Contributor Author

s-mag commented Jan 18, 2020

@llaske After 5-6 clicks together this happens. Is this a bug or just exceeding the limit which causes internal errors.

@llaske
Copy link
Owner

llaske commented Jan 19, 2020

I reproduced it on Firefox thought it perfectly work on Chrome. Could you confirm you've tested it on Firefox?

@s-mag
Copy link
Contributor Author

s-mag commented Jan 19, 2020

I reproduced it on Firefox thought it perfectly work on Chrome. Could you confirm you've tested it on Firefox?

Yes it is in firefox, no problem in chrome. I forgot to mention that!

@llaske llaske changed the title Harsh Sound in TamTam Micro activity in keyboard mode. Harsh Sound in TamTam Micro activity in keyboard mode on Firefox Jan 19, 2020
@llaske llaske changed the title Harsh Sound in TamTam Micro activity in keyboard mode on Firefox Harsh Sound in TamTam Micro activity in keyboard mode on Firefox and Safari Jan 22, 2020
@llaske
Copy link
Owner

llaske commented Jan 22, 2020

For information, same issue on Safari

@pidddgy
Copy link
Contributor

pidddgy commented Jan 25, 2020

very very interesting bug.... when I reproduce the bug then play audio on anything else e.g. other tabs, spotify desktop app, system noises that is distorted and crackly too, until i close the tab with sugarizer. tested on firefox

@s-mag
Copy link
Contributor Author

s-mag commented Jan 25, 2020

very very interesting bug.... when I reproduce the bug then play audio on anything else e.g. other tabs, spotify desktop app, system noises that is distorted and crackly too, until i close the tab with sugarizer. tested on firefox

Yes, this might be internal system error in firefox aswell. I am not sure btw

@s-mag
Copy link
Contributor Author

s-mag commented Jan 25, 2020

@llaske @pidddgy according to http://forums.mozillazine.org/viewtopic.php?f=38&t=2542145 forum The issue comes in Firefox 15

@llaske
Copy link
Owner

llaske commented Jan 25, 2020

@s-mag This Firefox issue is very old, I'm not sure it's related.
@pidddgy Is there a way to reproduce the problem with just a Tone.js sample?

@s-mag
Copy link
Contributor Author

s-mag commented Jan 25, 2020

@s-mag This Firefox issue is very old, I'm not sure it's related.

Oh yeah it’s very old...

@pidddgy
Copy link
Contributor

pidddgy commented Jan 27, 2020

perhaps could be related to
https://developers.google.com/web/updates/2017/09/autoplay-policy-changes#webaudio? i'm really not sure. I tried changing the play code in piano.js to:

Tone.context.resume().then(() => {
            var player = new Tone.Player('./audio/database/'+currentPianoMode+".mp3");
            var pitchShift = new Tone.PitchShift({
                pitch: pitchMap[pitchName]
            }).toMaster();
            player.connect(pitchShift);
            player.autostart = true;
            console.log('playing');
        })

as according to https://stackoverflow.com/questions/50281568/audiocontext-not-allowed-to-start-in-tonejs-chrome but that didn't do anything. I am getting console warnings about The AudioContext was not allowed to start. It must be resumed (or created) after a user gesture on the page.

@llaske
Copy link
Owner

llaske commented Jan 28, 2020

I know this limit regarding playing sound. It happens in some case in other activities.
But I don't think it's related to our problem. Plus, the problem is on Firefox, not Chrome.
Will be interested to try to reproduce the issue on a simple Tone.js sample so we could ask to Tone.js community.

@pidddgy
Copy link
Contributor

pidddgy commented Jan 29, 2020

@llaske https://github.com/pidddgy/tonejstest i've written up a short demonstration here

@llaske
Copy link
Owner

llaske commented Jan 29, 2020

Nice! Could you open an issue in ToneJS repo?

@llaske
Copy link
Owner

llaske commented Jan 30, 2020

@pidddgy did you see the answer from ToneJS community? Very interesting!
Guess your current code could be improved to try to reuse ToneJS objects.

@llaske
Copy link
Owner

llaske commented Feb 13, 2020

@pidddgy any progress on it? I've got a new TamTam mode planned but don't want to launch before this issue is fixed

@pidddgy
Copy link
Contributor

pidddgy commented Feb 14, 2020

sorry, I haven't made significant progress on the issue. I'll likely be taking a break from open source for the next few months to focus on OI contests/school.

For anyone interested in tackling this issue, the problem is that Tone.Player objects are being created on every play and not being disposed of. To solve this issue I think the solution is to create a global map of Tone.Player objects, perhaps through the Tone.Players class which is supposed to be a collection of Tone.Players, and start the appropriate player on click. I have tried setting an interval to dispose of a player if it is currently not playing, but that is too slow and while improves the issue it will bug out if you click really fast.

relevant reading:
https://tonejs.github.io/docs/13.8.25/Player
https://tonejs.github.io/docs/13.8.25/Players
pixijs/sound#65
https://github.com/Tonejs/Tone.js/wiki/AudioContext

I'm not quite familiar with the Tone.js library, but I think it's just a matter of "getting it to work" and finding the right combination of code.

@llaske
Copy link
Owner

llaske commented Feb 16, 2020

Thanks for your help @pidddgy. I've fixed it in 5f1889a

@llaske llaske added the to be release Fixed, to be release label Feb 16, 2020
@llaske llaske added this to the v1.3 milestone Mar 11, 2020
@llaske llaske closed this as completed Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug to be release Fixed, to be release
Projects
None yet
Development

No branches or pull requests

3 participants