DSP-Quantizer class in C++

Question

I have written a class to quantize values to arbitrary bit depths within a specified amplitude. It is meant to process one value at a time, and has two ways of quantizing values.

I wrote the class so that i could create objects of it as a property for another class, so that the quantization process can easily be included into other, more complex operations. This class I wrote, however, has some things I am quite sure about whether they were solved optimally.

My Header file, Quantizer.h, looks like this:

#pragma once
#include <cmath>
class CQuantizer
{
public:
    ~CQuantizer();

    enum qType {
        lin_midrise,
        lin_midtread,
        numberOfTypes
    };

    CQuantizer(CQuantizer::qType type = CQuantizer::lin_midtread,
        int nBits=4, float amplitude = 1.f);

    float processOneSample(float in);
    void setNBits(int bits);
    void setAmplitude(float amplitude);
    void setType(CQuantizer::qType type);


private:
    int m_nBits;

    float m_amplitude;

    CQuantizer::qType m_type;

    float processOneSampleLinMidtread(float in);
    float processOneSampleLinMidrise(float in);

    float (CQuantizer::*processSampleFunc)(float);
};

The first thing I am wondering about is the enum I used for the types: is it a good idea to include numberOfTypes as last element for later loops through the enum etc.?

Also, I saw somewhere else for a similar class that as some kind of switch, function pointers were used, which I did for the processOneSample-method. In this scenario, is it appropriate to do so?

Also, below I've included my implementation:

#include "Quantizer.h"


CQuantizer::~CQuantizer()
{
}

CQuantizer::CQuantizer(CQuantizer::qType type, int nBits, float amplitude)
{
    setType(type);
    setNBits(nBits);
    setAmplitude(amplitude);

}

float CQuantizer::processOneSample(float in)
{
    if (m_amplitude == 0.f) {
        // watch out for zero-amplitude
        return 0.f;
    }else{
        // use function specified by type
    return (this->*processSampleFunc)(in);
    }
}

void CQuantizer::setNBits(int bits)
{
    m_nBits = bits;
}

void CQuantizer::setAmplitude(float amplitude)
{
    m_amplitude = amplitude;
}

void CQuantizer::setType(CQuantizer::qType type)
{
    m_type = type;

    switch (m_type) {
    case CQuantizer::lin_midtread:
        processSampleFunc = &CQuantizer::processOneSampleLinMidtread;
        break;
    case CQuantizer::lin_midrise:
        processSampleFunc = &CQuantizer::processOneSampleLinMidrise;
        break;
    }

}

float CQuantizer::processOneSampleLinMidtread(float in)
{
    // upscaling
    float out = std::roundf( (in / m_amplitude) * powf(2.f, m_nBits - 1.f));

    // check for upper boundary
    if (out > powf(2.f, m_nBits - 1.f) - 1.f) {
        out = powf(2.f, m_nBits - 1.f) - 1.f;
    }

    // check for lower boundary
    if (out < -powf(2.f, m_nBits - 1.f)) {
        out = -powf(2.f, m_nBits - 1.f);
    }
    // downscaling
    return m_amplitude * out / powf(2.f, m_nBits - 1.f);
}

float CQuantizer::processOneSampleLinMidrise(float in)
{
    // upscaling
    float out = std::round((in / m_amplitude) * powf(2.f, m_nBits - 1.f) + 0.5f) - 0.5f;

    // check for upper boundary
    if (out > powf(2.f, m_nBits - 1.f) - 0.5f) {
        out = powf(2.f, m_nBits - 1.f) - 0.5f;
    }
    // check for lower boundary
    if (out < -powf(2.f, m_nBits - 1.f) + 0.5f) {
        out = -powf(2.f, m_nBits - 1.f) + 0.5f;
    }
    //downscaling
    return m_amplitude * out / powf(2.f, m_nBits - 1.f);
}

In my processing methods, i feel like there is too much going on: is there a smarter way for me to round these values and ensure they are within a set range?

