Ability to traverse generator graph

[ndonald2]

Related to #183

The ability to actually keep track of the connections between generators and traverse the graph would open up a lot of possibilities.

• Context-aware generators
  • Each generator knows about its graph context
• Recursive initialization
  • Can initialize and/or reset the entire graph from its root node
• Cloning synths/generator graphs

How to make this somewhat automatic is proving to be a challenge.

[ndonald2]

@morganpackard I'm considering the following approach and I'd to get your thoughts on it, if it's possible to follow my techno-babbling.

Added to Generator_

typedef void (Generator_::*GeneratorSetter)(Generator gen);
std::map<string, Generator>                             inputGenerators_;
std::map<string, Generator_::GeneratorSetter>           inputGeneratorSetters_;

  void Generator_::setInputGenerator(string name, Generator gen)
  {
    std::map<string, Generator_::GeneratorSetter>::iterator it = inputGeneratorSetters_.find(name);
    if (it != inputGeneratorSetters_.end())
    {
      Generator_::GeneratorSetter setter = it->second;
      (this->*setter)(gen);
      inputGenerators_[name] = gen;
    }
    else
    {
      // fatal exception
      error("Input generator not registered for name \"" + name + "\". Did you use TONIC_INIT_GEN_INPUT?", true);
    }
  }

Changes to Macros

The requirement is that TONIC_INIT_GEN_INPUT is called in the constructor of the Generator_ subclass for each input generator.

• propertyName - the name of the property representing the input, exposed in the smart pointer
  • Must be the same in both macros - no easy way to enforce this at compile time
• setterFunctionPointer - pointer to the setter function for this input
• initialValue - initial value of the generator

