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

Support double precision processing #150

Merged
merged 2 commits into from
May 8, 2024

Conversation

baconpaul
Copy link
Collaborator

If supprotsDoublePrecision in the processor is true, advertise FLOAT64 audio ports and adjust the processing loop.

Tested by running Airwindows Consolidated as CLAP in reaper (which supports double) and Bitwig (which does not) and seeing appropriate behavior.

If `supprotsDoublePrecision` in the processor is true, advertise
FLOAT64 audio ports and adjust the processing loop.

Tested by running Airwindows Consolidated as CLAP in reaper
(which supports double) and Bitwig (which does not) and seeing
appropriate behavior.
@baconpaul baconpaul requested a review from jatinchowdhury18 May 8, 2024 15:44
@baconpaul
Copy link
Collaborator Author

@jatinchowdhury18 if you wouldn't mind a peek I would appreciate it!

Copy link
Collaborator

@jatinchowdhury18 jatinchowdhury18 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@@ -1559,40 +1583,84 @@ class ClapJuceWrapper : public clap::helpers::Plugin<
{
for (uint32_t ch = 0; ch < process->audio_inputs[idx].channel_count; ++ch)
{
auto *ic = process->audio_inputs[idx].data32[ch] + n;
if (inputChannels < outputChannels)
if (supportsDouble && process->audio_outputs[idx].data64 != nullptr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it makes a difference logically, but it seems a little bit odd from the "reader's" perspective to be checking the process->audio_outputs pointers, when the body of this if statement is mostly concerned with process->audio_inputs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh no that’s a mistake for sure! I’ll fix and merge thanks!

@baconpaul baconpaul merged commit a3227c5 into free-audio:main May 8, 2024
19 checks passed
@baconpaul baconpaul deleted the double-process-149 branch May 8, 2024 17:35

if (supportsDouble)
{
doubleBusses.fill(nullptr);
Copy link

Choose a reason for hiding this comment

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

Neither this fill nor busses.fill(nullptr) should be necessary since both busses and doubleBusses are already zero-initialized with {}.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So you are absolutely correct we init busses with {}

I thought we had not used the {} form, hence the fill, so conditionalized the double busses fill so it is unused.

I think our best bet is to remove the {} on double busses and keep the fill but it's not that much assembly difference. Basically I wanted case three here: https://godbolt.org/z/ev7TzrKGj but I accidentally wrote case 4!

Copy link

Choose a reason for hiding this comment

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

Sounds like a good idea.

For busses, zero-init with {} and no fill, and for doubleBusses, don't use zero-init and do a conditional fill. That would probably generate the best assembly.

While it isn't a huge assembly difference, it's inside the process method and calling fill is O(N), so it would be good to fix this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup that’s why I put the fill in an if. Just I didn’t see the {} above!

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