(The effect I'd like to achieve with this class is that for a given amplitude, there can only be \$2^{\text{nBits}}\$ possible output values within a certain range.)

Edward · Accepted Answer · 2018-07-22 13:19:59Z

I see a number of things that may help you improve your program.

Omit empty destructor

The compiler will automatically generate a destructor that would, in this case be identical to the empty one your wrote. It's better to simply omit it and let the compiler do its thing.

Cleanly separate interface and implementation

In all, the files are fairly neatly divided, but I'd make a couple of changes. First, I'd move the #include <cmath> to the .cpp file because it's an implementation detail (and as you'll see a bit later, might not be necessary). Second, I'd remove the processOneSampleLinMidtread() and processOneSampleLinMidrise() from the .h file and make them static free-standing functions in the .cpp file. More on that in a moment, too.

Avoid comparing floating point numbers for equality

It's generally a good idea to avoid comparing floating point numbers for equality, even if it's only comparing with zero. See the oft-cited and still excellent What Every Computer Scientist Should Know About Floating-Point Arithmetic, by David Goldberg for a readable technical discussion for why this is so.

Reconsider the interface

At the moment, there's no way to read back out how many bits are in effect and no way to find out what the digital encoding would have been for a particular input float. I'd suggest a couple of changes to address that. First, it is probably useful to provide functions for reading back the publically settable parameters. Second, I'd suggest splitting the current processOneSample into two pieces: int encode(float n) which would return the binary encoding of the input n and float decode(int x) which would return the corresponding floating point value for the encoding x.

Reduce repeated calculations where practical

The expression powf(2.f, m_nBits - 1.f) is used very many times within the class, but it only depends on m_nBits and not on any other value. For that reason, it would make more sense to calculate it once and store the result.

Avoid floating point where practical

It's often faster to do integer mathematics rather than floating point. For that reason, and because we know that the result of powf(2.f, m_nBits - 1.f) is always going to be an integer, I'd suggest instead to calculate using the expression 1 << (bits - 1).

Use standard library features

Instead of the current code checking for upper and lower boundaries, it's much more succinctly expressed using std::clamp if you are using a C++17 compiler. If not, it's not hard to write your own version.

Use `const` where practical

The processOneSample() routine does not (and should not!) alter the underlying object, so it should be declared const.

float processOneSample(float in) const;

Think about future uses

Many people agree that the YAGNI principle is a sound one, and I agree with that. But there is a balance between only coding what's needed right now and anticipating at least the possibility that there may be future uses. An example in this case is using other kinds of digitizing algorithms such as A-law or \$\mu\$-law for audio digitizing. An alternative approach might be to have a base class (possibly virtual) and then derived classes that implement each kind of alternative encoding.

Think about whether you need a class at all

This may seem heretical for C++, but not everything needs to be an object. Really what this code is about is turning one floating point value into another. It may be that freestanding functions (with the appropriate parameters) might work as well. It depends on how often you anticipate changing the parameters of the base object vs. how many times processOneSample is used.

Consider using a modern constructor style

An alternative to this constructor:

CQuantizer::CQuantizer(CQuantizer::qType type, int nBits, float amplitude)
{
    setType(type);
    setNBits(nBits);
    setAmplitude(amplitude);
}

might be this:

CQuantizer::CQuantizer(CQuantizer::qType type, int nBits, float amplitude) :
    m_nBits{nBits},
    m_amplitude{amplitude},
    m_type{type}
{}

What is the difference from the constructor style you suggest to the one I used? — Jonas Schwarz
– Jonas Schwarz, Commented Jul 22, 2018 at 9:57
This includes a few answers to that question: stackoverflow.com/questions/926752/… — Edward
– Edward, Commented Jul 22, 2018 at 13:19

vnp · Accepted Answer · 2018-07-21 18:31:52Z

2

powf(2.f, m_nBits - 1.f) should be a class member, to be computed in setNBits.
Test for m_amplitude == 0.f also feels as belonging to setAmplitude. This of course requires raising an exception (since it is used in a constructor).

As a side note, I strongly advise against comparing floats for equality. A very small, yet non-zero, amplitude may yield unpleasant results.
Clipping is better expressed with std::min and std::max.

answered Jul 21, 2018 at 18:31

vnp

58.7k4 gold badges55 silver badges144 bronze badges

Add a comment |

Stack Exchange Network

DSP-Quantizer class in C++

2 Answers 2

Omit empty destructor

Cleanly separate interface and implementation

Avoid comparing floating point numbers for equality

Reconsider the interface

Reduce repeated calculations where practical

Avoid floating point where practical

Use standard library features

Use `const` where practical

Think about future uses

Think about whether you need a class at all

Consider using a modern constructor style

You must log in to answer this question.

Hot Network Questions

DSP-Quantizer class in C++

2 Answers 2

Omit empty destructor

Cleanly separate interface and implementation

Avoid comparing floating point numbers for equality

Reconsider the interface

Reduce repeated calculations where practical

Avoid floating point where practical

Use standard library features

Use const where practical

Think about future uses

Think about whether you need a class at all

Consider using a modern constructor style

You must log in to answer this question.

Related

Hot Network Questions

Use `const` where practical