Tracktion ValueTreeSynchronizer

This is a bit of a weird situation and I can’t wrap my head around how to address it. I’m currently using tracktion_engine and attempting to synchronize the edit ValueTree to another application. Everything seemed to be working fine, however I’ve noticed that things are going haywire when adding a new track. Here’s what’s happening (apologies in advance for my poor C++ knowledge)

I’m attaching a ValueTreeSynchronizer to the edit value tree (basically ValueTreeSynchroniser(edit->state)). I then call edit->insertNewAudioTrack to add a new audio track to the edit. This is where things get weird - the synchronizer on the receiving end winds up receiving a bunch of spurious, out-of-order synchronization messages that muck up the tree:

// (Debug output)
PROPERTY CHANGED valueTree: TRACK idvalue: 1045
CHILD ADDED parent: TRACK child: MODIFIERS index: 1
CHILD ADDED parent: EDIT child: TRACK index: 0

I’ve found that the reason for these out-of-order synchronization messages is as follows: The edit-insertNewTrack method first creates a new ValueTree of type IDs::TRACK (tracktion_Edit.cpp) then attaches it to the edit ValueTree. This eventually causes the underlying ValueTree to call sendChildAddedMessage (juce_ValueTree.cpp), which ought to trigger the valueTreeChildAdded for the ValueTreeSynchronizer. However it first calls the valueTreeChildAdded method for the tracktion_engine::TrackList ValueTreeObjectList (tracktion_ValueTreeUtilities.h) which triggers the actual construction of the Track class. This involves construction of an EditItem in the Track initializer list (tracktion_Track.cpp), which actually winds up calling sendPropertyChangeMessage for the Track ValueTree. Since the Track ValueTree has been attached to the Edit ValueTree, the callListenersForAllParents call winds up triggering the valueTreePropertyChanged of the Edit ValueTreeSynchronizer, sending the first spurious message (PROPERTY CHANGED valueTree: TRACK idvalue: 1045 in the above debug log).

Since these synchronization messages aren’t specific to a subtree, rather they specify the child indices, this message winds up setting an idvalue in the first child of the Edit ValueTree on the receiving end. Likewise, every other message which reflects some change to the Track or child of the Track are received by the other end and applied incorrectly, until the final sendChildAddedMessage for the actual Track is finally called. By this point, the tree on the receiving end is completely wrong.

How can I ensure that these changes are being reflected correctly? The only solution I can imagine would be to somehow ensure that the sendChildAddedMessage callback is called for the ValueTreeSynchronizer listener before the internal tracktion engine listeners, but I have no idea how to do that beyond modifiying ValueTree to insert listeners in a specific order…

Any thoughts or ideas would be greatly appreciated!

Just to add to this thread, I indeed got it “working” (at least as far as I can tell) by reversing the order of the parent tree callback order in callListenersForAllParents (juce_ValueTree.cpp)

template <typename Function>
void callListenersForAllParents (ValueTree::Listener* listenerToExclude, Function fn)
    std::vector<SharedObject*> v;
    for (auto* t = this; t != nullptr; t = t->parent)
    for (auto it = v.rbegin(); it != v.rend(); ++it)
        (*it)->callListeners (listenerToExclude, fn);

Because the topmost listener (the ValueTreeSynchronizer attached to the root Edit ValueTree) is called first, sendChildAddedMessage is called for the inserted Track and all of the subsequent property changes / child insertions are in the proper order.

This feels a little sketchy though, and I’m wondering if there’s any way to get around modifying the JUCE library code…

Hmm, this doesn’t sound like it’s TE specific but should be fixed in the ValueTreeSynchroniser?
It sounds as though it simply shouldn’t be possible to apply a property changed stateChanged message to a tree that doesn’t exist yet? If that failed gracefully, adding the tree should add all of its data.

I don’t think you can ever rely on the order of ValueTree callbacks. Your change may work for this one case but only because your listener is added last. If for some reason TE changed and added a later listener, this would break again. Usually you get around this by doing async updates for things like UI but is seems like the ValueTreeSynchroniser is always synchronous.

I can think of possibly one other hack which would be to create your ValueTreeSynchroniser first, before you pass the state to the Edit constructor. That way, when the new track gets added, it will call your synchroniser first?

It sounds as though it simply shouldn’t be possible to apply a property changed stateChanged message to a tree that doesn’t exist yet?

Yeah, that’s what I was hoping I could rely on to address the issue, but the problem is that the property stateChanged messages encode their targets using child indices instead of keys or ids or whatever so these “out of order” messages are just applied to “random” children of the Edit state tree on the client side… Indeed it is the case that after all the Track property/sub-children change messages are processed, a correct valueTreeChildAdded message is passed and processed on the client with a correct fully formed Track ValueTree, but by the time it’s added the client tree has been corrupted.

I don’t think you can ever rely on the order of ValueTree callbacks. Your change may work for this one case but only because your listener is added last.

I actually think this hack will hold up, because what I’m actually doing is attaching the synchronizer to a parent tree that holds the edit tree, so something like

parentTree.addChild(edit->state, -1, nullptr);
auto synchronizer = ValueTreeSynchronizer(parentTree);

My reverse order hack in callListenersForAllParents actually reverses the order of the listeners from bottom->top of the tree to top->bottom of the tree, so the root node calls its listeners first, which I think is actually more correct theoretically because changes at the leafmost nodes will be dispatched through the root anyway (without knowing how any of this might impact tracktion internals, of course).

Anyway, thanks for the super quick reply! Happy to have found something that at least will work for me in the short term. I was terrified that the corrupted ValueTree was because of a race condition or memory leak, this issue is actually much more grokkable than those would have been :stuck_out_tongue:

So if you remove your hack and add the listener to the top node does it work?
Or do you still need your juce hack?

No I still need the hack, because without it the tracktion TrackList listener (which is “on” the Edit node) gets called before the parent node listeners get called, because the loop traverses up the tree. This “fix” assumes that I have a parent/root node that’s higher in the tree than any other node and that there’s only my ValueTreeSynchronizer listener on that node, so by starting at the root and walking down I can guarantee my listener gets called first.

Long term, there are only 2 real solutions that I can think of to actually fix this, but they both feel pretty heavy handed.

First (and this seems more correct), it should be the case that messages for attaching a child should be dispatched before any message for properties of that child. This would mean there ought to be some sort of way of managing when those messages are dispatched. Right now, there’s just a list of listeners that get called and there’s nothing to prevent those listeners from triggering subsequent subtree property change messages before all attach messages are sent… This seems really tricky to solve given the way this all works currently. Perhaps there could be some sort of enforced ordering for listeners, or some sort of message triaging/batching that ensures that the messages are fired in order, but both of these seem really really difficult.

The other option (which is probably even harder) would be to use a different message passing scheme that is able to prevent “out-of-order” messages from being processed. If, say, every ValueTree node had a unique id and change messages targeted nodes by id rather than a sequence of child indices, it might be possible to filter out these early update messages on the receiving end since their target nodes wouldn’t yet exist. After all, eventually the childAdded message does come through with the fully populated track ValueTree. This would obviously be a huge change, but if I were to design something like this from scratch I would have used something like node ids or keys anyway instead of child path indices, because that scheme can fail in this exact way.

Anyway, just my first two ideas off the top of my head. But having looked at this a bit there doesn’t seem to be an obvious answer, and I’m happy with my hack/patch for the short term at least…