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

fixed abnormal behaviour of audio output in p5.signal example #476

Merged
merged 2 commits into from
May 2, 2020

Conversation

endurance21
Copy link
Collaborator

fixes #471

before :
carrier.start() was placed inside setup function, and as soon as the script get rendered , user hears a all of a sudden sound of carrier node without the user consent 😬 !!

after:
carrier.start() has been placed inside the canvasPressed() which ensures that output signal is heard by user when it Presses the canvas ! 😀

@endurance21
Copy link
Collaborator Author

@therewasaguy @montoyamoraga
please review this PR !!

@montoyamoraga
Copy link
Member

@endurance21 at the right side of the repo i have the option to ask for review, i am going to add myself, do you have that option? just wondering, so that it is easier than tagging.
also, it is easy to miss notifications, we do our best to review them, tag me anytime you need, best!

@endurance21
Copy link
Collaborator Author

@montoyamoraga i do see them, but i am not able to either assign myself or ask for review, i am wondering may be i don't have enough permissions😬

Copy link
Member

@montoyamoraga montoyamoraga left a comment

Choose a reason for hiding this comment

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

this is way better, but it would also execute carrier.start() everytime the canvas was pressed, which i don't think it's ideal, this is what i would add:

  • a boolean variable called hasStarted to detect if the user has given the consent yet, it is declared false.
  • on canvasPressed, first check if that boolean is true or false. if it is false, make it true and execute carrier.start(). after that check, execute carrier.amp(1.0)

this way, at first there is no audio, then only when canvas is pressed the first time, the audio is started and the amplitude is set to 1.0. when mouse is released the amplitude is set to 0.0 yay. then with next clicks, the audio is already sarted, and canvasPressed takes care of setting amplitude to 1.0 every time it is pressed :)

@endurance21
Copy link
Collaborator Author

@montoyamoraga
aha , that's more nicer way to do that ! 😁
also , whenever mouse is Realeased we have to make hasStarted = false

@endurance21
Copy link
Collaborator Author

@montoyamoraga
applied the changes ! ☺️

@endurance21 endurance21 requested a review from montoyamoraga May 2, 2020 16:45
Copy link
Member

@montoyamoraga montoyamoraga left a comment

Choose a reason for hiding this comment

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

can i ask for two little changes?

  1. can the boolean be declared in its own line? i find it less confusing.
  2. the hasStarted boolean is for being able to signal when the sketch goes from never having outputted audio, to when it started making audio, so it is only needs to become true, but never false again. the muting / unmuting afterwards happens because of the amplitude, i pasted the code here, sorry about the formatting :)

function canvasPressed() {
userStartAudio();

if(!hasStarted){
carrier.start();
hasStarted!=hasStarted;
}

carrier.amp(1.0);

}

function mouseReleased() {
carrier.amp(0);
}

@montoyamoraga
Copy link
Member

@endurance21 let me know if it makes sense and if it works please, thank you!
i hadn't used these features for reviewing here on GitHub so i am also learning :)

@endurance21
Copy link
Collaborator Author

@montoyamoraga

  1. if you are asking hasStarted = false ? , ya sure , we can sure initilize the boolean at the declaration time !!
  2. and yes i agree on it ! , it must be started only once ! i have tested it too, it works fine

@montoyamoraga
Copy link
Member

@endurance21 thank you!

yes, i mean having two separate lines like this:

let carrier, modulator;
let hasStarted = false;

@endurance21
Copy link
Collaborator Author

@montoyamoraga
ya that is more intuitive and readable , but i was kind'a avoiding use of multiple let 😅
but i think we should go with you idea ! 😀

Copy link
Member

@montoyamoraga montoyamoraga left a comment

Choose a reason for hiding this comment

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

awesome!

@montoyamoraga montoyamoraga merged commit 374d306 into processing:master May 2, 2020
@montoyamoraga
Copy link
Member

@endurance21 it's a matter of style and use,
i always use one line for every variable declaration,
i'm not sure if we have a formal style guide for p5.js examples,
maybe we should, maybe it's too much :)

@endurance21
Copy link
Collaborator Author

Infact same type of variables must be declared at same line, it enhances code readability 😎

