Closed tomcombriat closed 6 months ago
Memory usage change @ fdeb17a5b930e495c25dfe9bcec0e421a126a7f5
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Looks promising! I will need time to look at it in more depth, so just a first set of thoughts:
getInt()
is not an ideal choice, here, perhaps one asInt();
(same as asFloat()
, and one asRaw()
? Again, not sure on the name, but I think asRaw() would prevent somebody using it under a wrong assumption of "oh, I know what 'int' does."static UFixMath2<NI,NF> fromRaw(type raw);
template<byte NIout, byte NFout, byte NIother, byte NFother> UFixMath<NIout, NFout> mult(const UFixMath<NIother, NFother> other);
(declaration looks terrible, but function calls would be as easy as toto.mult<16, 16>(tata);
Regarding what code gets generated: Do you know godbolt.org? It's great for this sort of task, and also has AVR gcc (be sure to specify the "-O2" compiler option, manually!).
Edit: Thinking about it, I realize that my proposal about multiplication would bring us back the problem of when to shift, and probably does not really help any...
Thanks for your comments!
No great idea, so far, but I think we should think about how to avoid confusion between "int" (the regular number), and "int" the underlying storage type in UFixMath.
Good point. I will change that. Note that in my mind, to only get the integer part, one could have casted to a type with no fractional part but, while this will still be possible, I agree that asInt()
is misleading as it is now.
nto a static "factory" function (inside the class, so it can initialize the private storage: static UFixMath2<NI,NF> fromRaw(type raw);
I am not sure I followed you completely here but I think I do, will change also to be coherent with the first comment.
I agree with your reasoning. However, should there be a limit to the upshifting […]?
Maybe, but note that there are no limits for standard integers, you are free to overflow them at will ;). That being said, it should not be too hard to add some boundaries, for instance with a static_assert
here, or in IntegerType
(I tend to think it would make more sense there actually). Will try!
For the down cast, the good thing here is that you never need to, and it won't by default. But in some codes, it is needed to downcast before in order to keep the types down. Maybe a bad example but here is a snip of one of my synth source code:
int oscil1 = (((aSin_next * (255 - mix1) + aSquare_next * (mix1)) >> 8 ) * (255 - mix_oscil)) >> 5 ;
Downcasting only at the end would lead to an overflow, so it is needed to cast before to cut some bits as we go. But both are possible with the current solution, by default you can keep all precision and be safe (up to the limit of the board), or you can cast down, in fix type which is the neat thing, and control the size of what you end up with. It also allows (old types also, but I think this makes it clearer) to keep the precision only where it matters.
Division: No real idea, yet.
I have got an idea there, we'll see how that turns out.
Did not have time to work on it today, but will try to get a more complete version this weekend. I'll update my initial post as I go as a mock-up documentation.
Do you know godbolt.org?
No, I did not, but that looks super useful for exploring these kind of optimizations, thanks!
That might be a bit further down the line, but I am starting to think that, if this turns well, it could also be useful outside of Mozzi. So might be worth thinking of publishing it also as a separate library. Mozzi could embed a copy of it, or call it, depending what is the easiest… Once again, not sure we are there yet…
Edit, on your edit: That was my first try, but I lost a night of sleep trying to "shift" smartly but you really need to have information about what are the numbers involved by the user :/… Note that it does not preclude the user making a few custom multiplication by building on this paradigm. I wrote all this before seeing this edit.
Hi,
I took into consideration your suggestions. I still have a doubt if I understood your "split the constructor" correctly.
I also added an multiplication overload for "standard" types, like multiplying with an int
, we multiply the internal value. I choose not to prevent potential overloads in these case to avoid too much promotion (with int
used as default type, this adds 16bits with any multiplication). If you want to be safe, use only FixMath would be my motto but I feel this is a subjective choice and happy to discuss it.
On that I ran into an unforeseen problem that you might have a neat solution at hand: I had no problem defining the overload UFixMath2*T
(with T
a typename), but the reverse confuses the compiler, even when done outside the class as a free function like:
template<typename T,byte NI, byte NF>
UFixMath2<NI,NF> operator* (const T op, const UFixMath2<NI, NF>& uf)
{
return UFixMath2<NI,NF>(uf.asRaw()*op,true);
}
Seems that the compiler sees an ambiguity between the cast from any type T
to a FixType and this operation, leading to me defining (only a few for now) specifications for all the types, which is not satisfying. Am I missing something? Thanks!
Memory usage change @ 9a4d04c53278b83f939fd77951e50b395acf3a23
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
This is just a theory, I'm not even sure, I understand the problem correctly (what exactly do I need to paste into the compiler to produce the problem?). Anyway, I think it may be the int constructor that is causing the trouble. Since the second parameter has a default value, this c'tor would allow the compiler to cast int to a UFixMath.
If that is the case, one way would be to declare all constructors "explicit". Another - and this is what I had in mind earlier - is to formally avoid constructors, but rather (not sure, my syntax is 100% correct):
template<byte NI, byte NF> // NI and NF being the number of bits for the integral and the fractionnal parts respectively.
class UFixMath2
{
[...]
private:
UFixMath2() {}
public:
static UFixMath2<NI, NF> fromRaw(int raw) {
UFixMath2<NI, NF> ret;
ret.internal_value = raw;
return ret;
}
static UFixMath2<NI, NF> fromInt(int integer) {
UFixMath2<NI, NF> ret;
ret.internal_value = integer << NF;
return ret;
}
[...]
}
And then you'd write:
auto tata = UFixMath<8, 8>::fromInt(2);
for construction.
Of course a hybrid approach should work as well: "Regular" constructors declared explicit, and the "raw" constructor moved to a static factory function.
Hi again,
Sorry I did not give a precise test, indeed I think this is the problem too.
Okay I understand more what you meant earlier, thank!
I feel a bit bad to have to leave out the syntaxs (which might be ambiguous though):
UFixMath2<8,8> tata=10;
which would have been neat. I think the problem with the mixed approach is that, for multiplying say with standard int
would ideally work on the raw data…
Thanks for your advices, I'll try to come up with a neat solution building on that! The way I feel now would be to move only one of them outside (probably the asRaw
) to keep the syntax light for the user, but that won't resolve this ambiguity. Out of curiosity, except that it might be hard to maintain and a bit inelegant, is there anything very wrong about expliciting all the reverse operators (their syntax is very light).
Thanks again!
I'm actually starting to think that using this library straight might actually be the cleanest instead of recoding a whole new thing, maybe by putting it as required library upon Mozzi installation from the IDE. As of now, we would have license problems but that might change soon?
I haven't tried it though
is there anything very wrong about expliciting all the reverse operators
I'm not sure, what this implies, but if it makes things work, why not. I'm simply more used to "explict" on constructors.
I'm actually starting to think that using this library straight might actually be the cleanest
I cannot currently estimate, how much code (and work) your solution would mean. Avoiding an extra dependency would have some value, too. But in fact, if that means many hours of work, it may not be worth it.
Not that we probably don't have to be as complete as that other library (implementing logic operators and all that jazz). As long as we do have a basic UFixMath-type, it would still be easy enough - for those who really wish to, to interface with a more complete library.
Edit: To explain, how interfacing might be easy enough at user level:
Note that we could provide:
template<int A, int B> class UFixed;
template<int A, int B> UFixedMath2<A,B> UFixedtoMozziFixed(UFixed<A, B> n) {
return UFixedMath2<A,B>::fromRaw(n.getInternal());
}
in Mozzi, and this will still compile without actually including UFixed. Only as soon as it is actually used, the compiler would need to see the UFixed-headers.
Edit 2: By the same logic, of course, it may be good enough to provide the interfacing, only. I.e. Oscil and friends would gain (templated) member functions taking a UFixed, where applicable. That should allow using FixedPointArduino, without introducing a hard dependency.
Thanks!
I am not really afraid of the work involved, I think it is a great thing for me to get more familiar with this kind of programming. I am trying to come up with something really targeted toward performances for Mozzi, depending how that turns out we can switch to the other one if needed!
Memory usage change @ 385f033bb5f2f7af9382d4504670221bbdb3c78c
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ a74d66329cd81e6c01dc73acbca4ff6c0bac8ff1
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 7349ac425276d4d92b7565e92315dd52b5bb657b
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ b6ae357c63d28fd428a4b9efcf1ec575dbca6881
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ b4e0beeda8e27ed7de6628a3415e1af2472034c8
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Alright, I start to like the way it behaves so, before extending the part of Mozzi which could leverage this, maybe it is good to be all on the same page.
I have added some documentation inside the source file, and a test code in the initial comment of this discussion. The implementation is sometimes a bit verbish, you might have ways to shorten that, but I did not find how to do a few things more elegantly while keeping the way I wanted to work. Some of my ideas were impeded by the fact that only C++11 is allowed if I am correct… The idea is to have something as close as possible to standard types (float actually). So, as said in the documentation, all permitted operations between Fix types are safe and keep precision. The only thing one should take a bit care is not to cast unneededly with too many unnecessary bits.
At the end of the day, I do not think the user will actually to cast so many of these (Oscil.setFreq()
but usually called with Q16n16_mtof
and casting a UFixMath2<16,16> from a MIDI note is straightforward).
Of course, clean examples will be added in due time.
Memory usage change @ 6f3256e8770410305302860d32966a56585c6411
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 659da6adccf528be0569d91202203176b8fc592e
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ f267ff4be556108a09a9a5d5f4b673e485a700bf
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 2d12770d0b3afbba90393b280c9c0236bdea9481
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Just throwing a small idea here:
Is there anything wrong into moving all of mozzi_midi.cpp
inside mozzi_midi.h
? Would that pose some problems with respect to the midiToFreq
table? Moving it there would allow to be a bit more coherent as I had to move the templated version of mtof
for UFixMath
there. It would also allow some more inlining I guess.
Anyone against?
Edit:
In the same vein, I guess phase_increment_fractional = ((unsigned long)frequency) * ((256UL*NUM_TABLE_CELLS)/UPDATE_RATE);
and the likes could be replaced by: phase_increment_fractional = ((uint32_t)frequency) * ((256UL*NUM_TABLE_CELLS)/UPDATE_RATE);
, which would have no effect on AVR but provide a small boost for 32bitters by avoiding to go to 64 bits?
Memory usage change @ b6bd60f4dc781aa0b6f3546b4a77a87128ac1a4b
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -136 - -64 | -0.01 - -0.0 | :green_heart: -32 - -16 | -0.01 - -0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:green_heart: -100 - -92 | -0.04 - -0.04 | :green_heart: -20 - -16 | -0.06 - -0.05 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:green_heart: -16 - -8 | -0.0 - -0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 012ea082f086e47799c4c5767f3a059d21afb029
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -136 - -64 | -0.01 - -0.0 | :green_heart: -32 - -16 | -0.01 - -0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:green_heart: -100 - -92 | -0.04 - -0.04 | :green_heart: -20 - -16 | -0.06 - -0.05 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:green_heart: -16 - -8 | -0.0 - -0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 8127b4f4916608cf4c1dbea83e6274a47aca54a9
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -136 - -64 | -0.01 - -0.0 | :green_heart: -32 - -16 | -0.01 - -0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:green_heart: -100 - -92 | -0.04 - -0.04 | :green_heart: -20 - -16 | -0.06 - -0.05 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:green_heart: -16 - -8 | -0.0 - -0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 73273a5e053a0716f0a643c8e4ef3c5521f0b3db
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -136 - -64 | -0.01 - -0.0 | :green_heart: -32 - -16 | -0.01 - -0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:green_heart: -100 - -92 | -0.04 - -0.04 | :green_heart: -20 - -16 | -0.06 - -0.05 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:green_heart: -16 - -8 | -0.0 - -0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 179d05796644d1c04b60324dfd69ca8a4e04710f
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -136 - -64 | -0.01 - -0.0 | :green_heart: -32 - -16 | -0.01 - -0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:green_heart: -100 - -92 | -0.04 - -0.04 | :green_heart: -20 - -16 | -0.06 - -0.05 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:green_heart: -16 - -8 | -0.0 - -0.0 | 0 - 0 | 0.0 - 0.0 |
Alright, I think the important things are there.
This can now be used with mozzi_midi and with Oscil
. For these two, it falls back to the legacy mozzi_fixath to avoid code duplication. So far untested, will try to do that and to provide nice examples soon.
Best,
Memory usage change @ 11545ddd74078d41be6d00907678a64f43e5b343
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 3f09dedca42ab36c9379f16f434acefb4e1a3c15
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Memory usage change @ 0c495ebf805cd22c19fe1bbaf17e9e11c3d457bf
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
Just throwing a small idea here:
Is there anything wrong into moving all of
mozzi_midi.cpp
insidemozzi_midi.h
? Would that pose some problems with respect to themidiToFreq
table? Moving it there would allow to be a bit more coherent as I had to move the templated version ofmtof
forUFixMath
there. It would also allow some more inlining I guess.
Totally agree with this. That's also one step further towards the "single compilation unit". I also think (as in talking without testing), that it should not be a problem for the midiToFreq table. The trouble to watch out for, is what will happen if two separate .cpp files both include a call to mtof(). Will the linker be smart enough to discard one copy of the table? I hope so, but am not 100% sure.
Edit:
In the same vein, I guess
phase_increment_fractional = ((unsigned long)frequency) * ((256UL*NUM_TABLE_CELLS)/UPDATE_RATE);
and the likes could be replaced by:phase_increment_fractional = ((uint32_t)frequency) * ((256UL*NUM_TABLE_CELLS)/UPDATE_RATE);
, which would have no effect on AVR but provide a small boost for 32bitters by avoiding to go to 64 bits?
Good catch!
Just throwing a small idea here: Is there anything wrong into moving all of
mozzi_midi.cpp
insidemozzi_midi.h
? Would that pose some problems with respect to themidiToFreq
table? Moving it there would allow to be a bit more coherent as I had to move the templated version ofmtof
forUFixMath
there. It would also allow some more inlining I guess.Totally agree with this. That's also one step further towards the "single compilation unit". I also think (as in talking without testing), that it should not be a problem for the midiToFreq table. The trouble to watch out for, is what will happen if two separate .cpp files both include a call to mtof(). Will the linker be smart enough to discard one copy of the table? I hope so, but am not 100% sure.
Afterthought: Maybe it's worth some extra effort to make the lookup-table private, though, so we remain free to re-design, should the need arise. Suggested solution:
class MozziMidiPrivate {
private:
friend function int mtof(uint8_t); // add other functions using this, here
static CONSTTABLE_STORAGE(uint32_t) midiToFreq[128];
};
static CONSTTABLE_STORAGE(uint32_t) MozziMidiPrivate::midiToFreq[128] = [...];
That should give the linker a definite hint not to duplicate the table, too.
Thanks!
Looking more into it, I wonder if the line
phase_increment_fractional = ((unsigned long)frequency) / (UPDATE_RATE/NUM_TABLE_CELLS);
could not benefit from calculating the inverse (NUM_TABLE_CELLS/UPDATE_RATE
with a preliminary shift on the upper part of the fraction, which would be removed after, for performances purposes. I need to try if that actually makes things faster of if the compiler is smart enough. Then it would ressemble a bit more like the Q24n8 version (with an additional right shift).
Just added comparisons between FixMath types. Did not added with other types as they can be converted to the user very easily.
Actually, I wonder if all the overloads with standards types are actually needed… Users can convert anything to FixMath using the constructors. The drawbacks would be a bit of longer syntax for the user (and might lead to type explosion if they are not set quite small to start with), but at the same time this ensures type safety and overflows. It would also ease quite a bit the implementation file…
Any opinion?
Memory usage change @ 47e028ce3bfa74b6cf8591744e9a521f4a28813a
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7492 - +20 | -11.43 - +0.03 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3424 - 0 | -0.17 - 0.0 | :green_heart: -8 - 0 | -0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +12 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +16 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
Memory usage change @ 434cd2b99e0a59ffa70a336e7206bcb7c27d4042
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7492 - +20 | -11.43 - +0.03 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3424 - 0 | -0.17 - 0.0 | :green_heart: -8 - 0 | -0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +12 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +16 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
Just added comparisons between FixMath types. Did not added with other types as they can be converted to the user very easily.
Actually, I wonder if all the overloads with standards types are actually needed… Users can convert anything to FixMath using the constructors. The drawbacks would be a bit of longer syntax for the user (and might lead to type explosion if they are not set quite small to start with), but at the same time this ensures type safety and overflows. It would also ease quite a bit the implementation file…
Any opinion?
Since these overloads are unsafe, it may in fact be a good idea to remove them. Importantly, I imagine users would often not be aware that something "unsafe" is going on behind the scenes, at all.
I have not really played with fixed math, myself, and I'm not sure, how much of a hassle this will actually mean in the real world. You definitely have more experience, here.
But, on very general grounds, adding convenient features, later, is always possible, while removing features means breaking existing sketches. So I'd recommend to err towards a more reduced implementation, initially.
But, on very general grounds, adding convenient features, later, is always possible, while removing features means breaking existing sketches.
Good point! I'll convert the examples and see how it turns. I see advantages of both paradigms (completely safe and type preservation), so will see how this turns out.
In all cases, I think good documentation and especially tutorials might be helpful (and not onlyy for this, also for outputting wiring diagrams for instance). If I recall correctly, there are now several places where this kind of things are:
I do not know how much the "learn" section is updated automatically alongside the wiki (@sensorium ?) but might be good to agree on one place for additional documentation?
Hi Tom, maybe this could be a different thread (about Documentation?), but some quick answers here for now:
places where this kind of things are:
"learn" in the website how much the "learn" section is updated automatically alongside the wiki Ha ha, there's no automatic updating of the learn section, only some very minor updates by extreme force of will through the years.
"wiki" on github I wonder if the (presently empty) wiki could be a way forward...
FWIW: The "learn" pages are served from a branch: https://github.com/sensorium/Mozzi/tree/gh-pages .
This is a little unfortunate, actually, as it makes it hard to update the code and the documentation in sync. In a world of unlimited resources, I would recommend to move them into a folder inside the sources, instead. In that same ideal world, I think it's quite possible to create a script from updating the example sketches inside learn automatically. But yes, even in the real world, we should see to getting these pages into up-to-date shape, for Mozzi 2.
I'm not immediately convinced that wiki pages are actually going to help much. When I came to Mozzi, I really loved the "learn" pages with their wealth of hands-on-documentation.
Memory usage change @ 27392d82ec28c9cd71247657cacfac3e44067f37
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7492 - +20 | -11.43 - +0.03 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3424 - 0 | -0.17 - 0.0 | :green_heart: -8 - 0 | -0.0 - 0.0 |
arduino:renesas_uno:minima |
:grey_question: -560 - +16 | -0.21 - +0.01 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +12 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +16 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
Interestingly, it seems that, at least on the Uno, jumping to float
to compute the inverse and casting back to FixMath is more efficient (2%) than invAccurate
(but 2.5x slower than invFast
for the same number of output bit). So might be worth replacing the inside of this invAccurate
as a wrapper to
1./self.asFloat();
Will do more test to confirm this, especially on other platforms. Compilers are getting quite good, code inspection reveals a very similar algorithm than mine when passing via a float…
This only stands for big types on the Uno (32 bits or more). Given the moderate gains I suggest not making test cases in the function to know which method to use (which would probably lead to something quite unreadable and an hassle to maintain for each platform) and keep the current one which performs slightly less fast (2% drop) on big types but performs 2x faster on small types, on AVR. Opinion?
The current one might also save some Flash by not loading any float routine if no float are involved.
Also seems like the renesas did not like it…
Memory usage change @ 77c6132ac048c315646695a6ce304a0a79f2779d
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7492 - +20 | -11.43 - +0.03 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3424 - 0 | -0.17 - 0.0 | :green_heart: -8 - 0 | -0.0 - 0.0 |
arduino:renesas_uno:minima |
:grey_question: -560 - +16 | -0.21 - +0.01 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +12 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +16 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
The current one might also save some Flash by not loading any float routine if no float are involved.
Yes, that point settles it, IMO.
Memory usage change @ 6cdc837f4a923ba088b50548edee2bbe9d8b758b
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7492 - +20 | -11.43 - +0.03 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3424 - 0 | -0.17 - 0.0 | :green_heart: -8 - 0 | -0.0 - 0.0 |
arduino:renesas_uno:minima |
:grey_question: -560 - +16 | -0.21 - +0.01 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +12 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +16 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
Memory usage change @ 7252b2adac5cad27e948f1be274ad75f45a4efdd
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7492 - +20 | -11.43 - +0.03 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3424 - 0 | -0.17 - 0.0 | :green_heart: -8 - 0 | -0.0 - 0.0 |
arduino:renesas_uno:minima |
:grey_question: -560 - +16 | -0.21 - +0.01 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +12 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +16 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
Memory usage change @ 358a45bdc13ef10c13e17f545711862a4fcfd1b7
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7496 - +16 | -11.44 - +0.02 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
0 - 0 | 0.0 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:mbed_giga:giga |
:green_heart: -3416 - 0 | -0.17 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14492 - +8 | -5.53 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +8 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
Memory usage change @ 7cdebee02c180f04cfb2cca4b3c27dbcf5dbb258
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7480 - +16 | -11.41 - +0.02 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
:green_heart: -2 - 0 | -0.01 - 0.0 | :small_red_triangle: 0 - +1 | 0.0 - +0.05 |
arduino:mbed_giga:giga |
:green_heart: -3416 - 0 | -0.17 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14476 - +8 | -5.52 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +8 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
You could wrap them into namespace MozziPrivate, (or MozziFixMathPrivate, or place them as private statics into the U/SFixMath classes).
@tfry-git For the private static, I was a bit afraid that would lead to a lot of duplicates in the generated code by the compiler (hence penalty in flash) as any U/SFixMath needs a lot of different U/SBITSTOBYTE
, I guess it would be optimized away though, can check. I think the namespace is more the way to go!
Memory usage change @ 267b69bf5b7a3129a60ef8089164fe5977e7fb03
Board | flash | % | RAM for global variables | % |
---|---|---|---|---|
STMicroelectronics:stm32:GenF1:pnum=BLUEPILL_F103C8 |
:grey_question: -7480 - +16 | -11.41 - +0.02 | :green_heart: -16 - 0 | -0.08 - 0.0 |
arduino:avr:uno |
:green_heart: -2 - 0 | -0.01 - 0.0 | :small_red_triangle: 0 - +1 | 0.0 - +0.05 |
arduino:mbed_giga:giga |
:green_heart: -3416 - 0 | -0.17 - 0.0 | 0 - 0 | 0.0 - 0.0 |
arduino:samd:adafruit_circuitplayground_m0 |
:grey_question: -14476 - +8 | -5.52 - 0.0 | :green_heart: -4 - 0 | -0.01 - 0.0 |
esp8266:esp8266:huzzah |
N/A | N/A | N/A | N/A |
rp2040:rp2040:rpipico |
:grey_question: -2464 - +8 | -0.12 - 0.0 | :green_heart: -48 - 0 | -0.02 - 0.0 |
This is my results of trying to get a nice FixMath working. This is still working in progress: it is quite messy, ~only
unsigned
is done and except for casts, only the multiplication is implemented~ but that gives the idea and allow for some discussion before I go further.(@tfry-git you asked for templates, well you'll be served, and macros too ;) ).
In brief:
This defines a new fixmath type2 for unsigned:
UFixMath2<N,n>
andSFixMath2<N,n>
whereN
is the number of integer bits andn
the number of fractional bits.These two values can be anything, but value safety is only ensured up to 63 bits total (I think this is enough). A static assert prevents going further.
Any
UFixMath2<N,n>
can be casted into anyUFixMath2<N2,n2>
, but if the initial value cannot be represented in the new type you are running into overflow, just like with standard types.After fiddling for quite some time, I changed the paradigm from what was discussed in #207: the multiplication between
UFixMath2<N,n>
andUFixMath2<N1,n1>
returns aUFixMath2<N+N1,n+n1>
. Basically, (after nearly skipping a night on it), I came to the conclusion that if you return to the first type of the multiplication (as discussed in #207) you have to either:The current paradigm allows for optimisation if you choose the input types so that it fits, but with no margin. The results are as accurate as they can be and won't overflow. If you do not need precision you can always cast them lower prior to multiplication (faster), or after if you want something precise but with fewer bits:
In the example above, the choice of the type for
toto
is not smart at all as the value would fit in 8 bits integral part which is why the return type is sub-optimal.So these types are quite easy to optimize if you know the expected range of your values, then you can choose the smallest container that accommodate them, with the number of fractional bits setting the precision.
Methods (so far)
float
,double
UFixMath2
UFixMath2<8,8> tata(10);
UFixMath2<8,8> tata(10,as_frac=true);
UFixMath2.asFloat()
returns the value as floatUFixMath2.getInt()
returns the internal integer.The internal integer is using
IntegerType.h
to find the correct container. The formula is a bit weird:typedef typename IntegerType<((NI+NF-1)>>3)+1>::unsigned_type internal_type ;
, but the-1, +1
ensures that your container is big enough.Other methods will to be implemented if we agree with the current layout already. On that, can we assume the compiler to be smart enough to transform
int a/2
intoint a>>1
and skip the shifting operators? I read somewhere that it should but if someone knows for sure. Integration into Mozzi, especially for theOscil.setFreq()
are also on the list.I am also wondering if the division between the FixPoints should be implemented. Once again, assumptions need to be made to shift left the left hand side of the division. Or provide a crude division that does not do it (and let the users cast up if needed). I do not think I ever used fixed point division in any of my Mozzi codes though, might mislead more than help. Of course, division by an integer is absolutely needed but the case is nearly trivial if we assume no shift before.
It is a bit messy for now, tried quite a few things last night on the pre-shift way before convincing myself that this is not satisfying. Will clean up soon.
Of course, some standard types (like Q8n8) will be typedef'd in the end.
Welcoming comments and suggestions as always!
Planned and acheived
Notes
Any operations between FixType is supposed to be safe, in terms of overflow and precision. However, any operation with other types, like
int
is not. This is basically to avoid these types to get promoted to easily for instance:I think this will push to use only FixType which, might not be a bad thing as long as people do not abuse on the container size because they can't overflow.
One idea for that might be to allow automatic construction from
const int
(and affiliated), where the constructor (or an affiliated function) determines theNI
automatically to the smallest needed value. Ideally, it would be neat to do:and the compiler finds itself that NF=2, but not sure it is possible.
Example code (without interaction with Mozzi yet)