Audio examples #644
Audio examples #644
Conversation
example/lfo.cpp
Outdated
|
|
||
| musical_context m_context; | ||
| quantity<si::hertz, float> m_frequency; | ||
| quantity_point<angular::radian, float> m_phase{0.f}; |
There was a problem hiding this comment.
SI also has radian but does not have revolutions. I am not sure which one is the better use case considering that you use SI already.
There was a problem hiding this comment.
I'm not sure I have a strong opinion. I went with this both because of angular::revolutions and having a base dimension angle seems more natural than angle being m / m.
If you think it would be better for the example to use si::radian and 2π * si::radian instead of angle::revolutions. Any strong opinions?
I can also poll my colleagues (who are not on holiday) tomorrow about this to get their thoughts.
There was a problem hiding this comment.
This was just an FYI so you can use the best tool for the job. You know better what is needed than me 😉
Split out audio units and musical context functions into individual headers to be used in future examples in addition to the LFO example.
Add an example of wrapping an unsafe OS or host application API with a type-safe quanitites API. The `audio::get_musical_context` API used in this example will also be used in other audio examples.
rothmichaels
force-pushed
the
feature/audio-examples
branch
from
b0fbe94 to
aec61bc
Compare
|
|
||
| void set_period(QuantityOf<isq::time> auto period) | ||
| { | ||
| m_frequency = 1.f / value_cast<float>(period); |
There was a problem hiding this comment.
You can also use inverse() from math.h that provides additional safety (https://aurora-opensource.github.io/au/main/reference/math/#inverse_as-inverse_in). However, it is not that mandatory as you are using floating-point representations.
There was a problem hiding this comment.
Oh nice, I like this.
| void set_period(QuantityOf<audio::beat_count> auto period) | ||
| { | ||
| std::cout << MP_UNITS_STD_FMT::format("Setting period to {} -- ", period); | ||
| set_period(value_cast<float>(period) / m_context.current_tempo); |
There was a problem hiding this comment.
This value_cast might not be needed here as current_tempo is already a floating point.
| void reset() { m_phase = phase_t{0.f * angular::radian}; } | ||
|
|
||
| private: | ||
| using phase_t = quantity_point<angular::radian, mp_units::default_point_origin(angular::radian), float>; |
There was a problem hiding this comment.
Yes, it is unfortunate that a point origin must be provided when we want to specify a specific representation type. I have no ideas on how to improve here. I believe that for points, most people will use double, and the main interest will be to work against different origins. This is why I provided arguments in such order. But maybe I am wrong, or is there another way to improve here?
There was a problem hiding this comment.
If the origin is an expression, {}.
There was a problem hiding this comment.
I am not sure what you mean by that?
There was a problem hiding this comment.
I meant something like this, but it turns out that it doesn't work with auto NTTPs, whereas it does with function parameters (https://cpp1.godbolt.org/z/hPY9KeYfa):
#include <initializer_list>
#include <type_traits>
template<int Origin = 1, class Rep = double>
struct point;
static_assert(std::is_same_v<point<{}, int>, point<0, int>>);| // duration equal to 2 measures of 4/4 music (i.e. 2 whole notes at | ||
| // the current tempo): | ||
| const auto beats = 2 * audio::whole_note; | ||
| const auto buffer_duration = value_cast<float>(beats) / context.current_tempo; |
There was a problem hiding this comment.
value_cast is probably not needed here?
| // of audio samples. In this example we will create a buffer with | ||
| // duration equal to 2 measures of 4/4 music (i.e. 2 whole notes at | ||
| // the current tempo): | ||
| const auto beats = 2 * audio::whole_note; |
There was a problem hiding this comment.
It is up to you, and this is purely a matter of personal preference. 😉 AAA (Almost Always Auto) is a nice style, and I like it in many cases (especially not to repeat myself).
However, with time, I have learned to appreciate CTAD as it makes the code easier to reason about. Using quantity and quantity_point CTAD placeholders instead of auto probably makes the code easier for a new user to understand or maintain.
There was a problem hiding this comment.
const Quantity auto beats is also an option, but it is more to type and does not have any benefits here over just const quantity beats.
| std::cout << MP_UNITS_STD_FMT::format("Filling buffer with values from LFO @ {}", sin_gen.get_frequency()); | ||
| std::generate(begin(buffer_1), end(buffer_1), sin_gen); | ||
|
|
||
| assert(buffer_1.size() > 0u); |
There was a problem hiding this comment.
I am not sure what this assert does as you set the vector's size explicitly in line 132. Did you mean reserve() and then using back_inserter in the algorithm?
| for (std::size_t i = 1u; i < buffer_1.size(); ++i) { | ||
| std::cout << MP_UNITS_STD_FMT::format(", {}", buffer_1[i]); | ||
| } |
There was a problem hiding this comment.
Again, it is up to you but I would use a range-for here 😉
| auto buffer_2 = buffer_t(buffer_1.size()); | ||
| std::generate(begin(buffer_2), end(buffer_2), sin_gen); | ||
|
|
||
| return buffer_1 == buffer_2; |
There was a problem hiding this comment.
The error in the CI is strange, but #include <vector> is missing, so it might be the reason why the comparison operator is not found on one of the configurations.
Co-authored-by: Mateusz Pusz <[email protected]>
Co-authored-by: Mateusz Pusz <[email protected]>
No description provided.