Fix rotation matrix inversion issue in HOA.

[drewbitllama]

Forgot to build library.

[drewbitllama]

Modified tests in order for changes to work and be tested properly.

[hoch]

@drewbitllama Can you take a look at Travis CI build? It seems to be failing on the rotation test.

[drewbitllama]

I'll tal tomorrow.

On Mon, Mar 5, 2018 at 3:42 PM, Hongchan Choi notifications@github.com wrote:

@drewbitllama https://github.com/drewbitllama Can you take a look at Travis CI build? It seems to be failing on the rotation test.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/omnitone/pull/87#issuecomment-370607902, or mute the thread https://github.com/notifications/unsubscribe-auth/AGDsNoXteMKu3y25Squ8CkjQxbcv0cX4ks5tbc12gaJpZM4SMXkM .

[drewbitllama]

This patch is passing on my machine. Is the TravisCL setup to use the latest tests even when we are updating the test files?

[hoch]

It might be numeric differences between platforms. If that's the case we might want to loosen the threshold a bit.

[hoch]

"Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves."

I see this message in the log.

[hoch]

I just confirmed that this PR does not pass the test locally.

var forward = [
      -Math.sin(azimuth) * Math.cos(elevation), Math.sin(elevation),
      -Math.cos(azimuth) * Math.cos(elevation)
];

The added minus signs broke the test and my failed log is identical to the one from Travis CI. Needs further investigation...

[hoch]

@drewbitllama Okay, here's how we got here:

1. The 3rd order rotation formula changed and merged.
2. The PR broke the build because the built library was not included in the PR.
3. The PR in 2 was reverted.
4. The second PR was introduced the fix the test accordingly.
5. However, this PR also could not pass the test because it did not have the change in 1.

So when I sum 1 and 4 into one PR and it passes the test, but now we have a bigger issue. The rotation direction has been reversed. This is a breaking change and I suggest we should not do this at this point. Please see this PR for the demonstration.

Could you clarify why we need this so much?

[drewbitllama]

If we don’t do this (currently the case) we get opposite behaviors for foarenderer.setmatrix and hoarenderer.setmatrix (the rotation goes the other way) On Tue, Mar 13, 2018 at 2:10 PM Hongchan Choi notifications@github.com wrote:

@drewbitllama https://github.com/drewbitllama Okay, here's how we got here:

1. The 3rd order rotation formula changed and merged https://github.com/GoogleChrome/omnitone/pull/86/files.
2. The PR broke the build because the built library was not included in the PR.
3. The PR in 2 was reverted.
4. The second PR https://github.com/GoogleChrome/omnitone/pull/87/files was introduced the fix the test accordingly.
5. However, this PR also could not pass the test because it did not have the change in 1.

So when I sum 1 and 4 into one PR and it passes the test, but now we have a bigger issue. The rotation direction has been reversed. This is a breaking change and I suggest we should not do this at this point.

Could you clarify why we need this so much?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/omnitone/pull/87#issuecomment-372819574, or mute the thread https://github.com/notifications/unsubscribe-auth/AGDsNqklhbKqqs0or9PuWab-8QT98Sbrks5teDXDgaJpZM4SMXkM .

[hoch]

I see. That's much bigger issue. So are you suggesting we bite the bullet and make this change? I guess this issue was found while you're dynamically switching between FOA/HOA renderer?

[drewbitllama]

That’s correct. So we need to do this so the renderers behave the same way. On Wed, Mar 14, 2018 at 9:05 AM Hongchan Choi notifications@github.com wrote:

I see. That's much bigger issue. So are you suggesting we bite the bullet and make this change? I guess this issue was found while you're dynamically switching between FOA/HOA renderer?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GoogleChrome/omnitone/pull/87#issuecomment-373077337, or mute the thread https://github.com/notifications/unsubscribe-auth/AGDsNvbX2rLtRwXlYjmbV_H53-PdOM29ks5teT_bgaJpZM4SMXkM .

[hoch]

Close this in favor of https://github.com/GoogleChrome/omnitone/pull/89.