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

Allow to play/draw one channel specified by parameter #689

Merged
merged 3 commits into from
Apr 2, 2016
Merged

Allow to play/draw one channel specified by parameter #689

merged 3 commits into from
Apr 2, 2016

Conversation

whenov
Copy link
Contributor

@whenov whenov commented Apr 1, 2016

No description provided.


for (var c = 0; c < channels; c++) {
this.splitter.connect(this.merger, channel === -1 ? c : channel, c);
}
Copy link
Owner

Choose a reason for hiding this comment

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

if (channel > -1) {
    this.splitter.connect(this.merger, channel);
} else {
    for (var c = 0; c < channels; c++) {
        this.splitter.connect(this.merger, c);
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to omit the third parameter also, but the output of splitter is only connected to the first channel of the merger (tested on newest Chrome/Firefox).

Although the last two parameters are marked as optional in the document of AudioNode.connect, the default behaviour is not described.

@katspaugh
Copy link
Owner

Nice! Does it add much overhead in terms of performance, though?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 4.756% when pulling 47a4217 on whenov:channel into 3e7b67e on katspaugh:master.

@whenov
Copy link
Contributor Author

whenov commented Apr 1, 2016

@katspaugh Although I haven't tested its performance (and I haven't considered how to do it), the added logic seems not to include costly overhead.

But I'm not sure about the splitter/merger nodes, whose performance are dependent on browser implementations. If this channel-switch function is not required by most users, we can directly connect analyser to gainNode when channel === -1, which however decreases a little readability.

@katspaugh
Copy link
Owner

Could you measure CPU load/memory usage with and without your changes? You can simply open the default OS activity monitor and watch the browser process.

@whenov
Copy link
Contributor Author

whenov commented Apr 1, 2016

@katspaugh Good idea, will do it tomorrow.

@whenov
Copy link
Contributor Author

whenov commented Apr 2, 2016

@katspaugh I tested its performance using Chrome's profiling tool, the increased execution time is negligible.

@katspaugh katspaugh merged commit 93f48f2 into katspaugh:master Apr 2, 2016
@katspaugh
Copy link
Owner

Cool, merged! Thanks!

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.

3 participants