Topic/atk fix speed of sound #319

joslloand · 2021-07-27T18:14:18Z

PR to address #317 (& ATK Quark 104)

Additional updates to ATK sc3-plugins README to align with ATK Quark README & in preparation for sc3-plugins v3.12.0 release.

FIX: supercollider#317 Update AtkFoa.speedOfSound == AtkHoa.speedOfSound == 343.0 m/s

Update for release, along with further updates to conform ATK sc3-plugins READMe to ATK Quark README.

dyfer · 2021-07-27T18:22:28Z

Hi @joslloand
I haven't looked at this closely, but wouldn't it be possible to have this value set from sclang?

source/ATK/AtkUGens.cpp

joslloand · 2021-07-27T18:56:00Z

I haven't looked at this closely, but wouldn't it be possible to have this value set from sclang?
@dyfer, ah, that might be better, as it would then be changeable?

Ah, this is a nice idea!

In practice, I don't think I'd want this to be an argument that the user supplies directly in the user interface to the UGen. Doing so could / would break existing code.

Instead, it should happen "under the hood" by setting AtkFoa.speedOfSound to the relevant value.

Ok, having had a little look, the changes to do so would have to happen in both AtkUGens.cpp here in sc3-plugins, and in ATK.sc found in the quark.

@joshpar, what do you think of this?

And.. I just noticed that, as we actually need normalized frequency in the calculation:

sc3-plugins/source/ATK/AtkUGens.cpp

Lines 1949 to 1950 in 444cef3

    
           float freq = 53.0 / distanceStart; 
        
           float wc = (twopi * freq) * SAMPLEDUR;

Could be updated to:

float wc = speedOfSound / distanceStart * SAMPLEDUR;

I'm feeling this would make things more simple, and clearer.

dyfer · 2021-07-27T19:02:02Z

Instead, it should happen "under the hood" by setting AtkFoa.speedOfSound to the relevant value.

Yes, that's what I was thinking

joslloand · 2021-07-27T19:14:32Z

Instead, it should happen "under the hood" by setting AtkFoa.speedOfSound to the relevant value.

Yes, that's what I was thinking

Good to know!

The slight danger is that users will need to make sure to update both sc3-plugins and the atk-sc3 quark. That shouldn't be a problem if the change is communicated in the update announcement..

@joshpar, how does this sound to you. (I'll add @mtmccrea, too.)

joshpar · 2021-07-27T19:19:33Z

yeah @joslloand - I think that's a cool idea

joslloand · 2021-07-27T19:31:51Z

yeah @joslloand - I think that's a cool idea

Ok, great!

As for actually making it happen, I'm super rusty on the plugin side and would need to have some guidance on getting it right.

In particular, once the speed of sound is set, it should be constant for the life of the UGen instantiation. I.e., it should be a fixed value that isn't modulate-able.

@dyfer, I may reach out over email for a quick chat on the matter....

joshpar · 2021-07-27T19:33:38Z

I can help as well if needed. Cheers!

…

On Jul 27, 2021, at 12:32, Joseph Anderson ***@***.***> wrote: yeah @joslloand <https://github.com/joslloand> - I think that's a cool idea Ok, great! As for actually making it happen, I'm super rusty on the plugin side and would need to have some guidance on getting it right. In particular, once the speed of sound is set, it should be constant for the life of the UGen instantiation. I.e., it should be a fixed value that isn't modulate-able. @dyfer <https://github.com/dyfer>, I may reach out over email for a quick chat on the matter.... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#319 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANHLALDVOQLR4FNPU2IZLTZ4CTFANCNFSM5BCYNZBA>.

joslloand · 2021-07-27T19:36:10Z

I can help as well if needed. Cheers!

Ok, great! I think I see what needs to happen... so that's fine. (However, since, I haven't tried to compile sc3-plugins on this machine in so long, I'm expecting a task!)

affects: FoaNFC, FoaProximity NOTE: interface requires update to atk-sc3 quark ATK: speed of sound as init value

joslloand · 2021-07-27T22:46:43Z

Ok, I've just made an updated push.

The updated atk-sc3 quark branch is here: https://github.com/ambisonictoolkit/atk-sc3/tree/refactor-foa-nfc-proximity

What I haven't done is tried to compile... so testing is required....

dyfer · 2021-08-06T17:18:52Z

@joslloand we've just added automated building with github actions. If you rebase your PR on the current main, you'll see an automated build in this PR, where you can also download the build artifacts to test. However, building locally is definitely recommended :)

