sensorium / Mozzi

sound synthesis library for Arduino
https://sensorium.github.io/Mozzi/
GNU Lesser General Public License v2.1
1.05k stars 184 forks source link

mozzi_rand cannot go full range #220

Open eclab opened 6 months ago

eclab commented 6 months ago

uint8_t rand(uint8_t maxval)

The largest value that can be passed into this function is 255, which means that the largest possible random number that can be generated is 254.

unsigned int rand(unsigned int maxval)

The largest value that can be passed into this function is 65535, which means that the largest possible random number that can be generated is 65534.

The alternative is to call xorshift directly and mask out the value. But there ought to be versions of these functions which provide full range for the datatypes. Furthermore, calling xorshift means I have to do a bit conversion to get an int8_t or int16_t, so I get to build a union.

tomcombriat commented 6 months ago

What range would you like to have? 16 bits on 8bitters already seems to be plenty!

eclab commented 6 months ago

I'd like to have 0...255 or -128...127 on 8 bit and 0...65535 or -32768 ... 32767 on 16 bit. The simplest solution would be something like

union _secret { uint8_t u; int8_t i; } secret;
uint8_t randu8() { return xorshift() & 0xFF; }
int8_t randi8() { _secret.u = randu8(); return _secret.i; }

... and similarly for 16 bit. [I'm sure there's a C++ convention for bit-converting unsigned to signed but I don't know what it is].

tomcombriat commented 6 months ago

I don't understand, why not using

int8_t rand(int8_t minval, int8_t maxval);
int rand(int minval, int maxval);

which are respectively 8 and 16 bits on AVR.

eclab commented 6 months ago

Because the range of rand is minval to maxval-1, not to maxval.

tomcombriat commented 6 months ago

Ah, I get it. Well, this is coherent with Arduino's random() function.

I am not at the origins of these but if you look at the implementation this is done to avoid type promotion. Of course, it would be quite easy to add overloaded functions without maxval as a parameter which would return in the full type range, for instance (untested):

int8_t rand()
{
    return (int8_t)  lowByte(xorshift96());
}

Welcome to submit a PR ;), otherwise will try to do it soon… If @tfry-git and @sensorium are not against this could go in.

Edit: I guess this version would not work as the compiler will not know which one to use between the different return types, with no parameter, hence the name rand() can probably not be used, but rand_uint8 could.

(Still wondering if having the last value, which will not pop-up very frequently indeed, is actually of any use, but thsese "bare" functions would be faster which sounds appealing).

tfry-git commented 6 months ago

Maybe we don't really need all of these. Essentially, all these would do is call xorshift96() and then cast to a type that needs to be named, explicitly, anyway. (Note that the existing overloads perform additional computation with a given precision, and therefore these do make sense, indeed).

Brainstorming some ideas:

And then of course, while we're at it, I suppose it will make sense to convert "int" and "long" to int16_t and int32_t?

tomcombriat commented 6 months ago

isn't this a great opportunity for templating? rand(), etc. Except of course, it may be overkill.

And to port that for direct, U/SFixMath random numbers :P ?