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

Wip/25 add gain node #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

muuki88
Copy link

@muuki88 muuki88 commented Feb 19, 2017

I added a minimal working solution for a gainNode. Tested on Google Chrome 56.0.2924.87 (64-bit) / Ubuntu 16.10. See #25

  • Make gainNode configurable
  • Provide a test
  • Test on more browsers

@muuki88 muuki88 mentioned this pull request Feb 19, 2017
3 tasks
@muuki88
Copy link
Author

muuki88 commented Feb 19, 2017

Sorry, I created the PR from the wrong branch on my fork.

I added a gain option to the IStreamClientOptions. Testing with the demo/typescript-project wasn't possible as it depends an actual deployed version. How to you test this locally?

@Gugic
Copy link
Contributor

Gugic commented Feb 20, 2017

To test if typescript-project works fine, you can use npm pack (which creates .tgz) + npm install (local tgz)

Considering your pull request: i thought that it should be possible to tune gainNode value "in runtime", not only during initialization. To make "mute" function or something like that. Correct me if i wrong.

And thank you for your work.

@muuki88
Copy link
Author

muuki88 commented Feb 22, 2017

i thought that it should be possible to tune gainNode value "in runtime", not only during initialization. To make "mute" function or something like that

Sounds like a good idea :) Will work on that ASAP

And thanks for the hint on how to test this locally. Wouldn't it be possible to reference it by going up the directory a few steps?

@Gugic
Copy link
Contributor

Gugic commented Feb 22, 2017

This is probably possible, but partially ignores node mechanism of modules resolve. You may be can also try this (https://docs.npmjs.com/cli/link)

Considering making it configurable, i think you can just add method like getGainNode to interface that will return existing gain node and that will allow users to manage it in any desired way. (probably we can also remove new 'gain' property from option).

What do you think about that?

@muuki88
Copy link
Author

muuki88 commented Feb 22, 2017

Considering making it configurable, i think you can just add method like getGainNode to interface that will return existing gain node and that will allow users to manage it in any desired way. (probably we can also remove new 'gain' property from option)

Sounds good to me. Will change ASAP

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.

2 participants