Preventing AAX chunk problems


#1

Pro Tools does not guarantee thread safety of GetChunkSize / GetChunk calls, although these calls will always be called in pairs from a thread, multiple different threads can make these pairs of calls simultaneously. To block this from happening I've added a chunkThreadId mutable varible to the JuceAAX_Processor class and use atomic operations around this to make sure Pro Tools won't break things:

// edit: added new error code
#define AAX_ERROR_PLUGIN_API_INVALID_THREAD 20700

        AAX_Result GetChunkSize (AAX_CTypeID chunkID, uint32_t* oSize) const override
        {
            if (chunkID != juceChunkType)
                return AAX_CEffectParameters::GetChunkSize (chunkID, oSize);

            Thread::ThreadID threadId = Thread::getCurrentThreadId ();
            if ((Thread::ThreadID)(nullptr) == chunkThreadId.compareAndSetValue (threadId, (Thread::ThreadID)(nullptr)))
            {
                tempFilterData.reset();
                pluginInstance->getStateInformation (tempFilterData);
                *oSize = (uint32_t) tempFilterData.getSize();
                return AAX_SUCCESS;
            }
            return AAX_ERROR_PLUGIN_API_INVALID_THREAD;
        }

        AAX_Result GetChunk (AAX_CTypeID chunkID, AAX_SPlugInChunk* oChunk) const override
        {
            if (chunkID != juceChunkType)
                return AAX_CEffectParameters::GetChunk (chunkID, oChunk);
            Thread::ThreadID threadId = Thread::getCurrentThreadId ();
            if (threadId == chunkThreadId.get ())
            {
                if (tempFilterData.getSize () <= oChunk->fSize)
                {
                    oChunk->fSize = (uint32_t) tempFilterData.getSize();
                    tempFilterData.copyTo (oChunk->fData, 0, tempFilterData.getSize());
                    tempFilterData.reset();
                    chunkThreadId.compareAndSetValue ((Thread::ThreadID)(nullptr), threadId);
                    return AAX_SUCCESS;
                }
                // if the chunk size wasn't good still clear the threadId to allow future attempts
                chunkThreadId.compareAndSetValue ((Thread::ThreadID)(nullptr), threadId);
            }
            return AAX_ERROR_PLUGIN_API_INVALID_THREAD;
        }

 

Then add the chunkThreadId as a member:


        // tempFilterData is initialized in GetChunkSize.
        // To avoid generating it again in GetChunk, we keep it as a member.
        mutable juce::MemoryBlock tempFilterData;
        mutable juce::Atomic<Thread::ThreadID> chunkThreadId;

