Damn!!! sorry for the spam, forgot to mention...
Julius says:
>
> Note that df1 and df2 are _not_ equivalent in the time-varying case
Yes, but I don't see the usage of modulated fb_comb() in *.lib ....
However, I can redo this PR with
_fb_comb1(dop,N,b0,aN) = *(aN) + *(b0) ~ dop(N-1);
_fb_comb2(dop,N,b0,aN) = + ~ aN * dop(N-1) : *(b0);
the latter is less efficient, but probably closer to the current implementation
in the time-varying case.
Oleg.
On 02/23, Oleg Nesterov wrote:
>
> Oh...
>
> I am so sorry for the huge delay. Trust me, I was very-very busy with
> my paid work.
>
> Please see https://github.com/grame-cncm/faustlibraries/pull/218
> Hmm... I get the same "Merging is blocked" error again...
>
> I decided to split this change. The second patch is trivial, but the
> changelog is not.
>
> Do you agree with the naming and documentation? If not - I will be happy
> to make V2.
>
> If this PR is accepted, I'll try to do more similar changes (fbcombfilter,
> allpass_comb, and ff_comb).
>
> With or without these changes, IMO the comments above fb_comb/fb_fcomb
> look a bit confusing/inaccurate. But this needs another "doc-only" patch
> and probably not from me ;)
>
> Oleg.
>
>
> On 11/12, Stéphane Letz wrote:
> >
> > Thanks Oleg,
> >
> > Then I can merge a PR, if you can take some time to work on it (with or
> > without the help of Julius, if it make sense.….)
> >
> > Stéphane
> >
> > > Le 12 nov. 2024 à 17:14, Oleg Nesterov <o...@redhat.com> a écrit :
> > >
> > > Hi Stéphan,
> > >
> > > sorry for the late reply, I was sick.
> > >
> > > On 11/08, Stéphane Letz wrote:
> > >>
> > >> I tried to revive this old question, here is Julius answer.
> > >>
> > >> What do you think here ?
> > >
> > > Well, I still think the same ;) Let me try to summarize, even if I forgot
> > > the details...
> > >
> > > 1. fb_comb/fb_fcomb are suboptimal and can be improved (without changing
> > > their semantics), this is simple.
> > >
> > > 2. The documentation doesn't match the code. Even if we forget about the
> > > extra "mem" at the end, the differential equation doesn't match the
> > > one in https://ccrma.stanford.edu/~jos/pasp/Feedback_Comb_Filters.html
> > >
> > > I agree with Julius, we should not change the current behaviour, this
> > > would break (or at least subtly change) the existing code. So I guess
> > > the documentation should be corrected.
> > >
> > > 3. The new generic helper, something like
> > >
> > > _fb_comb(dop,N,b0,aN) = + ~ aN * dop(N-1) : *(b0);
> > >
> > > makes sense, then we can trivially rewrite fb_comb/fb_fcomb using this
> > > helper (again, without changing the current behaviour).
> > >
> > > Plus this helper allows to use, say, dop = de.fdelay4a(max) or any other
> > > fractional delay.
> > >
> > > And... it's a pity that we're discussing this off-list :/
> > >
> > > Oleg.
> > >
> > >> Stéphane
> > >>
> > >>> Début du message réexpédié :
> > >>>
> > >>> De: Julius Smith <julius.sm...@gmail.com>
> > >>> Objet: Rép. : fb_comb/fb_fcomb implementation
> > >>> Date: 8 novembre 2024 à 19:42:07 UTC+1
> > >>> À: Stéphane Letz <l...@grame.fr>
> > >>>
> > >>> Hi Stéphane,
> > >>>
> > >>> Thanks for the forward. I am not keeping up on Discord!
> > >>> (If there is a way to trigger emails from Discord, please let me know.)
> > >>>
> > >>> I don't recall the details, but things like fb_comb were among the
> > >>> first things I ever wrote in Faust, so I do not expect "hardened code"
> > >>> there. The trailing MEM was probably to allow use in a feedback loop,
> > >>> as feedback combs started out as Schroeder reverb components. The case
> > >>> of one versus two delay lines is probably the question of direct form 1
> > >>> or direct form 2. I am traveling right now and have limited email
> > >>> time, so if we want to drill down on this, I can do it next week. In
> > >>> general, I am fine with any and all rewrites, but please use new names
> > >>> when not exactly equivalent! My favorite MO is when someone writes a
> > >>> whole new suite of primitives with a nice new set of design principles
> > >>> and naming conventions. To merely exend what I wrote, I would come up
> > >>> with a new name such as fb_comb_df2 for direct-form-2, and also write
> > >>> fb_comb_df1 such that fb_comb == fb_comb_df1 : mem. Note that df1 and
> > >>> df2 are _not_ equivalent in the time-varying case, and they are very
> > >>> different numerically (which could make a massive difference in a
> > >>> fixed-point version of Faust, should that ever happen, and might impact
> > >>> FPGA implementations now).
> > >>>
> > >>> - Julius
> > >>>
> > >>>
> > >>> On Fri, Nov 8, 2024 at 11:34 AM Stéphane Letz <l...@grame.fr
> > >>> <mailto:l...@grame.fr>> wrote:
> > >>> Hi Julius,
> > >>>
> > >>> This old thread came again on Discord :
> > >>> https://sourceforge.net/p/faudiostream/mailman/message/37884756/
> > >>> <https://sourceforge.net/p/faudiostream/mailman/message/37884756/>
> > >>>
> > >>> What do you think ? Should fb_comb/fb_fcomb be rewritten with Oleg
> > >>> proposal ?
> > >>>
> > >>> Thanks.
> > >>>
> > >>> Stéphane
> > >>>
> > >>>
> > >>> --
> > >>> For language models, Wittgenstein is right: "The limit of language is
> > >>> the limit of the world"
> > >>
> > >
> >
_______________________________________________
Faudiostream-devel mailing list
Faudiostream-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/faudiostream-devel