therewasaguy added a commit to processing/p5.js that referenced this pull request May 26, 2021
* Fix drywet function bug https://github.com/processing/p5.js-sound/pulls/606
* Fix function name in documentation processing/p5.js-sound#603
* Fix repeat initialization of audioWorklet processing/p5.js-sound#593
* Fix issue with p5.Score that prevented parts from being passed as argument processing/p5.js-sound#543
* Fallback when AudioWorklet buffer array length is zero
 processing/p5.js-sound#542
* Remove extra callbacks for SoundFile cue
 processing/p5.js-sound#449
* Fix abnormal behaviour of audio output in p5.signal example section
  processing/p5.js-sound#476
* Fix error while playing sound through `p5.soundFile.play()` method
 processing/p5.js-sound#542

* Library is re-written in ESM ( ECMASCRIPT MODULES).
* Audio Nodes are written as `Class` style rather than being  written as `function` style.
* Library using features of es6 like `let` , `const` keywords and many other es6+ features.
* Testing architecture has been made more clean and robust.
* All above listed improvements are part of [Divyanshu's](https://github.com/endurance21) GSOC-20 Project , more details can be found in his [wrap-up-post](https://github.com/endurance21/GSOC-20-WrapUp).
* Improvements in pre-commit-hook action  processing/p5.js-sound#574

* Fixed broken anchor tags in p5.Audioln
 processing/p5.js-sound#450
* Fixed typo  in p5.fft documentation
  processing/p5.js-sound#433
* Added documentations for P5.oscillator APIs
 processing/p5.js-sound#420
* Improved documentation of p5.soundloop
  processing/p5.js-sound#437
* Fixed broken links in p5.panner3D documentations
 processing/p5.js-sound#465
* Typo improvements in p5.soundloop reference
 processing/p5.js-sound#557

* renames: outputVolume method and Main class processing/p5.js-sound#610

We have deprecated functionality in an effort to focus the API
* p5.Signal
* p5.SoundFile.processPeaks
therewasaguy added a commit to processing/p5.js that referenced this pull request May 26, 2021
Bug Fixes
* Fix drywet function bug https://github.com/processing/p5.js-sound/pulls/606
* Fix function name in documentation processing/p5.js-sound#603
* Fix repeat initialization of audioWorklet processing/p5.js-sound#593
* Fix issue with p5.Score that prevented parts from being passed as argument processing/p5.js-sound#543
* Fallback when AudioWorklet buffer array length is zero
 processing/p5.js-sound#542
* Remove extra callbacks for SoundFile cue
 processing/p5.js-sound#449
* Fix abnormal behaviour of audio output in p5.signal example section
  processing/p5.js-sound#476
* Fix error while playing sound through `p5.soundFile.play()` method
 processing/p5.js-sound#542

Intrastructure Improvements
* Library is re-written in ESM ( ECMASCRIPT MODULES).
* Audio Nodes are written as `Class` style rather than being  written as `function` style.
* Library using features of es6 like `let` , `const` keywords and many other es6+ features.
* Testing architecture has been made more clean and robust.
* All above listed improvements are part of [Divyanshu's](https://github.com/endurance21) GSOC-20 Project , more details can be found in his [wrap-up-post](https://github.com/endurance21/GSOC-20-WrapUp).
* Improvements in pre-commit-hook action  processing/p5.js-sound#574

Documentation Fixes
* Fixed broken anchor tags in p5.Audioln
 processing/p5.js-sound#450
* Fixed typo  in p5.fft documentation
  processing/p5.js-sound#433
* Added documentations for P5.oscillator APIs
 processing/p5.js-sound#420
* Improved documentation of p5.soundloop
  processing/p5.js-sound#437
* Fixed broken links in p5.panner3D documentations
 processing/p5.js-sound#465
* Typo improvements in p5.soundloop reference
 processing/p5.js-sound#557

Deprecations and Renames
* renames: outputVolume method and Main class processing/p5.js-sound#610
* We have deprecated functionality in an effort to focus the API
  * p5.Signal
  * p5.SoundFile.processPeaks
@therewasaguy therewasaguy mentioned this pull request May 26, 2021
3 tasks
therewasaguy added a commit to processing/p5.js that referenced this pull request May 26, 2021
Bug Fixes
* Fix drywet function bug https://github.com/processing/p5.js-sound/pulls/606
* Fix function name in documentation processing/p5.js-sound#603
* Fix repeat initialization of audioWorklet processing/p5.js-sound#593
* Fix issue with p5.Score that prevented parts from being passed as argument processing/p5.js-sound#543
* Fallback when AudioWorklet buffer array length is zero
 processing/p5.js-sound#542
* Remove extra callbacks for SoundFile cue
 processing/p5.js-sound#449
* Fix abnormal behaviour of audio output in p5.signal example section
  processing/p5.js-sound#476
* Fix error while playing sound through `p5.soundFile.play()` method
 processing/p5.js-sound#542
* Bring back p5.PeakDetect which was missing in 1.0.0

Intrastructure Improvements
* Library is re-written in ESM ( ECMASCRIPT MODULES).
* Audio Nodes are written as `Class` style rather than being  written as `function` style.
* Library using features of es6 like `let` , `const` keywords and many other es6+ features.
* Testing architecture has been made more clean and robust.
* All above listed improvements are part of [Divyanshu's](https://github.com/endurance21) GSOC-20 Project , more details can be found in his [wrap-up-post](https://github.com/endurance21/GSOC-20-WrapUp).
* Improvements in pre-commit-hook action  processing/p5.js-sound#574

Documentation Fixes
* Fixed broken anchor tags in p5.Audioln
 processing/p5.js-sound#450
* Fixed typo  in p5.fft documentation
  processing/p5.js-sound#433
* Added documentations for P5.oscillator APIs
 processing/p5.js-sound#420
* Improved documentation of p5.soundloop
  processing/p5.js-sound#437
* Fixed broken links in p5.panner3D documentations
 processing/p5.js-sound#465
* Typo improvements in p5.soundloop reference
 processing/p5.js-sound#557

Deprecations and Renames
* renames: outputVolume method and Main class processing/p5.js-sound#610
* We have deprecated functionality in an effort to focus the API
  * p5.Signal
  * p5.SoundFile.processPeaks
therewasaguy added a commit to processing/p5.js that referenced this pull request May 30, 2021
Bug Fixes
* Fix drywet function bug https://github.com/processing/p5.js-sound/pulls/606
* Fix function name in documentation processing/p5.js-sound#603
* Fix repeat initialization of audioWorklet processing/p5.js-sound#593
* Fix issue with p5.Score that prevented parts from being passed as argument processing/p5.js-sound#543
* Fallback when AudioWorklet buffer array length is zero
 processing/p5.js-sound#542
* Remove extra callbacks for SoundFile cue
 processing/p5.js-sound#449
* Fix abnormal behaviour of audio output in p5.signal example section
  processing/p5.js-sound#476
* Fix error while playing sound through `p5.soundFile.play()` method
 processing/p5.js-sound#542
* Bring back p5.PeakDetect which was missing in 1.0.0

Intrastructure Improvements
* Library is re-written in ESM ( ECMASCRIPT MODULES).
* Audio Nodes are written as `Class` style rather than being  written as `function` style.
* Library using features of es6 like `let` , `const` keywords and many other es6+ features.
* Testing architecture has been made more clean and robust.
* All above listed improvements are part of [Divyanshu's](https://github.com/endurance21) GSOC-20 Project , more details can be found in his [wrap-up-post](https://github.com/endurance21/GSOC-20-WrapUp).
* Improvements in pre-commit-hook action  processing/p5.js-sound#574

Documentation Fixes
* Fixed broken anchor tags in p5.Audioln
 processing/p5.js-sound#450
* Fixed typo  in p5.fft documentation
  processing/p5.js-sound#433
* Added documentations for P5.oscillator APIs
 processing/p5.js-sound#420
* Improved documentation of p5.soundloop
  processing/p5.js-sound#437
* Fixed broken links in p5.panner3D documentations
 processing/p5.js-sound#465
* Typo improvements in p5.soundloop reference
 processing/p5.js-sound#557

Deprecations and Renames
* renames: outputVolume method and Main class processing/p5.js-sound#610
* We have deprecated functionality in an effort to focus the API
  * p5.Signal
  * p5.SoundFile.processPeaks
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.

Sound starts to play when user visits the page , without users consent!
2 participants