And initililse it in the constructor:


        JuceAAX_Processor()  
          : sampleRate (0)
          , lastBufferSize (1 << AAX_eAudioBufferLengthNative_Max) // assume the max of 1024 that is specified by the AAX sdk.
          , currentChannelLayoutIdx (0)
          , chunkThreadId ((Thread::ThreadID)(nullptr))
        {

#2

or do this in getPlugInDescription

 

      AAX_IPropertyMap *properties = descriptor.NewPropertyMap();

      properties->AddProperty(AAX_eProperty_RequiresChunkCallsOnMainThread, 1);

      descriptor.SetProperties(properties);

#3

This seems like a sensible thing to add! Does it work for you, Andrew?


#4

I don't think it is a good idea to set that flag. Avid may remove support for it in the future then Juce wrapped AAX plugins will break, which is messy, and also setting the flag disables certain features for the plugin as indicated in this post (you need an aax developer account to login and see)

https://developer.digidesign.com/index.php?L1=5&L2=13&L3=56

Rob from Avid has replied to me saying that my method is good, since in the odd case where two threads call the GetChunkSize / GetChunk an error message is displayed to the user, and otherwise normal operation is permitted, and no crashes will occur. They are providing a fix for this very shortly in a PT 11 update, so once that is in place the error code will never trigger.


    /** \brief Indicates whether the plug-in supports SetChunk and GetChunk calls on threads other than the main thread.
    *   It is actually important for plug-ins to support these calls on non-main threads, so that is the default.  
    *   However, in response to a few companies having issues with this, we have decided to support this constraint 
    *   for now.
    *
    *   property value should be set to true if you need Chunk calls on the main thread.
    *
    *  \li Apply this property at the \ref AAX_IEffectDescriptor level
    */
    AAX_eProperty_RequiresChunkCallsOnMainThread = 308,

#5

Ok guys, I've pushed some code that's hopefully a fix for this - I've not got an AAX build here to test right now so would appreciate sanity-checking!


#6

I doubled checked with Rob and he said don't set that flag, returning an error is a better solution. He also said to use an error code in the range:

AAX_ERROR_PLUGIN_BEGIN = -20600,
AAX_ERROR_PLUGIN_END = -21000

So I've added a new #define to my orriginal post.


#7

Hi Jules,

You've added a per thread local storage which is great, but there is a logic error in your code, the following will crash:


        AAX_Result GetChunk (AAX_CTypeID chunkID, AAX_SPlugInChunk* oChunk) const override
        {
            if (chunkID != juceChunkType)
                return AAX_CEffectParameters::GetChunk (chunkID, oChunk);
            MemoryBlock& tempFilterData = getTemporaryChunkMemory();
            if (tempFilterData.getSize() == 0)
                pluginInstance->getStateInformation (tempFilterData);
            oChunk->fSize = (int32_t) tempFilterData.getSize();
            tempFilterData.copyTo (oChunk->fData, 0, tempFilterData.getSize());
            tempFilterData.reset();
            return AAX_SUCCESS;
        }

Since the documentation says:

 


virtual AAX_Result AAX_IACFEffectParameters::GetChunkSize    (    AAX_CTypeID     iChunkID,
uint32_t *     oSize 
)        const
pure virtual
Get the size of the data structure that can hold all of a chunk's information.
If chunkID is one of the plug-in's custom chunks, initialize *size to the size of the chunk's data in bytes.
This method is invoked every time a chunk is saved, therefore it is possible to have dynamically sized chunks. However, note that each call to GetChunkSize() will correspond to a following call to GetChunk(). The chunk provided in GetChunk() must have the same size as the size provided by GetChunkSize().
Legacy Porting Notes:
In AAX, the value provided by GetChunkSize() should NOT include the size of the chunk header. The value should ONLY reflect the size of the chunk's data.
Parameters
[in]    iChunkID    ID of the queried chunk
[out]    oSize    The chunk's size in bytes
Implemented in AAX_CEffectParameters.
  

 

So that means the Juce code has to return an error if the current thread has no tempFilterData already fetched. The doco also says that the fSize, fVersion, and fData need to be filled in but, currently since everything is working without the fVersion filled in, I suggest we leave it that way in case this breaks things. I am guessing that this field is zero.


        AAX_Result GetChunk (AAX_CTypeID chunkID, AAX_SPlugInChunk* oChunk) const override
        {
            const int AAX_ERROR_PLUGIN_API_INVALID_THREAD = 20700;
            
            if (chunkID != juceChunkType)
                return AAX_CEffectParameters::GetChunk (chunkID, oChunk);
            
            MemoryBlock& tempFilterData = getTemporaryChunkMemory();
            
            if (tempFilterData.getSize() == 0)
                return AAX_ERROR_PLUGIN_API_INVALID_THREAD;
            
            oChunk->fSize = (int32_t) tempFilterData.getSize();
            //oChunk->fVersion = ??;
            tempFilterData.copyTo (oChunk->fData, 0, tempFilterData.getSize());
            tempFilterData.reset();
            return AAX_SUCCESS;
        }

#8

I'm getting the following error with Xcode 6.2, is it just me?

 

juce_AAX_Wrapper.cpp:478:9: error: reference to 'MemoryBlock' is ambiguous

        MemoryBlock& getTemporaryChunkMemory() const

        ^

In file included from juce_AAX_Wrapper.mm:26:

In file included from juce_AAX_Wrapper.cpp:36:

In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Cocoa.framework/Headers/Cocoa.h:12:

In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/Foundation.h:80:

In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/Foundation.framework/Headers/NSURLError.h:12:

In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/CoreServices.framework/Headers/CoreServices.h:23:

In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/CoreServices.framework/Frameworks/AE.framework/Headers/AE.h:20:

In file included from /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/CarbonCore.h:63:

/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/CoreServices.framework/Frameworks/CarbonCore.framework/Headers/MacMemory.h:130:41: note: 

      candidate found by name lookup is 'MemoryBlock'

typedef struct MemoryBlock              MemoryBlock;

                                        ^

In file included from juce_AAX_Wrapper.mm:26:

In file included from juce_AAX_Wrapper.cpp:39:

In file included from JUCE/modules/juce_audio_plugin_client/AAX/../utility/juce_IncludeModuleHeaders.h:25:

In file included from JUCE/modules/juce_audio_plugin_client/AAX/../utility/../juce_audio_plugin_client.h:28:

In file included from JUCE/modules/juce_audio_plugin_client/AAX/../utility/../../juce_gui_basics/juce_gui_basics.h:28:

In file included from JUCE/modules/juce_audio_plugin_client/AAX/../utility/../../juce_gui_basics/../juce_graphics/juce_graphics.h:29:

In file included from JUCE/modules/juce_audio_plugin_client/AAX/../utility/../../juce_gui_basics/../juce_graphics/../juce_events/juce_events.h:51:

JUCE/modules/juce_audio_plugin_client/AAX/../utility/../../juce_gui_basics/../juce_graphics/../juce_events/interprocess/juce_InterprocessConnection.h:29:7: note: 

      candidate found by name lookup is 'juce::MemoryBlock'

class MemoryBlock;


#9

Gah.. Forgot about that silliness when using AAX. Thanks, I'll sort that out.


#10

Many thanks Andrew - I've pushed again so fingers crossed should be all good now.


#11

You're welcome! I'm glad to be able to help out with such a great framework. I'm just about to deliver a retina ready fully scalable gorgeous looking interface with crisp text at all sizes since Juce supported this so easily!


#12

Hey,

Currently experiencing 20700 error in Pro Tools (AAX) when trying to save or while Pro-Tools tries to autosave, with a simple plugin without custom StateInformation (empty ::getStateInformation (MemoryBlock& destData)).

Curiously, adding destData.setSize(1); as a content to StateInformation(…) apparently solves this issue.

I suspect checking for size == 0 is the wrong way for triggering the 20700 error as it is triggered for all plug-ins not defining custom chunk (if I'm right)? What do you think t? Do you experience the same issues with your plugins which don't define custom chunks?

All the best


#13

Up? this happens with _every_ AAX plug-in not defining custom chunk…


#14

As you need to override the getStateInformation method in your AudioProcessor anyway, it should be possible to simply return a single byte in this method. You can ignore this when restore the state of the plug-in. Would this work?


#15

Hey Fabian,

Indeed, this is a workaround. One could argue that it's not a proper fix for this regression.


#16

Bump. As per fbecker - this is not a proper fix. Not all plugins need to return state. If they do then we should have some assertion that’s triggered. If not AAX_ERROR_PLUGIN_API_INVALID_THREAD is a silly error :slight_smile:


#17

Thanks for bringing this to our attention again. We are working on it…


#18

This should be fixed now with this commit on develop.

Can you confirm that this fixes your issue?


#19

Fabian - sorry I’m not set up to test this for a couple of weeks. But I think the test is simple:

  • Create plugin with null body for getStateInformation
  • Load plugin in protools
  • Press save.

Expected result: session saves.

Actual result prior to fix: error message appears.


#20

I’ve just read your commit and the AAX documentation though and it looks like it’ll fix it :slight_smile: