Provide a complete sample
For future reference, it's much easier to review code if it's complete. That is, instead of just writing a loop, you could instead wrap in a small but complete program. Your code refers to SampleSize
, bufferSize
, and micBuffer
but none of those are defined in the sample you've posted.
Establish and use consistent naming
Some of your variable names, such as SampleSize
are capitalized, while others, such as bufferSize
are not. One common C++ convention is to use lowercase names for variables (as with bufferSize
) and capitalized names for classes and structure types such as Vector
. There is no significance to these conventions to the compiler, but a consistently applied convention makes it easier for others to read and understand your code.
Avoid the use of hard-coded "magic numbers"
There are numbers, such as 960
and 32768.0f
in your code that don't have an obvious meaning. Better would be to declare them as constants with a meaningful name, or at least to have more explanatory comments.
Eliminate unused variables
This code declares a variable Test
at the top of the sample, and then redefines it within the scope of the if
statement. The second declaration of Test
is never used within the code. TestChannels
and TestSize
are also unused within this code. You can (and should) eliminate unused variables like this.Your compiler is smart enough to help you find this kind of problem if you know how to ask it to do so.
Remove loop invariants from the loop
A loop invariant is a value used within a loop construct that doesn't change. Right now your code checks the value of MixMono
and left
for every iteration through the loop. Since they don't change within the loop, it's better to check them just once and then select a loop, since you're looking for performance.
As an example of this, here's what your MixMono
loop would look like when extracted:
if (MixMono)
{
for (int i = 0; i < bufferSize; ++i)
{
int s = micBuffer[i] * scaleFactor; //32bit Float to 16Bit Int
ChannelCheck++;
copy += s;
if (ChannelCheck == 2)
{
Test[Tindex] = copy / 2;
copy = 0;
Tindex++;
ChannelCheck = 0;
}
}
}
Now that it's extracted, it's easier to see what's happening and to identify inefficiencies. One thing to note is that we're doing two multiplies, an add and a division for each mono sample. The math is currently (left * 32768 + right * 32768)/2
with each channel being a float
and the result being an int16_t
. Mathematically, this is the same as (left+right)*16384
so as long as you can assure that the samples are within range and that you always have an even number of samples, you can simplify this loop to this:
const float scaleFactor = 32768;
const float halfScaleFactor = scaleFactor/2;
if (MixMono)
{
for (int i = 0; i < bufferSize; i+=2)
{
Test[Tindex++] = (micBuffer[i] + micBuffer[i+1]) * halfScaleFactor;
}
}
The other loops can be similarly compressed and the s
, ChannelCheck
and copy
variables completely eliminated:
else if (left)
{
for (int i = 0; i < bufferSize; i+=2)
{
Test[Tindex++] = micBuffer[i] * scaleFactor;
}
}
else
{
for (int i = 0; i < bufferSize; ++i)
{
Test[Tindex++] = micBuffer[i] * scaleFactor;
}
}
Consider using pointers
Depending on the architecture and compiler, using pointers instead of array indexing may be faster. If your application requires more speed than indexing can provide, you might want to test using pointers and see if that will allow your code to meet its performance requirements. As always, you should prefer clearly written code and only attempt such speedups if you have measured the performance of the code and found it insufficient.
Consider using a struct
Another possibility is to define a struct
like this:
struct Sample
{
float left;
float right;
};
and then processing your micBuffer
data by casting it as an array of such structures. Your code might be easier to read and understand this way and may be faster. An example is this:
Sample *micBufferEnd = (Sample *)&micBuffer[bufferSize];
if (MixMono)
{
for (Sample *s = (Sample *)micBuffer; s < micBufferEnd; ++s)
{
Test[Tindex++] = (s->left + s->right) * halfScaleFactor;
}
}
This works by treating the samples a pair at a time. When the ++s
is executed at the end of each loop, it advances the pointer by the size of the structure which happens to be the size of two floats.
Note that as @JerryCoffin noted in his comment, that this is not guaranteed to work because the compiler is free to insert padding between values. For instance, on a 64-bit machine, the compiler may decide to align each member on a 64-bit boundary, possibly resulting in a gap between member items. Compilers often implement something like a #pragma pack
to force the behavior used here, but that's not portable, and it's even possible that it could change from version to version of an individual compiler. An alternative, also noted in the comments, was to define the structure differently. One way to do it might be this:
class Sample
{
private:
float chan[2];
public:
float left() const { return chan[0]; }
float right() const { return chan[1]; }
};
Using it would be almost identical except that left
and right
are now member functions rather than data items.
Sample *micBufferEnd = (Sample *)&micBuffer[bufferSize];
if (MixMono)
{
for (Sample *s = (Sample *)micBuffer; s < micBufferEnd; ++s)
{
Test[Tindex++] = (s->left() + s->right()) * halfScaleFactor;
}
}