Why does the dsp-module uses unsigned types for sample/channel indexes?

Audio Plugins
1

why?

  • all other juce routines using signed integers
  • signed types are better for any kind of offset calculation, with results below 0
3 Likes
2

I can only speculate, but all containers use unsigned int as indices, so might be a performance decision(?)
and by using unsigned int you cannot accidentally read before a buffer (had to find that out the hard way).
But if you overflow value range, it is a disaster anyway…

But yes, your second argument struck me also quite often…

3

exactly!
and also these kind of of jasserts will not fire if its overflown
jassert (startSample < numSamples);

4

FWIW, Stroustrup & Sutter on the topic convinced me here, e.g. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es107-dont-use-unsigned-for-subscripts

3 Likes
5

so probably, the signed variant of size_t, ssize_t is the better choice

6

There are performance reasons to avoid unsigned as well.

3 Likes
7

@jules can we have signed types?

8

Yes, I agree that the types should probably have been signed, we may replace them at some point.

2 Likes
9

Is this on the backlog?

10

Yeah, that was the first thing I noticed when checking out the new dsp classes and making a ProcessSpec. While using uint32 may be technically correct, it seems like a lot of extra static casting for simple variables like numChannels and samplesPerBlock.

11

From what I’ve read and heard, unsigned is almost always the wrong choice. Use int. And from a coding perspective, static_cast is just ugly, especially when code is written to use it from the start!

4 Likes
12

I was told a long time ago to use static_cast in favour over a regular cast, since it was supposed to be “more safe” (a C-style cast being more like a reinterpret_cast).
Is that outdated? Or preference?
A quick google suggested that it is still the preferred choice…

13

You misunderstand my meaning. When a cast is necessary, static_cast is preferable to a C style cast.

My comment was directed toward new code that requires a static_cast. so, I am saying the code should have been designed to not need the cast in the first place.

3 Likes
14

Ah, got you. Thanks, I thought it was a general style rant.
I agree, would have been nice, if the many casts were not necessary…

15

Yes, in my own code, I always figure if I’m having to use a cast, I’ve done something wrong! In other words, if I have control over the code, then I control the types that are used, and they should all match. If the types don’t match, then I must question (and re-do!) my design.

16

Are there any news?

1 Like
17

Also just recognised, the current API seems to be inconsistent, here a example from the AudioBlock class

First function uses size_t, second int

forcedinline SampleType* getChannelPointer (size_t channel) noexcept
{
    jassert (channel < numChannels);
    jassert (numSamples > 0);
    return channels[channel] + startSample;
}


SampleType getSample (int channel, int sampleIndex) const noexcept
{
    jassert (isPositiveAndBelow (channel, numChannels));
    jassert (isPositiveAndBelow (sampleIndex, numSamples));
    return channels[channel][startSample + sampleIndex];
}
18

I’m yet in a situation to add hundreds of static_casts (which will be redundant if the changes will be applied (and then again may hide real bugs)), to get rid of the warnings. So my questions, will JUCE use signed types for channels/sample-positions in the future and if yes, when?

19

It’s something that’s been on my mind a bit lately, we’ll have to have a chat about it amongst the team and see what we decide. There are pros and cons. There could also be tricks we could use, e.g. an index class that can silently cast between signed/unsigned, but that’d need some investigation.

20

I vote for signed int everywhere because of underflow issues in for loops with counters which can be decremented

7 Likes