Skip to content

Conversation

@andreasjeppsson
Copy link

This fixes an issue in Chrome where the audio in some cases gets stuck jumping from left to right channel.

Examples:
issue:
http://labs.plan8.se/panner_test/?audioparam=0
fix:
http://labs.plan8.se/panner_test/?audioparam=1

@mrdoob
Copy link
Owner

mrdoob commented Jun 10, 2017

/ping @hoch

listener.forwardZ.value = orientation.z;
listener.upX.value = up.x;
listener.upY.value = up.y;
listener.upZ.value = up.z;
Copy link

Choose a reason for hiding this comment

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

I am not really sure what this is trying to fix. What was the issue and how this can be a solution?

Copy link
Owner

Choose a reason for hiding this comment

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

/ping @andreasplan8

@andreasjeppsson
Copy link
Author

@hoch @mrdoob
The issue is that I get a flutter/tremolo effect where the audio is not panning smoothly when using setValueAtTime() as you can here in the example.

Setting the value directly on the audioParam seems to fix it. Not sure why.

(I'm testing with Chrome 59.0.3071.86)

@hoch
Copy link

hoch commented Jun 12, 2017

@andreasplan8

flutter/tremolo

This means you're changing the parameter value too fast. The parameter setter (=) in Chrome implicitly applies smoothing (leaky integration or dezipper) over the certain time constant, so those modulation artifacts are smoothed out.

Since the implicit smoothing is actually deprecated from the spec, I recommend to use the automation method. For example,

// To smooth parameter value over the frame-to-frame interval at 60fps.
const tau = 60/1000;
const later = this.context.currentTime + tau;

listener.positionX.linearRampToValueAtTime(position.x, later);
listener.positionY.linearRampToValueAtTime(position.y, later);
listener.positionZ.linearRampToValueAtTime(position.z, later);

More typing, but future-proof.

@mrdoob
Copy link
Owner

mrdoob commented Jun 12, 2017

const tau = 60/1000;

Hardcoding this is probably not a good idea?

@hoch
Copy link

hoch commented Jun 12, 2017

Yeah, my code was just an example. If you have a variable that keeps the frame interval, it is probably good enough.

Or you can calculate it from the time stamp from rAF() call?

@mrdoob
Copy link
Owner

mrdoob commented Jun 12, 2017

Déjà vu. #9845 (comment) 😁

@hoch
Copy link

hoch commented Jun 12, 2017

Heh. I also vaguely remembered we talked about this before. :)

@andreasjeppsson
Copy link
Author

@mrdoob @hoch Ok, but isn't it better just setting the value instead of scheduling it?

@hoch
Copy link

hoch commented Jun 20, 2017

@andreasplan8 It depends on the interval of setting values. If the interval is too coarse, then you'll hear glitches. Also I think what we want here is a smoothing transition between coordinates, thus the linear ramping makes sense. Chrome does the internal interpolation when the setter is used, which is very similar to the code example above in terms of its behavior, but that is a non-compliant to the spec.

@mrdoob mrdoob added this to the r91 milestone Jan 18, 2018
@mrdoob mrdoob modified the milestones: r91, r92 Mar 14, 2018
@mrdoob mrdoob modified the milestones: r92, r93 Apr 18, 2018
@hoch
Copy link

hoch commented May 16, 2018

@mrdoob Someone needs to pick this up? Or should we just close?

@mrdoob mrdoob modified the milestones: r93, r94 May 22, 2018
@mrdoob mrdoob modified the milestones: r94, r95 Jun 27, 2018
@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 27, 2018

I would like to work on this if there is room for improvement. At the moment, three.js updates the transformation of panner nodes like this:

panner.setPosition( position.x, position.y, position.z );
panner.setOrientation( orientation.x, orientation.y, orientation.z );

And this is the code for AudioListener:

if ( listener.positionX ) {
listener.positionX.setValueAtTime( position.x, this.context.currentTime );
listener.positionY.setValueAtTime( position.y, this.context.currentTime );
listener.positionZ.setValueAtTime( position.z, this.context.currentTime );
listener.forwardX.setValueAtTime( orientation.x, this.context.currentTime );
listener.forwardY.setValueAtTime( orientation.y, this.context.currentTime );
listener.forwardZ.setValueAtTime( orientation.z, this.context.currentTime );
listener.upX.setValueAtTime( up.x, this.context.currentTime );
listener.upY.setValueAtTime( up.y, this.context.currentTime );
listener.upZ.setValueAtTime( up.z, this.context.currentTime );
} else {
listener.setPosition( position.x, position.y, position.z );
listener.setOrientation( orientation.x, orientation.y, orientation.z, up.x, up.y, up.z );
}

I have tested this setup multiple times in different browsers with three.jss web audio examples and it seems to work. Is this approach the recommended way? If not, what would be the ideal solution?

@hoch
Copy link

hoch commented Jun 27, 2018

In Chrome, using setPosition() and using position{X,Y,X}.setValueAtTime() are internally identical. So I wouldn't bother to hand-roll the method as shown above. Also I suspect that Chrome is the only browser that implemented AudioParam in PannerNode.

One thing to think about is: Chrome now does not do the internal smoothing between position. If you want to smooth the transition you'll have to use linearRampToValueAtTime() or the other automation methods. So if the transition is too fast, the audio might end up with glitches. This is true theoretically, but I have not seen any bug report on it yet.

I think simply using setPosition and setOrientation might be good enough until all the browsers have the support for AudioParams in PannerNode.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 27, 2018

Many thanks for the information!

If you want to smooth the transition you'll have to use linearRampToValueAtTime() or the other automation methods.

Would it be okay to use setTargetAtTime() and a constant value for the parameter timeConstant? three.js does this already at different place e.g. for adjusting gain values:

this.gain.gain.setTargetAtTime( value, this.context.currentTime, 0.01 );

We did not find a nice solution so far in order to pass the real frametime to the audio entities. I guess this is one reasons why we are not using linearRampToValueAtTime() yet.

@hoch
Copy link

hoch commented Jun 27, 2018

Using a constant end time (now + 20ms) for linear ramping is sort of acceptable as well. For the real-time/interactive application, it will like a leaky integrator. It depends on your application.

Using setTargetAtTime() is a theoretically correct answer for this use case, but please keep in mind its end time because it does last forever unless the connected source stops.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 29, 2018

Closing in favor of #14393.

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.

4 participants