Skip to content

Add audio output support!#756

Merged
dhalbert merged 3 commits into
adafruit:masterfrom
tannewt:audio3
Apr 13, 2018
Merged

Add audio output support!#756
dhalbert merged 3 commits into
adafruit:masterfrom
tannewt:audio3

Conversation

@tannewt
Copy link
Copy Markdown
Member

@tannewt tannewt commented Apr 12, 2018

This evolves the API from 2.x (and breaks it). Playback devices are now
separate from the samples themselves. This allows for greater playback
flexibility. Two sample sources are audioio.RawSample and audioio.WaveFile.
They can both be mono or stereo. They can be output to audioio.AudioOut or
audiobusio.I2SOut.

Internally, the dma tracking has changed from a TC counting block transfers
to an interrupt generated by the block event sent to the EVSYS. This reduces
the overhead of each DMA transfer so multiple can occure without using up TCs.

Fixes #652. Fixes #522. Huge progress on #263

This evolves the API from 2.x (and breaks it). Playback devices are now
separate from the samples themselves. This allows for greater playback
flexibility. Two sample sources are audioio.RawSample and audioio.WaveFile.
They can both be mono or stereo. They can be output to audioio.AudioOut or
audiobusio.I2SOut.

Internally, the dma tracking has changed from a TC counting block transfers
to an interrupt generated by the block event sent to the EVSYS. This reduces
the overhead of each DMA transfer so multiple can occure without using up TCs.

Fixes micropython#652. Fixes micropython#522. Huge progress on micropython#263
Copy link
Copy Markdown
Collaborator

@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.

Massive! I couldn't review all the DMA stuff knowledgeably, but tried to look for obvious stuff. Looks great!

Comment thread ports/atmel-samd/clocks.c Outdated
// TODO(tannewt): Should we have a way of sharing GCLKs based on their speed? Divisor doesn't
// gaurantee speed because it depends on the source.
uint8_t find_free_gclk(uint16_t divisor) {
if (divisor > (1 << 8)) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be divisor >= (1 << 8), or maybe clearer divisor > 0xff? Otherwise divisor=0x100 could be used for an 8-bit clock, which won't work

Comment thread ports/atmel-samd/clocks.c
#endif
}

void reset_gclks(void) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's possible that if we need to turn on clocks in a complicated way later (say to enabled crystaled clocks), checking these #define may not be reliable. Just something to watch out for, nothing to change now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup totally. I figure when we make the clock init more complex we can do it in this file and stop using ASF4 completely.

#include "timers.h"

void audioout_reset(void) {
// Only reset DMA. PWMOut will reset the timer. Other code will reset the DAC.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comment is now moot?

audio_dma_stop(&self->right_dma);
#endif

// FIXME(tannewt): Do we want to disable? What if we're sharing with an AnalogOut on the 51?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe forbid this up front? Can't do audio out if the AnalogOut exists?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

Comment thread shared-bindings/audioio/WaveFile.c Outdated
//| :class:`WaveFile` -- Load a wave file for audio playback
//| ========================================================
//|
//| A .wav file prepped for audio playback
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you add doc here about what formats of WAV files are supported? Looks like 8-bit PCM (always unsigned) mono and stereo and 16-bit (always signed) mono and stereo. Is that right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's all I've tested so I added a comment about it.

Comment thread shared-bindings/audioio/RawSample.c Outdated
//| .. class:: RawSample(buffer, *, channel_count=1, sample_rate=8000)
//|
//| Create a RawSample based on the given buffer of signed values. If channel_count is more than
//| 1 then each channel's samples should rotate. In other words, for a two channel buffer, the
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"should rotate" -> "should alternate"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@tannewt
Copy link
Copy Markdown
Member Author

tannewt commented Apr 13, 2018

Thanks for the quick review! I don't expect it to be perfect but am happy to follow up with fixes later.

Copy link
Copy Markdown
Collaborator

@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.

Great!

@dhalbert dhalbert merged commit 10eabf6 into adafruit:master Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants