[SOLVED ]How to allocate dynamically AudioInputSource for a mixer

My aim is to develop a mixer, which has to perform the following things:

  • The user selects the name of the track from a ComboBox
  • The track is instantiated in the GUI, together with a volume bar and play/stop buttons

Despite everything seems fine, when I close the program I encountered several errors.
I suspect that the main issue reside in using OwnedArray for handling the several aspects of a mixer.

After several debugs, the problem seems to arise in the following lines of code:

void MainComponent::sampleRandomAudio(int idx)
{
    int randomInt = juce::Random::getSystemRandom().nextInt(listOfFiles.size() - 1);

    juce::AudioFormatReader* theReader = formatManager.createReaderFor(listOfFiles[randomInt]);

    if(theReader != nullptr)
    {
    
        if (readers.size() == 0)
        {

            readers.add(new juce::AudioFormatReaderSource(theReader, true));
        }
        else
        {
            readers.insert(idx, new juce::AudioFormatReaderSource(theReader, true));
        }
        

        if (transports.size() == 0)
        {
            auto* audioTransport = new juce::AudioTransportSource();
            audioTransport->setSource(new juce::AudioFormatReaderSource(theReader, true));
            audioTransport->addChangeListener(this);
            transports.add(audioTransport);
            mixer.addInputSource(transports[0], false);

        }
        else
        {
            auto* audioTransport = new juce::AudioTransportSource();
            audioTransport->setSource(new juce::AudioFormatReaderSource(theReader, true));
            audioTransport->addChangeListener(this);
            transports.insert(idx, audioTransport);
            mixer.addInputSource(transports[idx], false);

        }


        
        if (states.size() == 0)
        {
            states.add(new TransportState(Stopped));
        }
        else
        {
            states.insert(idx, new TransportState(Stopped));
        }

    }
}

This function is in charge of taking a music file in a random way from a folder, then the audio source is passed to a mixer of type MixerAudioSource.

In the header, I declared the following variables:

    enum TransportState
    {
        Stopped,
        Starting,
        Stopping,
        Playing
    };

    juce::OwnedArray<TransportState> states;
    juce::OwnedArray<juce::AudioTransportSource> transports;
    void transportStateChanged(TransportState newState, int index);
    void changeListenerCallback(juce::ChangeBroadcaster* source) override;

    void managingConstellations();
    void sampleRandomAudio(int idx);

    void playButtonClicked(int idx);
    void stopButtonClicked(int idx);
    juce::AudioFormatManager formatManager;

    juce::OwnedArray<juce::Slider> sliders;
    juce::OwnedArray<juce::DrawableText> texts;
    juce::OwnedArray<juce::TextButton> playButtons;
    juce::OwnedArray<juce::TextButton> stopButtons;

    juce::OwnedArray<juce::String> chosenConstellations;

    juce::MixerAudioSource mixer;

    juce::OwnedArray<juce::AudioFormatReaderSource> readers;

    juce::ComboBox listOfConstellations;
    juce::ComboBox alreadyAddedConstellations;

Any ideas on how to handle pointers in a smooth way, without encountering errors when the program is closed? Sorry for this post, Iā€™m a newbie and I recognize that there are several redundancies inside my code.

Ok, after revising the Audio Player tutorial, I wrote this:

void MainComponent::sampleRandomAudio(int idx)
{
    int randomInt = juce::Random::getSystemRandom().nextInt(listOfFiles.size() - 1);

    juce::AudioFormatReader* theReader = formatManager.createReaderFor(listOfFiles[randomInt]);

    if(theReader != nullptr)
    {

        std::unique_ptr<juce::AudioFormatReaderSource> tempSource(new juce::AudioFormatReaderSource(theReader, true));

        
        if (transports.size() == 0)
        {
            transports.add(new juce::AudioTransportSource());
            transports[0]->setSource(tempSource.get());

            transports[0]->addChangeListener(this);
            mixer.addInputSource(transports[0], false);
        }
        else
        {
            transports.insert(idx, new juce::AudioTransportSource());
            transports[idx]->setSource(tempSource.get());

            transports[idx]->addChangeListener(this);
            mixer.addInputSource(transports[idx], false);
        }


        if (states.size() == 0)
        {
            states.add(new TransportState(Stopped));
        }
        else
        {
            states.insert(idx, new TransportState(Stopped));
        }

        playSource.add(new std::unique_ptr<juce::AudioFormatReaderSource>());
        playSource[idx]->reset(tempSource.release());


    }
}

