Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various RP2040 and SAMD audio issues #5079

Merged
merged 7 commits into from Aug 11, 2021
Merged

Conversation

dhalbert
Copy link
Collaborator

RP2040 and SAMD51:

  • Detect when DMA has finished, and stop DMA audio explicitly.
  • Do not accidentally reuse first_buffer supplied by WaveFile or RawSample. Always realloc first_buffer and second_buffer

RP2040:

  • When audio playing is stopped, write a final zero to the output register. This prevents residual PWM tones.
  • Handle buffer size for 8-bit samples properly for 16-bit output.
  • Fail on some edge cases (which may not be possible at the moment).

I don't think this fixes all the current audio issues. There may still be glitches when a display is running at the same time, and I have seen some odd behavior occasionally, specifically:

  • Sometimes RP2040 loses USB connectivity. This may be unrelated to audio, or it may have to do with DMA (both audio and display) starving USB in some way
  • Occasionally I have seen files "play", but only silence is emitted, or the file is truncated.

An observation for future work:

  • The data put in the buffers to play appears to be correct. It is not being mangled on the way in.

I would appreciate testing by folks on both RP2040 and SAMD51. Thanks.

@ladyada
Copy link
Member

ladyada commented Jul 30, 2021

@kattni please try with the macropad demo you made to play audio clips and see if you're getting reasonable playback :)

@kattni
Copy link

kattni commented Jul 30, 2021

I tested the tone/wav demo, modified to play two wav files, and I'm still running into some crunchy playback, and the program hanging after the playback of one file. Worked with Dan on Discord to try out his demo code, which doesn't hang. I have provided him with the modified version of my demo so he can test the code I was using.

@dhalbert
Copy link
Collaborator Author

dhalbert commented Aug 2, 2021

I did some debugging of this over the weekend. The MacroPad library creates and destroys the PWMAudio object on each audio play. At first I thought that was the difference, but it's not. Instead, what I am seeing is that after the second audio play, background tasks are not being run. In particular, the scanning routine for the Keys object is not being called. So button presses don't play anything after the second play. (I am not sure it's consistently the second play, but it seems to be mostly). I replicated this in cut down version of @kattni's test program, and in my own simple test program.

@dhalbert
Copy link
Collaborator Author

dhalbert commented Aug 4, 2021

This is ready to test for serious problems. Still looking at "crackly" audio.

RP2040 and SAMD51:
- Detect when DMA has finished, and stop DMA audio explicitly.
- Do not accidentally reuse `first_buffer` supplied by WaveFile or RawSample. Always realloc `first_buffer` and `second_buffer`

RP2040:
- When audio playing is stopped, write a final zero to the output register. This prevents residual PWM tones.
- Handle buffer size for 8-bit samples properly for 16-bit output.
- Fail on some edge cases (which may not be possible at the moment).
Copy link

@kattni kattni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dhalbert Ok, tested with the MacroPad library using play_file(). Audio, as expected is still crackly, but the code continues running with this build following multiple playbacks.

I thought audio was only crackly on the first play before, and now it is crackly all the time. You may already be addressing this, but I wanted to let you know.

Here is the example I ran with examples previously converted as suggested by you, Dan. I'm pretty sure I sent them to you before, but let me know if you would like them again.

from rainbowio import colorwheel
from adafruit_macropad import MacroPad

macropad = MacroPad()

tones = ["/low_fade_16.wav", "/drama_16.wav", 246, 262, 294, 330, 349, 392, 440, 494, 523, 587]

while True:
    key_event = macropad.keys.events.get()

    if key_event:
        if key_event.pressed:
            macropad.pixels[key_event.key_number] = colorwheel(
                int(255 / 12) * key_event.key_number
            )
            if key_event.key_number == 0 or key_event.key_number == 1:
                macropad.play_file(tones[key_event.key_number])
            else:
                macropad.start_tone(tones[key_event.key_number])

        else:
            macropad.pixels.fill((0, 0, 0))
            macropad.stop_tone()

@dhalbert
Copy link
Collaborator Author

@kattni Assuming this builds, it is ready to try again. I no longer have crackly audio.

Main fix in the latest commit: Saleae pin instrumentation of the buffer filling in dma_audio.c showed that sometimes it gets out of sync at the very front of playing and fills the opposite buffer from what it intended to fill. Removed the flip/flop buffer management booleans from audio_dma_load_next_block() and audio_dma_convert_samples(), and instead pass in the desired buffers or buffer indices directly. In the interrupt handler, record which DMA channels have completed. Then, in the background task routine, fill the buffers associated with those channels.

Other recent changes:

  • Clean up tick enabling/disabling in keypad.
  • Make a length variable in WaveFile.c be 32 bits instead of 16 bits. I think it is potentially >65535.

@dhalbert
Copy link
Collaborator Author

I haven't yet looked at similar changes for SAMD51, but it may need them. nRF is more different but I should also audit that too.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Two minor things. Good otherwise.

ports/raspberrypi/audio_dma.c Outdated Show resolved Hide resolved
ports/raspberrypi/audio_dma.c Outdated Show resolved Hide resolved
Copy link

@kattni kattni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with the same code as in the previous comment. Wav playback is no longer crunchy. There is an audible click at the end of both wav and tone playback, but that will be addressed at a later time.

Thanks for the fix!

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on this!

@tannewt tannewt merged commit ebf0901 into adafruit:main Aug 11, 2021
@dhalbert dhalbert deleted the debug-audio branch August 11, 2021 18:19
@ladyada
Copy link
Member

ladyada commented Aug 11, 2021

@PaintYourDragon plz also test your game sound fx

@PaintYourDragon
Copy link

It’s sounding great! Much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants