BufferingAudioSource prepareToPlay blocks MessageThread


#1

I use BufferingAudioSources to buffer in the background. But they have a nice kickstart, which is to buffer already the first block in prepareToPlay, see:

My problem is, unfortunately I have an AudioReaderSource connected which doesn’t produce samples (probably a defective ogg file, I’m checking). But this means, I get in an infinite loop in the MessageThread, which invoked prepareToPlay.
Normal behaviour for BufferingAudioSource, when the buffer is not valid is to produce silence. Does it make sense to block prepareToPlay until one block is read?
IMHO there should be either a timeout or the while loop could be removed, because it will start in the background anyway… Or simply change the while into if, so it get’s started, but the rest is left for the background…


#2

Makes sense! See my latest commit on develop.


#3

Great, thanks :slight_smile:
Would it be possible to add this other method regarding blocking and non-blocking from a while ago,
a semaphore for non-realtime use of BufferingAudioThread, very handy to use multi threads for reading…

(a pity that this block quote always spoils the markup of the quoted post…)


#4

Hi daniel, I’m not quite sure I understand this one. If you processor is in offline mode, why don’t you just use the BufferingAudioSource’s underlying AudioSource directly?


#5

Hi Fabian,
because then all reading is done from one rendering thread.

I have a AudioProcessor, that mixes ~30 audiosources according to a special algorithm. If the getNextAudioBlock of the IO callback drives the whole chain from reading from disk to mixing, everything runs in one thread, i.e. on one CPU core.
So I have the BufferingAudioSource to prepare the buffers. This uses TimeSliceThreads to distribute the load of reading (I use 4 threads, runs perfectly).

a) Situation live: everything is perfect, because the calls from the audio device are coming at a fixed interval, leaving enough time to read
b) Situation rendering: the writer calls the mixing as fast as possible. So the background threads have not enough time to fetch the next audio block.

With the bool waitForNextAudioBlockReady (const AudioSourceChannelInfo &, const int sleepMiliSecs=10, const uint32 timeout=500) the reading is happening on several threads, but mixing is easy when using on the audioSource:

BufferingAudioSource source;
AudioSourceChannelInfo info (&buffer, 0, bufferSize);
if (isNonRealtime ())
    source.waitForNextAudioBlockReady (info);
source.getNextAudioBlock (info);

This way you can easily synchronize the threads.

Does this make sense?
Thanks for your time

Correction for the wait method: don’t block if source cannot produce any samples:

/** Wait for buffer ready. This will block until the buffer is prepared. DO NOT USE IN REALTIME! */
bool BufferingAudioSource::waitForNextAudioBlockReady (const AudioSourceChannelInfo& info, const int sleepMilliSecs, const uint32 timeout)
{
    if (!source || source->getTotalLength() <= 0)
        return false;

    if (nextPlayPos + info.numSamples < 0 ||
        (! isLooping() && nextPlayPos > getTotalLength())) {
        return true;
    }

    const uint32 endTime = Time::getMillisecondCounter () + timeout;
    int validStart;
    int validEnd;
    while (Time::getMillisecondCounter() < endTime) {
        {
            const ScopedLock sl (bufferStartPosLock);
            validStart = (int) (jlimit (bufferValidStart, bufferValidEnd, nextPlayPos) - nextPlayPos);
            validEnd   = (int) (jlimit (bufferValidStart, bufferValidEnd, nextPlayPos + info.numSamples) - nextPlayPos);
        }
        if (validStart <= 0 &&
            validStart < validEnd &&
            validEnd >= info.numSamples) {
            return true;
        }
        Thread::sleep (sleepMilliSecs);
    }

    return false;
}

#6

Btw. if there is a better method for synchronising threads using BufferingAudioSources, I’m open for suggestions…


#7

@Fabian: A function int BufferingAudioSource::getNumReady() similar to AbstractFIFO would be an alternative, even though I find the solution above easier to read, as I don’t have to write a loop everytime I want to wait for the buffering/decoding thread.


#8

No I like your original suggestion better. I’m working on it…


#9

OK I’ve implemented something on my on private repo. Can you head over there and check if this works for you?

https://github.com/hogliux/JUCE/tree/feature/BlockingBufferingAudioSource


#10

Thanks, looks good. I didn’t know the WaitableEvent, that’s an interesting concept.
But I’m afraid, it won’t work here. I don’t see, that the backgroundThread would signal the waiting thread.
In this case the thread will wait the full timeout:

if (! bufferReadyEvent.wait (endTime - now))

So it actually will slow things down instead of speeding up.
The alternative I proposed was to suspend the waiting thread for 5 ms and poll again. What’s your opinion on this?

Cheers


#11

No, the given time is the maximum time to wait. The corresponding signal is in the read thread (here). The version with the WaitableEvent will normally be faster then any version with sleep as the thread will immediately be waken up when there is data to read - unless I have a bug in there somewhere :slight_smile:


#12

Nice, I thought I missed something. I forgot, that the working thread calls a method of BufferingAudioSource as well. And I overlooked that line in your commit.
I will pull and try it out, but I am pretty sure it will work…


#13

Unfortunately something is wrong…
Initially it didn’t wait, so I got scrambled audio in my rendered file.
But if I remove the static keyword, it works as expected (or use const as I did):

And the waitableEvent gives also a noticable speedup :slight_smile:


#14

Wow I must have been completely beside myself yesterday: I wanted to write const not static?!? I’ve fixed it and merged it into develop.


#15

…and I thought another trick using static that I don’t know of :wink:
Thanks for taking the time


#16

Works perfect, thanks for adding (and for improving it)


#17

Hi @Fabian,
I use this function now in both, realtime and non-realtime like this:

    if (currentSource) {
        if (owningTheme->isNonRealtime()) {
            currentSource->waitForNextAudioBlockReady (bufferToFill, 500);
        }
        else {
            if (! currentSource->waitForNextAudioBlockReady (bufferToFill, 0)) {
                // we don't wait for wimps
                bufferDropout += 1;
            }
        }
        buffersPlayed += 1;
        currentSource->getNextAudioBlock(bufferToFill);
    }

But for this to work, the comparism on line 168 in BufferingAudioSource needs to be changed into less or equal:

uint32 now = Time::getMillisecondCounter();
const uint32 endTime = now + timeout;

while (now <= endTime)
//[...]

EDIT: and that change as well to make it not wait:

if ((endTime > now) && (! bufferReadyEvent.wait (static_cast<int> (endTime - now))))
        return false;

Do you think we could change that?

Thanks and have a nice weekend…


#18

Yup makes sense. It’s on develop now.


#19

Should the wrap around of the uint32 Time::getMillisecondCounter() should also be considered?


#20

Thanks @Fabian!

@chkn indeed, looking at the docs Time::getMillisecondCounter() it is noted there. If it is really zero at startup, the problem will arise after 49 days. Still, it can happen…