source/ATK/AtkUGens.cpp

joslloand · 2021-08-10T01:46:20Z

@joslloand we've just added automated building with github actions. If you rebase your PR on the current main, you'll see an automated build in this PR, where you can also download the build artifacts to test. However, building locally is definitely recommended :)

Thanks @dyfer! I didn't realize this... very convenient. (I ended up doing a merge rather than a rebase.)

I've done some tests, and both HOA and FOA are now matched. Hurrah! So the updates are working now.

... so... I think we're ready for a merge

joslloand · 2021-08-31T00:06:29Z

@joslloand we've just added automated building with github actions. If you rebase your PR on the current main, you'll see an automated build in this PR, where you can also download the build artifacts to test. However, building locally is definitely recommended :)

Thanks @dyfer! I didn't realize this... very convenient. (I ended up doing a merge rather than a rebase.)

I've done some tests, and both HOA and FOA are now matched. Hurrah! So the updates are working now.

... so... I think we're ready for a merge

@dyfer, @joshpar just picking this up again after some time away....

... I think this is all ready to go. The tests (matching NFE results w/ the HOA implementation) have passed.

So, I think this is ready to be closed and merged.

telephon · 2021-09-16T07:12:20Z

@joshpar would you also agree that it is ready to go?

joslloand · 2021-09-16T17:51:27Z

@joshpar would you also agree that it is ready to go?

@telephon I had a conversation with @dyfer yesterday, regarding backwards compatibility and resulting behavior given installs with mismatched quark and sc3-plugins.

I have a possible solution I'll be testing today that should resolve performance issues if the class and plugin aren't aligned. (I expect a simple check on inputs will do the job.)

If this does work, I'll go ahead and push this change... then I think we'll really be ready to go!

affects: FoaNFC, FoaProximity NOTE: resolves previously required update to atk-sc3 quark If AtkFoa.speedOfSound is not passed, plugins use 333.0 mps, which is equivalent to coeffiencts used in previous sc3-plugins (<=v3.11.1)

joslloand · 2021-09-16T21:45:58Z

@telephon I had a conversation with @dyfer yesterday, regarding backwards compatibility and resulting behavior given installs with mismatched quark and sc3-plugins.

I have a possible solution I'll be testing today that should resolve performance issues if the class and plugin aren't aligned. (I expect a simple check on inputs will do the job.)

If this does work, I'll go ahead and push this change... then I think we'll really be ready to go!

Ok, here's the fix: 8bc45e4

Which meets these cases, testing cases for matching and unmatching quark AND plugins:

matching: latest test quark (refactor-foa-nfc-proximity) AND latest test plugins (topic/atk-fix-speedOfSound)
--> pass: speed of sound updated to 343 m/s
NOT matching: previous released quark (v5.0.3) AND latest test plugins (topic/atk-fix-speedOfSound)
--> pass: speed of sound remains at 333 m/s [ plugins supply missing speed of sound ]
NOT matching: latest test quark (refactor-foa-nfc-proximity) AND previous plugins (v3.11.1)
--> pass: speed of sound remains at 333 m/s [ plugins cutoff frequency sets equivalent speed of sound, value supplied by class is ignored ]

@joshpar, @dyfer, @telephon, so now I'm thinking we're ready to go with this.

(Finally!)

dyfer · 2021-09-17T00:14:37Z

This seems to work as advertised, thank you!

@joshpar @telephon is it OK to leave that merge commit in?

joshpar · 2021-09-17T00:15:37Z

SHIP IT!

…

