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

StereoPannerNode sums input signal and outputs incorrect value #647

Closed
tambien opened this issue Aug 19, 2019 · 14 comments
Closed

StereoPannerNode sums input signal and outputs incorrect value #647

tambien opened this issue Aug 19, 2019 · 14 comments

Comments

@tambien
Copy link

tambien commented Aug 19, 2019

Here's a minimal example using standardized-audio-context.

const ac = new OfflineAudioContext(2, 10, 44100);
const panner = ac.createStereoPanner();
const sig = ac.createConstantSource();
sig.offset.value = 1;
sig.start(0);

sig.connect(panner).connect(ac.destination);

panner.pan.value = -1;

ac.startRendering().then(buffer => {
  console.log(buffer.getChannelData(0));
  console.log(buffer.getChannelData(1));
});

When you run this with the raw AudioContext it outputs:

image

But with stdAudioContext you get:

image

It seems like the ConstantSourceNode is getting connected twice in the input channel which makes the output be input * 2. At pan = 0, the output is 1 and 1 in both channels where it should be closer to 0.707 in both channels.

@chrisguttandin
Copy link
Owner

It took me a while to realize it but this is actually caused by a known limitation. The StereoPannerNode for Safari is implemented in a way which requires the number of channels of the input signal to be known. Therefore the channelCountMode is set to 'explicit' to achieve the same behavior across all browsers. (Back then I wrote an article about it, in case you like to know how I've done it.) Unfortunately I don't know of a way to detect the channel layout dynamically without using an AudioWorklet. But I'm happy for any ideas.

Could you work around that limitation and set the channelCount in your code? sig.channelCount = 1; will result in the expected behavior.

@tambien
Copy link
Author

tambien commented Aug 20, 2019

I think i could work with that limitation, i'm still a little confused about how i get the desired output.

When i used your suggestion, i still get L = 2 and R = 0 instead of L = 1 and R = 0:

sig.channelCount = 1;
sig.channelCountMode = "explicit";

I updated that codesandbox to illustrate the updated behavior.

@chrisguttandin
Copy link
Owner

Oh, sorry. My bad. The channelCount needs to be set on the StereoPanner.

@tambien
Copy link
Author

tambien commented Aug 20, 2019

Updated to setting the panner to channelCount = 1 and channelCountMode = 'explicit' and i get correct values: https://codesandbox.io/s/stereo-panner-sum-issue-zpm0u

It does seem counter intuitive to me that the stereo panner would be set to channelCount = 1 and then output a stereo output. Does channelCount and channelCountMode dictate only how 'input' connections are handled internally? i thought they also define the output channelCount.

@chrisguttandin
Copy link
Owner

Yes I totally agree that it is not so intuitive. The StereoPanner that comes with standardized-audio-context has a fixed channelCountMode set to 'explicit' which means the StereoPanner does perform an up/down-mix on the input signal to a get a signal with the number of channels defined by its channelCount property as input.

The output will always have two channels. There is no way to configure that.

I think the spec only indirectly says what I tried to summarize above. But it does say: "This attribute has no effect for nodes with no inputs." about the channelCountMode attribute which kind of implies that it only deals with the input.

@tambien
Copy link
Author

tambien commented Aug 22, 2019

This solution is working for me in both std-audio-context and the native AudioContext. going to close. thanks for your help!

@tambien tambien closed this as completed Aug 22, 2019
@tambien
Copy link
Author

tambien commented Jan 18, 2020

i've found a case where setting the channelCountMode and channelCount in such a way introduces a new issue. This is related to Tonejs/Tone.js#609.

If you chain two StereoPannerNodes, the panning of the first one is no longer respected.

Here's an example: https://codesandbox.io/s/std-issue-647-kmysk

If you remove the channelCount and channelCountMode it works as expected. Let me know what you think is the best way to deal with this.

@tambien tambien reopened this Jan 18, 2020
@chrisguttandin
Copy link
Owner

I think the behavior is correct. The first StereoPannerNode has a stereo output of two channels. But the second StereoPannerNode will down-mix the input to mono because the channelCount is set to 1. It will not apply any panning. Its output will again have two channels. Each channel will be the down-mixed input signal. That's how the first panning gets lost.

I hope this helps. Let me know if I misunderstood something.

@tambien
Copy link
Author

tambien commented Jan 18, 2020

Right the behavior does seem like the expected one to me as well.

My question is that i had added this line to Panner.ts in Tone.js to solve for the original bug, but now that same behavior has introduced this strange side effect that you can't chain two panners. I think if i remove that line, then i'll reintroduce this bug into Tone.js, but if i keep it then Panner is not chainable with itself.

@chrisguttandin
Copy link
Owner

Does it work if you set the channelCount to 2 here? If I'm not mistaken that would up-mix mono signals and keep stereo signals.

@tambien
Copy link
Author

tambien commented Jan 19, 2020

When i set it to 2, it reintroduces the original channel summing issue

You can see this behaves differently if you're using standardized-audio-context vs when you're using Chrome's AudioContext.

@chrisguttandin
Copy link
Owner

Ah, I see. Sorry that was a stupid idea.

Would it work for Tone.js if the user has to make that decision? Only the user knows what is connected to the Panner. Otherwise we would have to do some tracing to identify the input.

@chrisguttandin
Copy link
Owner

Hi @tambien, given that you closed the related issue, can we close this as well?

@tambien
Copy link
Author

tambien commented Jan 31, 2020

sure! i think the current solution is a bit of a bandaid, but seems like the issue won’t be fully resolved until safari implements the stereo panner node natively.

@tambien tambien closed this as completed Jan 31, 2020
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

No branches or pull requests

2 participants