With PlaySource being:

juce::OwnedArray<std::unique_ptr<juce::AudioFormatReaderSource>> playSource;

First of all: OwnedArray with unique_ptrs donā€˜t make any sense, you were better of before. OwnedArray is responsible for deleting its content properly at the correct time the same unique_ptr does. An equivalent to OwnedArray would look something like vector<unique_ptr>.

From what Iā€˜m seeing, you are using OwnedArrays just fine. Just looking over it, I donā€˜t think you are supposed to pass ā€žtheReaderā€œ over to your readers array AND the source of your audioTransport. At both places (or more like four) you are passing the reader with the Boolean set to true. The Boolean defines if the AudioFormatReaderSource will delete the reader whatā€™s it self gets deleted (kind of like the OwnedArray, but the OwnedArray always deletes its content, you donā€™t get to choose). So try changing the flag to false at two corresponding places. You are doing it the correct way when adding the transport to the mixer. You are managing the transports life time yourself (using the OwnedArray) and donā€˜t want the mixer to delete it as well.

Just a remark from my side: the code you sent doesnā€˜t really show, why you need the readers Array in the first place. I would consider to remove it. To pass the same FileReader into two places might be a little dangouros anyway.

When you hit the assertions from the LEAK_DETECTOR at the end of your program, you should be able to figure out which class leaks, that information would be very useful.

1 Like

Hi Rincewind! First of all, thank you for your answer :slightly_smiling_face:

  • About the OwnedArray: youā€™re totally right and, as a matter of fact, I wrote another versione of my code where I implemented an OwnedArray of AudioFormatReaderSource, exploiting the pointers by passing them throughout the execution. The program works smoothly but I encountered the same error when I close it, that is:
    if (oldMasterSource != nullptr)
        oldMasterSource->releaseResources();

inside the file juce_AudioTransportSource.cpp at the lines 103-104. This is due to the fact that oldMasterSource points to NULL: it seems something changed its address but Iā€™m not able to understand this behavior.

  • For the other advices: thank you so much! I will modify the code and I will post it as soon as possible :slightly_smiling_face:

If Iā€˜m not mistaken, this is exactly the bug I found in your code. You are passing the file reader as owned at two places. The oldMasterSource tries to delete an object from whom it thinks it owns which was already deleted by the other source you just put in the array. Not sure if you were saying you now understand the problem and can fix it :slight_smile:

1 Like

After following your advices, I changed the code as it follows:

  • I substituted the playSource array with a simpler juce::AudioFormatReaderSource* playSource;
  • At the end, I simply release the tempSource, as it follows: playSource = tempSource.release();

Now, the error changed:

*** Leaked objects detected: 2 instance(s) of class AudioFormatReaderSource
JUCE Assertion failure in juce_LeakedObjectDetector.h:92

What should I do now?

Ah, I forgot to mention this: the number of instances is equal to the number of tracks that I instantiate inside the GUI. In this case, I selected 2 tracks, subsequently the number of leaked objects detected is 2.

You were right, the key to solve this issue is in understanding how to manage the AudioFormatReaderSource

Now you are leaking the sources because your are creating one here:
readers.add(new juce::AudioFormatReaderSource(theReader, true));
but never calling delete to release the resource (donā€™t do that!!, thats what unique_ptrs and OwnedArrays are for)

My suggestion was to change this line to
readers.add(new juce::AudioFormatReaderSource(theReader, false));
Now the sources are properly deleted by the OwnedArray but do NOT delete the actual reader (by ā€œrecursionā€). This is done by the transport source.

