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

audio_test - Noisy generated to generated saw tooth tone #3010

Open
1 task done
fcooperti opened this issue Mar 6, 2025 · 7 comments · May be fixed by #3017
Open
1 task done

audio_test - Noisy generated to generated saw tooth tone #3010

fcooperti opened this issue Mar 6, 2025 · 7 comments · May be fixed by #3017
Labels

Comments

@fcooperti
Copy link

Operating System

Windows 10

Board

MSP-EXP432E401Y

Firmware

examples/device/audio_test - master branch (commit 6bba410)

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.

Image

Image

I have checked existing issues, dicussion and documentation

  • I confirm I have checked existing issues, dicussion and documentation.
@HiFiPhile
Copy link
Collaborator

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 ?

@fcooperti
Copy link
Author

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.

@fcooperti
Copy link
Author

fcooperti commented Mar 6, 2025

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

Image

and I'm seeing this in the raw USB data packets.

Image

So it looks like data is being mangled some where.

Looks like I can't attach a .c but here is a diff.

Image

@fcooperti
Copy link
Author

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

The packets I receive now matches exactly what I expect.

Image

Going back to the original example I see the following waveform with the above change.

Image

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.

@HiFiPhile
Copy link
Collaborator

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

*tx_fifo = tu_unaligned_read32(src);

@fcooperti
Copy link
Author

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.

Image

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

  1. Force only 8 bit read and writes in the fifo.
  2. 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.
  3. 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.

@HiFiPhile
Copy link
Collaborator

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 ?

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

Successfully merging a pull request may close this issue.

2 participants