hathach
audio_test - Noisy generated to generated saw tooth tone
Operating System
Windows 10
Board
MSP-EXP432E401Y
Firmware
examples/device/audio_test - master branch (commit 6bba41045a4224abb68256dcf2fce893da47a743)
What happened ?
I was tweaking the audio_test example to play back a raw PCM audio file and noticed that the sound seemed noisy. I went back and tried playing the audio from the unmodified audio_test example and it sounded noisy as well. I'm running this test on Windows so I wasn't able to get plot_audio_samples.py working but I am using Audacity to record and play back the audio. Looking at the sound wave you can clearly see some unexpected noise.
I'm curious if anyone else seen this issue on other devices or if this is purely a MSP432E or Mentor IP software implementation issue.
How to reproduce ?
Run audio_test on MSP-EXP432E401Y launchpad. Use Audacity to record the sound coming from the microphone instance.
Debug Log as txt file (LOG/CFG_TUSB_DEBUG=2)
msp432e_audio_test_debug_log.txt
Screenshots
Images of the wave form.
I have checked existing issues, dicussion and documentation
- [x] I confirm I have checked existing issues, dicussion and documentation.
plot_audio_samples.py support Windows, you may need install dependencies and modify device = 'Microphone (MicNode) MME'.
When you did the recording you left logging on or off ?
My issue was the script wouldn't get passed import sounddevice as sd. I'm not sure where I saw it but there is a software library (not just a python one) that I need to install on my Windows PC. The link for it pointed to a page that no longer had a reference to sounddevice for Windows.
No logging was used for my test or screenshot. I just use "make BOARD=msp_exp432e401y" to build the binary without modifying any sources.
So debugging this a bit more. I wanted to repeatedly send a buffer with very recognizable pattern. So I filled test_buffer_audio array with 0x0000,0x1111,0x2222.....0x7777 and then repeat.
I'm seeing this from Audacity
and I'm seeing this in the raw USB data packets.
So it looks like data is being mangled some where.
Looks like I can't attach a .c but here is a diff.
Ok. I think the issue is with the musb dcd layer. For some reason writing 32 bit values to the fifo is causing this corruption issue. Only doing 16 bit or 8 bit writes to the fifo appears to work and leads to the packets received on the PC matching what is expected.
Here are the changes I made.
The packets I receive now matches exactly what I expect.
Going back to the original example I see the following waveform with the above change.
My best guess is that this is a memory aligned issue with the tinyusb fifo being in unaligned memory for 32 bit values and the mentor fifo register having issues with it. Although my attempts to try to align the value with a temp variable or using memcpy didn't seem to solve the problem.
Nice analyze, but the reason is something else.
Even if 32bit values are stored unaligned in FIFO, doing reg->u32 = *(uint32_t const *)addr; will first load the value into CPU register with a LDR instruction, since Cortex-M4 support unaligned memory access it should be ok, then the value is written into USB fifo.
You can test tu_unaligned_read32() for 32bit read loop https://github.com/hathach/tinyusb/blob/334ac8072650e3b18278042c2c2b402db73c7359/src/portable/synopsys/dwc2/dwc2_common.c#L292
Your correct. Since none of my workarounds were working I did feel like something else was going on. Using the above function you mentioned didn't resolve the issue.
Looking at the FIFO register in the MSP432E TRM it seems like for a single packet mixing 32 bit, 16 bit and 8 bit writes are not allowed.
Taking a look at another device with the same Mentor IP it seems like they purely stick with 8 bit writes. https://github.com/yuvadm/tiva-c/blob/master/driverlib/usb.c#L2993
What initially confused me is that each packet in this example was 96 bytes so the current code should be able to write the entire 96 byte packet with 24x 32 bit writes. But I think the issue is that pipe_read_write_packet_ff splits writing a single packet due to dealing with fifo wrap around. So although 96 bytes need to be written its possible that only 34 bytes are written initially and then 62 bytes are written. So I can see within 1 packet this mix match causing a problem.
So a few ideas
- Force only 8 bit read and writes in the fifo.
- Calculate the minimum number of bytes that need to be written for the entire packet and update pipe_write_packet to include a parameter where you specify which type of write operation to use.
- Create a buffer the size of the max fifo size which is 2kb. Copy all the data need to be written into this buffer. Then based on the number of bytes that need to be written specific via param which type of write to perform.
I think option 3 is out of the question. Option 2 may work but its a little risky that a future update may cause this issue to pop up. I'm not sure how much of a lift it would provide over option 1 but that is my current preference.
So although 96 bytes need to be written its possible that only 34 bytes are written initially and then 62 bytes are written. So I can see within 1 packet this mix match causing a problem.
Just read the DS, me too I think it should be the issue.
In fact (3) is already implemented by audio class (the only one who use fifo based transfer) as half of MCUs doesn't support fifo based transfer.
You can test with define TUD_AUDIO_PREFER_RING_BUFFER=0
We can either fix or remove fifo support, @kkitayam what do you think ?