Even if this is working, that is bad software design. As already pointed out I would highly suggest not to share an AudioFormatReader (you might have a good reason though?). The reason why this could blow up is when the transport source deletes the reader that the source is still trying to use. By your current software design this is not at all secured. You might just run into this issue by simple swapping the order of member variables in your header file.

1 Like

I removed the readers.add lines. The program works but thereā€™s still the same error. Iā€™ll try to move the elements inside the header

Ah, plus I remove the OwnedArray juce::OwnedArray<juce::AudioFormatReaderSource> readers; too

You shouldnā€™t have to that, when you get your implementation right it doesnā€™t matter :wink:

Could you share you current code and current assertion again?

1 Like

The actual code:

void MainComponent::sampleRandomAudio(int idx)
{
    int randomInt = juce::Random::getSystemRandom().nextInt(listOfFiles.size() - 1);

    juce::AudioFormatReader* theReader = formatManager.createReaderFor(listOfFiles[randomInt]);


    if(theReader != nullptr)
    {

        std::unique_ptr<juce::AudioFormatReaderSource> tempSource(new juce::AudioFormatReaderSource(theReader, false));

        

        if (transports.size() == 0)
        {
            transports.add(new juce::AudioTransportSource());
            transports[0]->setSource(tempSource.get());

            transports[0]->addChangeListener(this);
            mixer.addInputSource(transports[0], false);

        }
        else
        {
            transports.insert(idx, new juce::AudioTransportSource());
            transports[idx]->setSource(tempSource.get());

            transports[idx]->addChangeListener(this);
            mixer.addInputSource(transports[idx], false);

        }


        if (states.size() == 0)
        {
            states.add(new TransportState(Stopped));
        }
        else
        {
            states.insert(idx, new TransportState(Stopped));
        }

        tempSource.release();
    }
}

The header:

private:
    enum TransportState
    {
        Stopped,
        Starting,
        Stopping,
        Pausing,
        Paused,
        Playing
    };

    juce::OwnedArray<TransportState> states;
    juce::OwnedArray<juce::AudioTransportSource> transports;

    void transportStateChanged(TransportState newState, int index);
    void changeListenerCallback(juce::ChangeBroadcaster* source) override;

    void managingConstellations();
    void deletingConstellations();
    void sampleRandomAudio(int idx);

    void playButtonClicked(int idx);
    void stopButtonClicked(int idx);

    juce::AudioFormatReaderSource* playSource;
    juce::AudioFormatManager formatManager;

    juce::OwnedArray<juce::Slider> sliders;
    juce::OwnedArray<juce::DrawableText> texts;
    juce::OwnedArray<juce::TextButton> playButtons;
    juce::OwnedArray<juce::TextButton> stopButtons;

    juce::OwnedArray<juce::String> chosenConstellations;
    juce::OwnedArray<juce::String> doomedConstellations;

    juce::MixerAudioSource mixer;

    int indexMessage = -1;
    int elementsOffset = 55;
    juce::ComboBox listOfConstellations;
    juce::ComboBox alreadyAddedConstellations;

Ahh ok, after reading the docs (should have done that earlier, my bad) I finally understand how you came up with that code in your original question. JUCE: AudioTransportSource Class Reference

You have to delete the AudioSource you pass into the AudioTransport yourself. That means: add back the OwnedArray sources into your header file.
Replace this line
std::unique_ptr<juce::AudioFormatReaderSource> tempSource(new juce::AudioFormatReaderSource(theReader, true));
with

auto source = new juce::AudioFormatReaderSource(theReader, true)
arraySources.add(source);

And then these two lines
transports[0]->setSource(tempSource.get());
with
transports[0]->setSource(source);

1 Like

I correct the code as you suggested. Now it came back the if (oldMasterSource != nullptr) oldMasterSource->releaseResources(); error :confounded:

Ok, I think then a similar thing happens, what I already said: the source is deleted before the transport is deleted and the transport tries to call ā€œreleaseResourcesā€ on an already deleted source.

You have to ensure somehow, that the transport array gets deleted first and the sources array second. Try adding a destructor:

