Just taking a look at this as it's been pulled in, so wanted to ensure all is still well with dynamic audio objects. Made a few tweaks, and while I was scanning the code found these
two nasties:
This one contradicts the API which states "This function may return NULL if no memory is available". Nope, by the look of it, if no memory is available it'll stall the whole application until a block
becomes available. Which may be never...
Code:
int16_t * AudioPlayQueue::getBuffer(void)
{
if (userblock) return userblock->data;
[COLOR="#FF0000"]while (1) {
userblock = allocate();
if (userblock) return userblock->data;
yield();
}[/COLOR]
}
Because getBuffer() is called from AudioPlayQueue:
lay(), that inherits the problem, too, but because play() returns void there's no way for the user to
know there's a problem (apart from the stall).
This would appear to stall until the next audio interrupt causes AudioPlayQueue::update to free up a queue space, but in this case doesn't even call yield():
Code:
void AudioPlayQueue::playBuffer(void)
{
uint32_t h;
if (!userblock) return;
h = head + 1;
if (h >= max_buffers) h = 0;
[COLOR="#FF0000"]while (tail == h) ;[/COLOR] // wait until space in the queue
queue[h] = userblock;
head = h;
userblock = NULL;
}
I can see from git Blame that these are both long-standing bugs, not part of your new code.
I'd suggest getBuffer() should behave as documented, and return NULL if no audio block is available.
I'd suggest playBuffer() should return some sort of success/fail indication: it's not 100% clear what, as there are three outcomes:
- no userblock allocated, so it wasn't queued / didn't need to be
- no queue space available, userblock not queued
- userblock queued as requested
Thoughts?
Cheers
Jonathan