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.

Sign up for GitHub

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
Merged

Support double precision processing #150

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1962870
Support double precision processing
baconpaul May 8, 2024
4ad1f78
Respond to jatin's review request
baconpaul May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 82 additions & 14 deletions src/wrapper/clap-juce-wrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,13 @@ class ClapJuceWrapper : public clap::helpers::Plugin<
info->flags = 0;
}

if (processor->supportsDoublePrecisionProcessing())
{
info->flags |= CLAP_AUDIO_PORT_SUPPORTS_64BITS;
info->flags |= CLAP_AUDIO_PORT_PREFERS_64BITS;
info->flags |= CLAP_AUDIO_PORT_REQUIRES_COMMON_SAMPLE_SIZE;
}

if (processor->getBus(!isInput, (int)index) != nullptr)
{
// this bus has a corresponding bus on the other side, so it can do in-place processing
Expand Down Expand Up @@ -1477,8 +1484,17 @@ class ClapJuceWrapper : public clap::helpers::Plugin<
*/
static constexpr uint32_t maxBuses = 128;
std::array<float *, maxBuses> busses{};
std::array<double *, maxBuses> doubleBusses{};
busses.fill(nullptr);

bool supportsDouble = processor->supportsDoublePrecisionProcessing();
bool hostCalledWithDouble = false;

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!

}

// we can't advance `n` until we know how many samples we're processing,
// so we'll increment it inside the loop.
for (int n = 0; n < numSamples;)
Expand Down Expand Up @@ -1548,7 +1564,15 @@ class ClapJuceWrapper : public clap::helpers::Plugin<
{
for (uint32_t ch = 0; ch < process->audio_outputs[idx].channel_count; ++ch)
{
busses[outputChannels] = process->audio_outputs[idx].data32[ch] + n;
if (supportsDouble && process->audio_outputs[idx].data64 != nullptr)
{
doubleBusses[outputChannels] = process->audio_outputs[idx].data64[ch] + n;
hostCalledWithDouble = true;
}
else
{
busses[outputChannels] = process->audio_outputs[idx].data32[ch] + n;
}
outputChannels++;
}
}
Expand All @@ -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!

{
if (ic == busses[inputChannels])
auto *ic = process->audio_inputs[idx].data64[ch] + n;
if (inputChannels < outputChannels)
{
// The buffers overlap - no need to do anything
if (ic == doubleBusses[inputChannels])
{
// The buffers overlap - no need to do anything
}
else
{
juce::FloatVectorOperations::copy(doubleBusses[inputChannels], ic,
numSamplesToProcess);
}
}
else
{
juce::FloatVectorOperations::copy(busses[inputChannels], ic,
numSamplesToProcess);
doubleBusses[inputChannels] = ic;
}
hostCalledWithDouble = true;
}
else
{
busses[inputChannels] = ic;
auto *ic = process->audio_inputs[idx].data32[ch] + n;
if (inputChannels < outputChannels)
{
if (ic == busses[inputChannels])
{
// The buffers overlap - no need to do anything
}
else
{
juce::FloatVectorOperations::copy(busses[inputChannels], ic,
numSamplesToProcess);
}
}
else
{
busses[inputChannels] = ic;
}
}
inputChannels++;
}
}

auto totalChans = juce::jmax(inputChannels, outputChannels);
juce::AudioBuffer<float> buffer(busses.data(), (int)totalChans, numSamplesToProcess);

if (processor->isSuspended())
if (hostCalledWithDouble)
{
buffer.clear();
juce::AudioBuffer<double> buffer(doubleBusses.data(), (int)totalChans,
numSamplesToProcess);

if (processor->isSuspended())
{
buffer.clear();
}
else
{
FIXME("Handle bypass and deactivated states")
processor->processBlock(buffer, midiBuffer);
}
}
else
{
FIXME("Handle bypass and deactivated states")
processor->processBlock(buffer, midiBuffer);
juce::AudioBuffer<float> buffer(busses.data(), (int)totalChans,
numSamplesToProcess);

if (processor->isSuspended())
{
buffer.clear();
}
else
{
FIXME("Handle bypass and deactivated states")
processor->processBlock(buffer, midiBuffer);
}
}


if (processorAsClapExtensions && processorAsClapExtensions->supportsOutboundEvents())
{
processorAsClapExtensions->addOutboundEventsToQueue(process->out_events, midiBuffer,
Expand Down
Loading