~MainComponent() {
  transports.clear(); // Clear (=delete) all transports first
  arraySources.clear();
}
1 Like

Ok, now the error changed again: it occurs inside the function void MixerAudioSource::releaseResources():

void MixerAudioSource::releaseResources()
{
    const ScopedLock sl (lock);

    for (int i = inputs.size(); --i >= 0;)
        inputs.getUnchecked(i)->releaseResources();

    tempBuffer.setSize (2, 0);

    currentSampleRate = 0;
    bufferSizeExpected = 0;
}

The error is reported at the line inputs.getUnchecked(i)->releaseResources();

Man, thank you so much, you saved me!!!

Now it all works! I added the line mixer.removeAllInputs(); and now everythingā€™s fine!!!

The final code is the following:

MainComponent::~MainComponent()
{
    transports.clear();
    sources.clear();
    mixer.removeAllInputs();
    shutdownAudio();
}

Again, thank you soooo much!!!

Do removeAllInputs first, before you delete the transports (= the inputs).

And if you have the time: I would recommend creating a class that combines the transport and the audio source. This class can assure that its transport is deleted before its source and your code will increase in security and readability. Then youā€™ll only need one OwnedArray in your MainComponent storing your new type

1 Like

Hi Rincewind!
Following your suggestions, I decided to build up a Mixer through the AudioProcessorGraph: each channel is added by adding a node to the masterNode, which is the main PluginProcessor in my case.

Everything works smoothly and the code (you were right!) is way mooore readable. But now I encounter the same Leaked object error, always for AudioFormatReaderSource.

Hereā€™s the code of the Mixer channel, which is of AudioProcessor type:

#include <JuceHeader.h>
#include "ProcessorBase.h"

class mixerChannel : public ProcessorBase
{
public:
    mixerChannel()
    {
        addParameter(volume = new juce::AudioParameterFloat("volume", "Volume", 0.0, 1.0, 0.0));
        addParameter(reproductionState = new juce::AudioParameterChoice("state", "State", states, 0));

        juce::AudioFormatManager formatManager;
        formatManager.registerBasicFormats();

        juce::File audioPath = "C:/Users/renda/Documents/provaJuce/mixerProject/Source/audio";
        juce::Array<juce::File> listOfFiles = audioPath.findChildFiles(2, false);
        int randomInt = juce::Random::getSystemRandom().nextInt(listOfFiles.size() - 1);
        std::unique_ptr<juce::AudioFormatReader> reader(formatManager.createReaderFor(listOfFiles[randomInt]));

        if (reader)
        {
            std::unique_ptr<juce::AudioFormatReaderSource> source(new juce::AudioFormatReaderSource(reader.release(), false));
            source->setLooping(true);
            transport.setSource(source.release());
            transport.start();
        }
    }

    void prepareToPlay(double sampleRate, int samplesPerBlock) override
    {
        transport.prepareToPlay(samplesPerBlock, sampleRate);
    }
    
    void processBlock(juce::AudioSampleBuffer& buffer, juce::MidiBuffer&) override
    {
        auto totalNumInputChannels = getTotalNumInputChannels();
        auto totalNumOutputChannels = getTotalNumOutputChannels();

        for (auto i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
            buffer.clear(i, 0, buffer.getNumSamples());

        buffer.applyGain(*volume);
        transport.getNextAudioBlock(juce::AudioSourceChannelInfo(buffer));

    }

    void releaseResources() override
    {
        transport.releaseResources();
    }

    ~mixerChannel() override
    {

    }

    const juce::String getName() const override { return "Mixer Channel"; }
    
    juce::StringArray states{ "Stopped", "Starting", "Stopping", "Pausing", "Paused", "Playing"};
    juce::String lastState;
    juce::AudioParameterChoice* reproductionState;
    juce::AudioParameterFloat* volume;
    juce::AudioTransportSource transport;

When you pass the reader to the source, you call release on the unique ptr (= freeing the pointer from its ownership without deleting it) but you donā€˜t tell the source that its now the new owner. Set the Boolean flag to true and you should be good to go.

1 Like