#define TONIC_INIT_GEN_INPUT(propertyName, setterFunctionPointer, initialValue)                            \
{                                                                                                    \
   inputGenerators_[#propertyName] = initialValue;                                                  \
   Generator_::GeneratorSetter setter = static_cast<Generator_::GeneratorSetter>(setterFunctionPointer);   \
   inputGeneratorSetters_[#propertyName] = setter;                                                  \
}

#define TONIC_MAKE_GEN_SETTERS(generatorClassName, propertyName)                        \
                                                                                        \
  generatorClassName& propertyName(Generator arg){                                      \
    this->gen()->setInputGenerator(#propertyName, arg);                                 \
    return static_cast<generatorClassName&>(*this);                                     \
  }                                                                                     \
                                                                                        \
  generatorClassName& propertyName(float arg){                                          \
    return propertyName( FixedValue(arg) );                                             \
  }                                                                                     \
                                                                                        \
  generatorClassName& propertyName(ControlGenerator arg){                               \
    return propertyName(  FixedValue().setValue(arg) );                                 \
  }

Example

In TableLookupOsc_::TableLookupOsc_():

      TONIC_INIT_GEN_INPUT(freq, &TableLookupOsc_::setFreqGen, FixedValue(440))

In the class declaration for TableLookupOsc:

      TONIC_MAKE_GEN_SETTERS(TableLookupOsc, freq);

I've tested this for a few generators and it seems to work correctly on OSX at least. I'm not 100% sure it will work on all compilers, though, due to some weird member function pointer inheritance business (see this: http://www.codeproject.com/Articles/7150/Member-Function-Pointers-and-the-Fastest-Possible)

Also, this pattern is sort of weird, and I'm not sure it's really easy to figure out for someone wanting to create a new generator.

Thoughts?

[ndonald2]

Also works on iOS. Haven't tested windows or linux.

[morganpackard]

Will check it out when I get a minute.

Sent from my iPhone

On Aug 13, 2013, at 6:01 PM, "Nick D." notifications@github.com wrote:

Also works on iOS. Haven't tested windows or linux.

— Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/207#issuecomment-22601098 .

[morganpackard]

Haven't had a clear chunk of time to think this through, and may not for days. But a couple things:

1. If we adopted the convention that Generator parameters were public, and didn't have setters, we could enforce the convention that Generator setter functions have the same name as the public Generator parameters. That might get us somewhere. That way we could have a definitive list of all the inputs, and a way to access them.
2. With some better understanding of C++ initialization sequences (when static stuff gets built, or something), I still feel like we might be able to do something much nicer here. I feel like there's a right way to do this that we haven't found yet, and it's worth taking the time to really look hard for it.

[ndonald2]

If we adopted the convention that Generator parameters were public, and didn't have setters, we could enforce the convention that Generator setter functions have the same name as the public Generator parameters

I'm not sure I follow what you mean here. Just because they have the same names, I'm not sure how you could bridge the name of the instance variable from the underscore class to the parameter setters int the smart pointer using one macro. And c++ has no capability for string-to-variable-name introspection.

Also don't forget that some input setters have other side effects, for example changing whether an effect processes as stereo or mono, and I think it's important to keep that logic in the underscore class and out of the smart pointer - the smart pointer implementation should be as thin as possible.

With some better understanding of C++ initialization sequences (when static stuff gets built, or something), I still feel like we might be able to do something much nicer here. I feel like there's a right way to do this that we haven't found yet, and it's worth taking the time to really look hard for it.

Global static initialization happens prior to main(), but in no guaranteed order. I considered that as an option too, but I'm not sure it's possible to use static initialization to initialize instance-specific information such as a list of inputs. Memory offsets are not well-defined at compile time for non-POD objects.

[ndonald2]

Memory offsets are not well-defined at compile time for non-POD objects.

Or I suppose, not well-defined at all - AFAIK it's not possible to reliably use offsetof on a non-POD object to find the location of a member variable.

[morganpackard]

Ok. Are you really eager to move quickly on this or can we marinate on it a bit?

Sent from my Speak & Spell

On Aug 13, 2013, at 10:18 PM, "Nick D." notifications@github.com wrote:

Memory offsets are not well-defined at compile time for non-POD objects.

Or I suppose, not well-defined at all - AFAIK it's not possible to reliably use offsetof on a non-POD object to find the location of a member variable.

— Reply to this email directly or view it on GitHub.

[morganpackard]

Btw--the band limited stuff is AWESOME!

Sent from my Speak & Spell

On Aug 13, 2013, at 10:18 PM, "Nick D." notifications@github.com wrote:

Memory offsets are not well-defined at compile time for non-POD objects.

Or I suppose, not well-defined at all - AFAIK it's not possible to reliably use offsetof on a non-POD object to find the location of a member variable.

— Reply to this email directly or view it on GitHub.

[ndonald2]

Sweet! Glad you like it.

I'm not in a hurry. We can marinate until the flavor is just right. Your feedback actually gave me another idea that might be a bit simpler.

[ndonald2]

So I did a bit more research on C++ instance construction, and as far as I can tell, there is literally no way to modify an instance of an object at the exact time of its construction, except for in the constructor. Static member variables of a class are initialized in the global initialization phase, before main(), but have no knowledge of the class itself or any instance thereof.

Unless there is some really really tricky indirection that we haven't discovered yet, I'm leaning toward a three-fold macro approach to make "generic" generator creation easier. Basically you still have to remember to use the macros correctly, but IMO that's better than requiring hand-coding for everything. Obviously there will be specific cases that fall outside the capabilities of generic macros, but I feel like that's a less-common use case.

Here's what I'm thinking:

• TONIC_DECLARE_GEN_INPUT( variableName, setterName )
  • Macro to declare an input in the Generator_ subclass. Give it a variable name and a setter name, it makes both.
  • Caveats: variable should probably be protected, setter should be public. We can add that to the macro but it should be used at the very start of the class declaration to avoid changing the visibility for things declared below it.
• TONIC_MAKE_GEN_SETTERS( className, parameterName )
  • Macro to create the Generator smart-pointer setters. Already exists, could be extended.
• TONIC_INIT_GEN_INPUT( parameterName, setterFunctionPointer, initialValue)
  • Macro called for each input that is used in the constructor. This adds the setters to the map, like I outlined above, and sets the initial values.
  • The trickiest part is the setter function pointer, since it has to be scoped.
  • Alternatively, TONIC_INIT_GEN_INPUT( className_, parameterName, setterName, initialValue) and handle the scoping part in the macro
  • Harder to remember to use it, but better than doing it all manually. It would be good to somehow make it fail immediately if you forget to use it (or better yet, at compile time, but I'm not sure if that's possible).
  • Opting for setter function pointers over access to member variables directly. Member function pointers can be inherited/downcasted and work on any instance (within limits - should be OK for our purposes), vs. member variable pointers, which only work on one instance.

[morganpackard]

Was just pondering this, after having been away from the problem, and perhaps not remembering all the details of it. In my own words, here's the issue:

1) We need to be able to copy (Control)Generators from one object to another, in the correct slot, during the copy process 2) We need to be able to traverse the generator graph in order to send messages (like reset) to each generator.

I think we can handle #1 with some member function pointer wizardry / ugliness in the setter macros. For #2, isn't "tick" a nice, built-in graph traversal technique? Why couldn't we just pass messages through that way?

Surely I've missed something. I'll go back and read and see where I'm being stupid.

[ndonald2]

After having spent probably 12+ hours on problem 1) using the function pointer wizardry approach, I'm pretty unconvinced that it's the way to go. I had a semi-working implementation but it required the use of at least 2 or 3 macros and a lot of casting assumptions (member function pointer definitions + inheritance = weird). I also think any sort of "hack" that's obfuscated away into a macro is not a best-practice solution - it relies too heavily on unclear logic and hackery to be easily understood, and there's no way to enforce the usage of a macro at compile time.

That being said, I'm prepared to be wrong if you have an approach in mind. I think having something akin to a "protocol" to which generators should conform is the right approach. In c++ that's either unimplemented or pure virtual inherited methods that should be implemented in a subclass (e.g. computeOutput, copy, etc). Recalling our conversation from the other day, the fact that this is such a difficult problem leads me to believe that implementing NSCopying in Objective-C is such a pain in the ass because it's hard to do deep-copy with smart-pointer memory management any other way.

For 2), tick may indeed be a good way to do that. I think we could probably refactor it to pass down a "message" struct or something in order to prompt generators to do a number of things:

