Ok, I believe I addressed all your concerns now.
> Please squash the intermediate commits that contain "bug fix" and "printf".
> As long as things aren't merged you can still fix up the branch history to
> have meaningful commits only. git rebase -i also makes it really easy to fix
> or squash intermediate states.
Done.
> Why is "using std::max" required in a namespace that already does "using
> namespace std" ? And, in general it's better not to do "using namespace std",
> long term that has high potential of causing conflicts, e.g. with the
> introduction of std::any we're likely running into problems with Aida::Any.
> Instead, use std::max, std::vector, or "using std::vector;" because you know
> that'd the one vector you mean to make use of in your code. I.e. just remove
> and fixup the "using namespace std" in portamento.cc.
Right. Today, I also believe that we should never have code like 'using
namespace std', ever. I removed it and it still compiles.
> Get rid of "FIXME"s in code meant to be merged into mainline. For the
> step/page increments I'd say just remove them in a commit and then rebase+fix
> that commit into the one that used to introduce the FIXMEs. If we find out
> later the increments don't do what we want, we can adjust them later.
Done.
> Seeing that your plugin does two multiplications per sample just to convert
> back and force between frequency values makes me wonder if we choose the
> right path there. This should be tracked in a different bug, but what's your
> take on making BSE_SIGNAL_TO_FREQ / BSE_SIGNAL_FROM_FREQ NOPs, i.e. *1.0 ? DO
> we really rely on freqs being scaled down into 0..1 somewhere? It's not like
> freqs are all that useful to run through a low-pass or an amp...
Yes, we discussed this in person: I don't have strong feelings for either
version, but I believe we could safely use plain frequencies and maybe
introduce some kind of type-tag on streams to show whether a frequency is
expected in the ui.
On the other hand, if speed is all you care about, as long as
BSE_SIGNAL_TO_FREQ / BSE_SIGNAL_FROM_FREQ is a linear function (not exponential
or anything), we could simply operate on the scaled values directly, without
calling either macro once-per-sample.
> Finally, please rebase again, to track latest master ;-)
Done.
Note: I also added a minimal portamento demo, which just shows off this feature
in isolation. As things should be self contained, I neither used bleposc nor
drum samples (i.e. from some hydrogen kit), although this would probably
improve the overall sound quality of the demo.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tim-janik/beast/pull/28#issuecomment-371281958
_______________________________________________
beast mailing list
beast@gnome.org
https://mail.gnome.org/mailman/listinfo/beast
