Maybe a bug in dsp::Oversampling?

ncthom · January 6, 2018, 3:10pm

In the snapToZero method on the Oversampling2TimesPolyphaseIIR oversampling engine in the new DSP module, there’s this following line:

github.com

WeAreROLI/JUCE/blob/master/modules/juce_dsp/processors/juce_Oversampling.cpp#L441


    }



    // Snap To Zero

    snapToZero (false);

}



void snapToZero (bool snapUpProcessing)

{

    if (snapUpProcessing)

    {

        for (auto channel = 0; channel < OversamplingEngine<SampleType>::buffer.getNumChannels(); channel++)

        {

            auto lv1 = v1Up.getWritePointer (channel);

            auto numStages = coefficientsUp.size();



            for (auto n = 0; n < numStages; n++)

                util::snapToZero (lv1[n]);

        }

    }

    else

    {

Why does this line iterate through the number of channels in OversamplingEngine<SampleType>::buffer.getNumChannels(); but then try to read from the v1up buffer? Would it not make more sense to iterate through v1up.getNumChannels()?

I ask because I’m using the oversampling class with the PolyphaseIIR filter in a plugin of mine, and it’s failing auval during the 1-channel tests because v1up gets configured with 1 channel but OversamplingEngine<SampleType>::buffer.getNumChannels(); resolves to 2 channels during a call to snapToZero. Then a call to v1Up.getWritePointer(1) goes through and hits a jassert because v1Up doesn’t have a channel at index 1.

I am able to build and validate successfully using the FIR Equiripple filter, so I don’t think the issue has to do with how I’m configuring the oversampling processor, but surely I could be wrong.

cc @IvanC or @jules maybe?

Thanks!

daniel · January 6, 2018, 7:08pm

I would say, the arguments in the constructor are swapped.
When the Oversampling2TimesPolyphaseIIR constructor calls it’s base class constructor, it calls
OversamplingEngine<SampleType> (2, numChannels), but it is defined as:
OversamplingEngine (size_t newNumChannels, size_t newFactor).

However, it seems to be fixed on develop, says git:

Base class constructor:

github.com

WeAreROLI/JUCE/blob/master/modules/juce_dsp/processors/juce_Oversampling.cpp#L40


{



/** Abstract class for the provided oversampling engines used internally in

the Oversampling class.

*/

template <typename SampleType>

class OversamplingEngine

{

public:

//===============================================================================

OversamplingEngine (size_t newNumChannels, size_t newFactor)

{

    numChannels = newNumChannels;

    factor = newFactor;

}



virtual ~OversamplingEngine() {}



//===============================================================================

virtual SampleType getLatencyInSamples() = 0;

size_t getFactor() { return factor; }

Called on master:

github.com

WeAreROLI/JUCE/blob/master/modules/juce_dsp/processors/juce_Oversampling.cpp#L285


*/

template <typename SampleType>

class Oversampling2TimesPolyphaseIIR : public OversamplingEngine<SampleType>

{

public:

//===============================================================================

Oversampling2TimesPolyphaseIIR (size_t numChannels,

                                SampleType normalizedTransitionWidthUp,

                                SampleType stopbandAttenuationdBUp,

                                SampleType normalizedTransitionWidthDown,

                                SampleType stopbandAttenuationdBDown) : OversamplingEngine<SampleType> (2, numChannels)

{

    auto structureUp = dsp::FilterDesign<SampleType>::designIIRLowpassHalfBandPolyphaseAllpassMethod (normalizedTransitionWidthUp, stopbandAttenuationdBUp);

    dsp::IIR::Coefficients<SampleType> coeffsUp = getCoefficients (structureUp);

    latency = static_cast<SampleType> (-(coeffsUp.getPhaseForFrequency (0.0001, 1.0)) / (0.0001 * 2 * double_Pi));



    auto structureDown = dsp::FilterDesign<SampleType>::designIIRLowpassHalfBandPolyphaseAllpassMethod (normalizedTransitionWidthDown, stopbandAttenuationdBDown);

    dsp::IIR::Coefficients<SampleType> coeffsDown = getCoefficients (structureDown);

    latency += static_cast<SampleType> (-(coeffsDown.getPhaseForFrequency (0.0001, 1.0)) / (0.0001 * 2 * double_Pi));



    for (auto i = 0; i < structureUp.directPath.size(); i++)

Called on develop:

github.com

WeAreROLI/JUCE/blob/develop/modules/juce_dsp/processors/juce_Oversampling.cpp#L134


*/

template <typename SampleType>

class Oversampling2TimesEquirippleFIR : public OversamplingEngine<SampleType>

{

public:

//===============================================================================

Oversampling2TimesEquirippleFIR (size_t numChannels,

                                 SampleType normalizedTransitionWidthUp,

                                 SampleType stopbandAttenuationdBUp,

                                 SampleType normalizedTransitionWidthDown,

                                 SampleType stopbandAttenuationdBDown) : OversamplingEngine<SampleType> (numChannels, 2)

{

    coefficientsUp = *dsp::FilterDesign<SampleType>::designFIRLowpassHalfBandEquirippleMethod (normalizedTransitionWidthUp, stopbandAttenuationdBUp);

    coefficientsDown = *dsp::FilterDesign<SampleType>::designFIRLowpassHalfBandEquirippleMethod (normalizedTransitionWidthDown, stopbandAttenuationdBDown);



    auto N = coefficientsUp.getFilterOrder() + 1;

    stateUp.setSize (static_cast<int> (numChannels), static_cast<int> (N));



    N = coefficientsDown.getFilterOrder() + 1;

    auto Ndiv2 = N / 2;

    auto Ndiv4 = Ndiv2 / 2;

Might be worth a cherry pick to master…?

Hope that helps…

mcmartin · January 6, 2018, 8:25pm

@daniel your last two links are not pointing to the same class. One is pointing to Oversampling2TimesPolyphaseIIR (on master), while the other one is pointing to Oversampling2TimesEquirippleFIR (on develop).

However you are right, @t0m pushed a commit to develop to fix the issue in Oversampling2TimesPolyphaseIIR (https://github.com/WeAreROLI/JUCE/commit/1ff97d3688dbdb67a61c7de1cafc58f3f48a24af), and that commit should maybe be cherry-picked to master.

daniel · January 6, 2018, 8:32pm

Indeed, I slipped to the wrong class, so Oversampling2TimesPolyphaseIIR on master is the only wrong one…
Oversampling2TimesPolyphaseFIR was always right, on master and develop…

My apologies…

ncthom · January 6, 2018, 10:11pm

Ah shoot, you’re right, thanks guys. I thought I had checked the develop branch.

Sorry for the noise.

t0m · January 8, 2018, 10:16am

Yeah, that looks like a safe cherry pick to the master branch. I’ll do that this morning.

Topic		Replies	Views
dsp::Oversampling setup General JUCE discussion	19	1935	July 23, 2021
Issue with oversampling General JUCE discussion	1	547	May 9, 2019
Correct usage of dsp::Oversampling class...? Development	0	447	March 10, 2021
[DSP module discussion] New Oversampling class Audio Plugins	46	9749	November 25, 2018
Oversampling typefilterHalfBandPolyphaseIIR returns 2 channels when 1 is specified General JUCE discussion	2	540	November 20, 2017

Maybe a bug in dsp::Oversampling?

Purchase

Discover

Learn

Support

About

Events

Maybe a bug in dsp::Oversampling?

Related topics

Purchase

Discover

Learn

Support

About

Events