microphone: PDM MEMS microphone support using I2S interface #50
Conversation
deadprogram
force-pushed
the
features/microphone
branch
from
b9d2809 to
7e29aec
Compare
|
Any implementation feedback for this PR? |
| Mode: machine.I2SModePDM, | ||
| AudioFrequency: defaultSampleRate * quantizeSteps / 16, | ||
| ClockSource: machine.I2SClockSourceExternal, | ||
| Stereo: true, |
There was a problem hiding this comment.
I see this only now after the I2S PR was merged but a channel count might have been more flexible.
On the other hand, AFAIK I2S has really been built for mono/stereo and there are very few DAC/ADCs with more than two channels. I could be wrong, though.
There was a problem hiding this comment.
A bit of research shows a number of multi-channel chips, but not too many MCUs. I suggest we defer that until a future iteration.
microphone/microphone.go
Outdated
| d.data = make([]int32, d.SampleCountForSPL) | ||
| d.buf = make([]uint32, (quantizeSteps / 16)) | ||
|
|
||
| return |
There was a problem hiding this comment.
Nit: this return is unnecessary.
| sum = applySincFilter(d.buf) | ||
|
|
||
| // adjust to 10 bit value | ||
| s := int32(sum >> 6) |
There was a problem hiding this comment.
Is there a reason to cap at 10bit?
While cheap microphones probably don't have anything but noise beyond that point, it seems better to me to not lose bits unnecessarily.
There was a problem hiding this comment.
This might be somewhat specific to the mic in the Circuit Playground Express. Might be good to make that a setting.
There was a problem hiding this comment.
Why a setting? No common devices are more efficient at 10bit than at 16bit (and most even support 32bit just as efficiently). Using more bits always avoids all this and is most likely both simpler to implement and to use, apart from being more efficient.
Or am I missing something?
There was a problem hiding this comment.
I added a separate method ReadWithFilter() to provide this capability, and put in a simpler raw Read() implementation instead for the more general cases.
microphone/microphone.go
Outdated
| for i := 0; i < len(d.data); i++ { | ||
| v := d.data[i] | ||
| if v < 0 { | ||
| v *= -1 |
There was a problem hiding this comment.
Why not just v = -v?
The effect should be the same, but -v seems easier to read for me.
There was a problem hiding this comment.
Did not realize you could do that.
| pos := 0 | ||
| for j := 0; j < len(samples); j++ { | ||
| // takes only the low order 16-bits | ||
| sample = uint16(samples[j] & 0xffff) |
There was a problem hiding this comment.
Technically the & 0xffff us unnecessary here but I agree it may be easier to read (to make the capping explicit).
There was a problem hiding this comment.
Good to be explicit here, probably.
deadprogram
force-pushed
the
features/microphone
branch
2 times, most recently
from
24d2b73 to
7654861
Compare
|
This PR is ready for review now. @conejoninja you might be interested in this, as you have a CPE board. You could make a very cool VU meter with it, I bet... ;) |
Signed-off-by: Ron Evans <[email protected]>
deadprogram
force-pushed
the
features/microphone
branch
from
7654861 to
f1b126a
Compare
|
Squashed everything here into single commit. |
This PR implements support for PDM MEMS microphones using I2S interface, such as the microphone built-in to the Adafruit Circuit Playground Express.
The PR in the TinyGo repo tinygo-org/tinygo#331 must be merged before this PR can be merged, since it requires the I2S implementation..