daniel
June 28, 2017, 12:15pm
1
Hi everyone,
Can we get another asserts for numSamples being positive?
jassert (isPositiveAndBelow (destChannel, numChannels));
jassert (destStartSample >= 0 && numSamples >= 0 && destStartSample + numSamples <= size);
jassert (source != nullptr);
if (numSamples > 0)
{
isClear = false;
FloatVectorOperations::copy (channels[destChannel] + destStartSample, source, numSamples);
}
}
/** Copies samples from an array of floats into one of the channels, applying a gain to it.
@param destChannel the channel within this buffer to copy the samples to
@param destStartSample the start sample within this buffer's channel
@param source the source buffer to read from
@param numSamples the number of samples to process
@param gain the gain to apply
@see addFrom
*/
…and the similar occasions…
t0m
June 29, 2017, 7:50am
2
Calling any of the AudioBuffer functions with a negative number of samples is not undefined behaviour - it just won’t have any effect.
yfede
June 29, 2017, 8:34am
4
Disregard my previous post, I misinterpreted the code posted by the OP. Need coffeee…
daniel
June 29, 2017, 9:21am
5
Not quite. The argument is an int, which does a raw cast to a size_t. So we are copying quite a bit of memory and certainly not in the limit.
And the jasserts in copyFrom asserting that the int is intact. In my case I ended up with this unlucky case:
buffer.copyFrom (0, 32, buffer2, 0, 32, -32);
jassert (destStartSample >= 0 && destStartSample + numSamples <= size); // doesn't blow, 0 <= 32 + -32
But it is copying 18446744073709551584 * sizeof(float) bytes. That’s not really what I call no effect
template <typename Size>
void fill (double* dest, double valueToFill, Size num) noexcept
{
#if JUCE_USE_VDSP_FRAMEWORK
vDSP_vfillD (&valueToFill, dest, 1, (vDSP_Length) num);
#else
JUCE_PERFORM_VEC_OP_DEST (dest[i] = valueToFill,
val,
JUCE_LOAD_NONE,
const Mode::ParallelType val = Mode::load1 (valueToFill);)
#endif
}
template <typename Size>
void copyWithMultiply (float* dest, const float* src, float multiplier, Size num) noexcept
{
#if JUCE_USE_VDSP_FRAMEWORK
vDSP_vsmul (src, 1, &multiplier, dest, 1, (vDSP_Length) num);
#else
JUCE_PERFORM_VEC_OP_SRC_DEST (dest[i] = src[i] * multiplier,
jules
June 29, 2017, 2:30pm
6
Yep, those functions could use some assertions!
1 Like
t0m
June 29, 2017, 3:22pm
7
But FloatVectorOperations::copy is always guarded with
if (numSamples > 0)
somewhere earlier in the call stack - you can see an example in your first post. How are you getting through?
daniel
June 29, 2017, 3:59pm
8
Fair point, maybe it wasn’t the copyFrom. I didn’t actually check (because the memory corruption was screwing up in a different call stack). I rather fixed my math not to send negative numbers.
But I researched it now, /probably/ it was applyGain here:
/** Applies a gain multiple to a region of one channel.
For speed, this doesn't check whether the channel and sample number
are in-range, so be careful!
*/
void applyGain (int channel, int startSample, int numSamples, Type gain) noexcept
{
jassert (isPositiveAndBelow (channel, numChannels));
jassert (startSample >= 0 && startSample + numSamples <= size);
if (gain != (Type) 1 && ! isClear)
{
Type* const d = channels [channel] + startSample;
if (gain == 0)
FloatVectorOperations::clear (d, numSamples);
else
FloatVectorOperations::multiply (d, gain, numSamples);
}
}
ending up here, where vDSP_Length is an “unsigned long”:
#if JUCE_USE_VDSP_FRAMEWORK
vDSP_vmulD (src1, 1, src2, 1, dest, 1, (vDSP_Length) num);
#else
JUCE_PERFORM_VEC_OP_SRC1_SRC2_DEST (dest[i] = src1[i] * src2[i], Mode::mul (s1, s2), JUCE_LOAD_SRC1_SRC2, JUCE_INCREMENT_SRC1_SRC2_DEST, )
#endif
}
void JUCE_CALLTYPE FloatVectorOperations::multiply (float* dest, float multiplier, int num) noexcept
{
#if JUCE_USE_VDSP_FRAMEWORK
vDSP_vsmul (dest, 1, &multiplier, dest, 1, (vDSP_Length) num);
#else
JUCE_PERFORM_VEC_OP_DEST (dest[i] *= multiplier, Mode::mul (d, mult), JUCE_LOAD_DEST,
const Mode::ParallelType mult = Mode::load1 (multiplier);)
#endif
}
void JUCE_CALLTYPE FloatVectorOperations::multiply (double* dest, double multiplier, int num) noexcept
{
#if JUCE_USE_VDSP_FRAMEWORK
vDSP_vsmulD (dest, 1, &multiplier, dest, 1, (vDSP_Length) num);
Edited: not line 722 but line 758 in FloatVectorOperations.cpp - different arguments…
jules
June 29, 2017, 4:02pm
9
I’ve already looked at this and added a bunch of assertions. There were a few methods where negative numbers were safe, others where they weren’t - I’ve just added assertions everywhere because I think you’d always want to find out about it if your code starts passing around numbers like that.
This would be such a great place for us to use a paradigm like array_view… We really must add something like that to the library and it’ll simplify a whole heap of this kind of code.
3 Likes