On Sep 16, 2021, at 17:14, Marcin Pączkowski ***@***.***> wrote: This seems to work as advertised, thank you! @joshpar <https://github.com/joshpar> @telephon <https://github.com/telephon> is it OK to leave that merge commit in? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#319 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANHLAG43567L2YNH5MCVDUCKB7RANCNFSM5BCYNZBA>.

dyfer

Shippin' it

dyfer · 2021-09-17T00:19:36Z

@joshpar is your change request satisfied? :)

@joshpar

@joshpar said "SHIP IT!"

joslloand · 2021-09-17T18:03:07Z

Thanks everyone!

joslloand added 2 commits July 27, 2021 10:48

ATK fix: FoaProximity, FoaNFC conform FOA speed of sound to HOA

1d4cd02

FIX: supercollider#317 Update AtkFoa.speedOfSound == AtkHoa.speedOfSound == 343.0 m/s

update: ATK README sc3-plugins v3.12.0

2ecf81e

Update for release, along with further updates to conform ATK sc3-plugins READMe to ATK Quark README.

joshpar previously requested changes Jul 27, 2021

View reviewed changes

source/ATK/AtkUGens.cpp Outdated Show resolved Hide resolved

ATK refactor: add user settable speed of sound

b00a7a6

affects: FoaNFC, FoaProximity NOTE: interface requires update to atk-sc3 quark ATK: speed of sound as init value

mtmccrea reviewed Aug 9, 2021

View reviewed changes

source/ATK/AtkUGens.cpp Show resolved Hide resolved

Merge branch 'supercollider:main' into topic/atk-fix-speedOfSound

23e6544

joslloand requested a review from joshpar August 10, 2021 19:52

dyfer approved these changes Sep 17, 2021

View reviewed changes

dyfer merged commit fe7ef28 into supercollider:main Sep 17, 2021

joslloand deleted the topic/atk-fix-speedOfSound branch September 17, 2021 18:09

joslloand mentioned this pull request Sep 17, 2021

FOA speed of sound ambisonictoolkit/atk-sc3#104

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Topic/atk fix speed of sound #319

Topic/atk fix speed of sound #319

joslloand commented Jul 27, 2021

dyfer commented Jul 27, 2021

joslloand commented Jul 27, 2021 •

edited

Loading

dyfer commented Jul 27, 2021

joslloand commented Jul 27, 2021

joshpar commented Jul 27, 2021

joslloand commented Jul 27, 2021

joshpar commented Jul 27, 2021 via email

joslloand commented Jul 27, 2021

joslloand commented Jul 27, 2021

dyfer commented Aug 6, 2021

joslloand commented Aug 10, 2021

joslloand commented Aug 31, 2021

telephon commented Sep 16, 2021

joslloand commented Sep 16, 2021

joslloand commented Sep 16, 2021

dyfer commented Sep 17, 2021

joshpar commented Sep 17, 2021 via email

dyfer left a comment

dyfer commented Sep 17, 2021

joslloand commented Sep 17, 2021

Topic/atk fix speed of sound #319

Topic/atk fix speed of sound #319

Conversation

joslloand commented Jul 27, 2021

dyfer commented Jul 27, 2021

joslloand commented Jul 27, 2021 • edited Loading

dyfer commented Jul 27, 2021

joslloand commented Jul 27, 2021

joshpar commented Jul 27, 2021

joslloand commented Jul 27, 2021

joshpar commented Jul 27, 2021 via email

joslloand commented Jul 27, 2021

joslloand commented Jul 27, 2021

dyfer commented Aug 6, 2021

joslloand commented Aug 10, 2021

joslloand commented Aug 31, 2021

telephon commented Sep 16, 2021

joslloand commented Sep 16, 2021

joslloand commented Sep 16, 2021

dyfer commented Sep 17, 2021

joshpar commented Sep 17, 2021 via email

dyfer left a comment

Choose a reason for hiding this comment

dyfer commented Sep 17, 2021

joslloand commented Sep 17, 2021

joslloand commented Jul 27, 2021 •

edited

Loading