• Initialize/reset, with a "context" payload
• Produce new output
• Perform an action (this could be a good mechanism for inter-thread communication, using a thread-safe queue)
• Custom messages

Excellent suggestion.

[ndonald2]

Also FYI my comment previous to your most recent one outlines the semi-functional approach I had worked out.

[morganpackard]

Ok. Here's a new idea. Commited some initial code into https://github.com/TonicAudio/Tonic/tree/MP-Deep_Copy

Abstract ParameterCopier class:

    class ParameterCopier{
    public:
      virtual void copyInto(void*) = 0;
    };

The Generator_ holds a map of pointers to ParameterCopiers

    void Generator_::addParamterCopier(string name, ParameterCopier* copier){
      ParameterCopier* previous = parameterCopiers[name];
      parameterCopiers[name] = copier;
      if(previous){
        delete previous;
      }
    }

Define subclasses of the ParameterCopier inside the macro-generated setter functions:

    // TODO: Add this to the setter macro
    void Generator::setFoo(Generator gen){
      class ParameterCopierImpl : public Tonic_::ParameterCopier{
        Generator param;
      public:
        ParameterCopierImpl(Generator paramArg){
          param = paramArg;
        }
        void copyInto(void* genArg){
          Generator* gen = static_cast<Generator*>(genArg);
          gen->setFoo(param);
        }
      };
      obj->addParamterCopier("foo", new ParameterCopierImpl(gen));
      // also set, using gen()
    }

CreateCopy method:

    Generator Generator::createCopy(){
       // Need to figure out how instantiate copy objects for subclasses. One idea: make
       // ParameterCopier subclasses also function as factories. That way we
       // can instantiate the correct Generator subclass in code generated by the macros.
      Generator newGen;
      std::map<std::string, Tonic_::ParameterCopier*> copiers = obj->getParameterCopiers();
      std::map<std::string, Tonic_::ParameterCopier*>::iterator it = copiers.begin();
      for(; it != copiers.end(); it++){
        it->second->copyInto(&newGen);
      }
      return newGen;
    }

[ndonald2]

Beautiful. I dig it.

[ndonald2]

I made a few comments on your commit as well. I'm not sure how to solve the type issue. Is the goal to be able to have createCopy return the correct subclass type?

Not sure if it's relevant, but apparently you can't use templates in locally-defined classes (defined within a function): http://stackoverflow.com/a/876069/1128820

[morganpackard]

If createCopy was going to return the correct generator subtype, we'd have to declare TemplatedGenerator subclasses like `class GeneratorSub : public TemplatedGenerator<Tonic::GeneratorSub, Tonic::GeneratorSub>

I do think that it probably should return the correct generator subtype though. And requiring that the GeneratorSub type be passed to the template would also take care of the problem of instantiating the copy.

[ndonald2]

