Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 7 of 7

Thread: Possible enhancements to AudioPlayQueue

  1. #1
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,365

    Possible enhancements to AudioPlayQueue

    Pull request here: https://github.com/PaulStoffregen/Audio/pull/410

    Basically add missing methods in a plausible fashion, and add a way to limit how greedy it is for audio buffers.

    This came out of this thread: https://forum.pjrc.com/threads/68507...ot-being-freed

    Feedback welcome.

    __Mark

  2. #2
    Senior Member
    Join Date
    Apr 2021
    Location
    Cambridgeshire, UK
    Posts
    146
    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;
    	while (1) {
    		userblock = allocate();
    		if (userblock) return userblock->data;
    		yield();
    	}
    }
    Because getBuffer() is called from AudioPlayQueue::play(), 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;
    	while (tail == h) ; // 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:
    1. no userblock allocated, so it wasn't queued / didn't need to be
    2. no queue space available, userblock not queued
    3. userblock queued as requested


    Thoughts?

    Cheers

    Jonathan

  3. #3
    Senior Member
    Join Date
    Jul 2020
    Posts
    1,365
    Yes I quite agree it doesn't seem to be very friendly to the rest of the system, but I added the minimum
    extra to allow individual sample queuing, in a way that matched the existing logic. At least you can
    specify a smaller max_buffers now as the defaults are quite large.

    Changing the function will break existing code out there, so there's that to consider. The "many caveats"
    comment in the docs perhaps suggests that its in need of reworking nonetheless.

  4. #4
    Senior Member
    Join Date
    Apr 2021
    Location
    Cambridgeshire, UK
    Posts
    146
    It's pretty hard to see how any functional code could depend on the horrible behaviour seen here!

    I may do a branch and PR to see if Paul will contemplate getting a fix in, since he's clearly paying attention to AudioPlayQueue changes at the moment. Unless you want to have a go at it, given that I'm very much a n00b around these parts...

  5. #5
    Senior Member
    Join Date
    Apr 2021
    Location
    Cambridgeshire, UK
    Posts
    146
    Issue reported in github, working on amended class and test code.

  6. #6
    Senior Member
    Join Date
    Apr 2021
    Location
    Cambridgeshire, UK
    Posts
    146
    Amended class now available on https://github.com/h4yn0nnym0u5e/Aud...y_queue_stall: there's an example sketch in the (new) examples/Queues folder. Need to do the GUI documentation now done, too...
    Last edited by h4yn0nnym0u5e; 11-20-2021 at 05:44 PM.

  7. #7
    Senior Member
    Join Date
    Apr 2021
    Location
    Cambridgeshire, UK
    Posts
    146

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •