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

experimental change support async audio and other enhancements #1

Closed
wants to merge 10 commits into from

Conversation

gneverov
Copy link
Owner

@gneverov gneverov commented Mar 6, 2023

The core of this change is to start using "streams" as a common abstraction for IO. The stream protocol is an existing (though seemingly unused) part of MicroPython. MicroPython streams would be more commonly identified as "file-like objects" in CPython. In this change, audio out devices, files, sockets, and MP3 decoders are all streams.

If you want to jump to the end, this example shows a network streaming MP3 player. The example is not actually async yet because it depends on the socket api being async. And the public Python api is not polished.

The use of streams allows the developer to source their audio data from anywhere. (You can write your own stream-like class in Python.) For example, a network socket, a microphone, and have your own transformers in between (e.g., to change format, demux/reassemble data, observing the raw data).

The behavior of the stream interface (e.g., how blocking, eof, partial transfers are handled) is already specified by CPython. This behavior can be tricky at times, so standardization and reuse are important to facilitate composition across the wide array of modules and devices that CircuitPython supports. In subsequent changes, the stream abstraction will also serve a common entry point for supporting awaitable operations.

Other notes about the change:

  • The change removes the audiocore module as it is no longer needed. The wav file reader can be implemented in pure Python. And raw samples just becomes BytesIO.
  • In order to make MP3 decoding more efficient, I move some functions to ram. The MP3 decoder is in a separate repo, and this is the corresponding PR.
  • I've only implemented PWM audio. I haven't done anything with I2S audio yet.
  • The change also removes audio_dma, the use of background tasks, and the audio buffer protocol.

Copy link

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

Several questions not related to streams.

@@ -175,7 +175,7 @@ endif
# Remove -Wno-stringop-overflow after we can test with CI's GCC 10. Mac's looks weird.
DISABLE_WARNINGS = -Wno-stringop-overflow -Wno-cast-align

CFLAGS += $(INC) -Wall -Werror -std=gnu11 -nostdlib -fshort-enums $(BASE_CFLAGS) $(CFLAGS_MOD) $(COPT) $(DISABLE_WARNINGS) -Werror=missing-prototypes
CFLAGS += $(INC) -Wall -Werror -std=gnu11 -fshort-enums $(BASE_CFLAGS) $(CFLAGS_MOD) $(COPT) $(DISABLE_WARNINGS) -Werror=missing-prototypes
Copy link

Choose a reason for hiding this comment

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

Could you explain why you are now using stdlib?

Comment on lines +81 to +86
uint8_t ch = (uint8_t)uart_get_hw(uart)->dr;
if (ch == mp_interrupt_char) {
mp_sched_keyboard_interrupt();
continue;
}
ringbuf_put(r, ch);
Copy link

Choose a reason for hiding this comment

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

I don't understand why you are checking for ctrl-C in raw UART reading.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Every port is meant to do this, but it was missing from this port. This is how ctrl-C is implemented. I discovered this because I was using the uart instead of usb cdc and ctrl-c didn't work.

Copy link

Choose a reason for hiding this comment

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

But people use UART's to send and receive binary data, not just to the REPL, and ctrl-C is a valid binary data character. I don't think you want to check at this level of the code.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I see. I thought I copy-pasted this from another port's code. I will check where this is coming from.

Comment on lines +1 to +11
#include <stdlib.h>

#include "py/gc.h"

void *malloc(size_t size) {
return gc_alloc_possible() ? gc_alloc(size, 0, false) : NULL;
}

void free(void* ptr) {
gc_free(ptr);
}
Copy link

Choose a reason for hiding this comment

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

I don't know why you are doing this instead of using the usual gc alloc routijnes.

Copy link
Owner Author

Choose a reason for hiding this comment

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

This was just to help port C libraries that rely on malloc from libc. It's not related to streams.

@gneverov gneverov closed this Mar 22, 2023
@gneverov gneverov deleted the async_audio branch November 8, 2023 03:24
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.

2 participants