If createCopy was going to return the correct generator subtype, we'd have to declare TemplatedGenerator subclasses like `class GeneratorSub : public TemplatedGenerator

We already do that, though, right? TemplatedGenerator already exists and is used almost ubiquitously, except with Effect and any other special base types.

If I follow, you're saying that TemplatedGenerator should be where createCopy is declared, with a templated return type? The issue there is that we always declare parameters as just Generator so the templated createCopy would not be visible.

Maybe I'm misunderstanding.

[morganpackard]

TemplatedGenerator knows about the Generator_ subclass, but not about the Generator.

You're right, we do need a createCopy in Generator. However, I'm not sure how useful this is. Since we can't cast from Generator to a subclass, there's no way to actually use a Generator once we copy it. No way to set parameters, trigger, etc. Maybe we need two versions of createCopy -- one for Generator, and one for TemplatedGenerator.

[morganpackard]

Maybe createCopy always returns a generator, but we add Generator::copyInto(Generator*). I don't think we can cast a smart pointer, so need to pass in a castable pointer.

[ndonald2]

Yeah, the smart pointer makes it tough. Not too hard to do with standard instances:

http://www.lwithers.me.uk/articles/covariant.html

[ndonald2]

Maybe we have Generator::baseCopy() and TemplatedGenerator<GenType>::copy().

The ParameterCopier and the deep-copy traversal uses baseCopy because the type (usually) doesn't matter for parameter inputs, but if you want a type-casted copy for building a synth, you can just use copy.

Generator::copyInto(Generator*) would also work but that seems less intuitive.

One thing to keep in mind as this starts taking shape is that many Generator_ subclasses have other helper objects as member variables (like DelayLine) so we need to make sure those are copied/initialized correctly when necessary.

[morganpackard]

Maybe I'm misunderstanding the covariant return type stuff, but it looks to me like that's exactly what we need. Do covariant return types need to be pointer types, or can it be anything?

On Mon, Oct 7, 2013 at 11:00 AM, Nick D. notifications@github.com wrote:

Maybe we have Generator::baseCopy() and TemplatedGenerator::copy().

The ParameterCopier and the deep-copy traversal uses baseCopy because the type (usually) doesn't matter for parameter inputs, but if you want a type-casted copy for building a synth, you can just use copy.

Generator::copyInto(Generator*) would also work but that seems less intuitive.

One thing to keep in mind as this starts taking shape is that many Generator_ subclasses have other helper objects as member variables (like DelayLine) so we need to make sure those are copied/initialized correctly when necessary.

— Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/207#issuecomment-25815635 .

Morgan Packard cell: (720) 891-0122 twitter: @morganpackard

[ndonald2]

They need to be pointer types, apparently. I tried a quick test and it would not compile when returning the smart pointer by value. Returning a pointer instead to a new heap instance did successfully compile, but obviously we don't want that.

[leavittx]

Ok. Here's a new idea. Commited some initial code into https://github.com/TonicAudio/Tonic/tree/MP-Deep_Copy

Hi @morganpackard @ndonald2 I'm currenltly trying to see what's possbile to do with the Generator cloning. Was hoping to find some starting point in the mentioned branch, however it doesn't seem to be in the repo:( Do you have any tips on where to look for the proof of concept code for Generator cloning? Any advice on the mentioned topic is highly appreciated!

It would be soooo cool to get it working

[morganpackard]

Hi Lev, I’m so happy to see that some folks are still using Tonic! I haven’t so much as glanced at or thought about the code in years and don’t think I can be much help. Good luck! -Morgan

On Thu, May 16, 2024 at 1:56 PM Lev Panov @.***> wrote:

Ok. Here's a new idea. Commited some initial code into https://github.com/TonicAudio/Tonic/tree/MP-Deep_Copy

Hi @morganpackard https://github.com/morganpackard @ndonald2 https://github.com/ndonald2 I'm currenltly trying to see what's possbile to do with the Generator cloning. Was hoping to see find some starting point in the mentioned branch, however it doesn't seem to be in the repo:( Any tips on where to look for the proof of concept code for Generator cloning?

— Reply to this email directly, view it on GitHub https://github.com/TonicAudio/Tonic/issues/207#issuecomment-2115869990, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHWSZN4GPUJS6X2NHVBF73ZCTXLVAVCNFSM4AHGKEV2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJRGU4DMOJZHEYA . You are receiving this because you were mentioned.Message ID: @.***>