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

tud_audio_read() doesn't return the right value after merge #2739 #2755

Closed
1 task done
ra1nb0w opened this issue Aug 8, 2024 · 8 comments · Fixed by #2762
Closed
1 task done

tud_audio_read() doesn't return the right value after merge #2739 #2755

ra1nb0w opened this issue Aug 8, 2024 · 8 comments · Fixed by #2762
Labels

Comments

@ra1nb0w
Copy link
Contributor

ra1nb0w commented Aug 8, 2024

Operating System

Linux

Board

STM32H563

Firmware

custom build

What happened ?

after c60934e tud_audio_read() returns only half of the requested bytes.
reverting the merge it returns the right bytes.
I read the commits but nothing obvious to me was found.

How to reproduce ?

uint16_t audio_read_bytes = tud_audio_read(&audio_buffer[0U], AUDIO_BLOCK_SIZE * sizeof(int16_t));

Debug Log as txt file (LOG/CFG_TUSB_DEBUG=2)

tinyusb.log.txt

Screenshots

No response

I have checked existing issues, dicussion and documentation

  • I confirm I have checked existing issues, dicussion and documentation.
@ra1nb0w ra1nb0w changed the title tud_audio_read() doesn't return the right value after c60934eedcc363f320cb66695f0034312094bfd8 tud_audio_read() doesn't return the right value after merge #2739 Aug 8, 2024
@ra1nb0w ra1nb0w mentioned this issue Aug 8, 2024
@ra1nb0w
Copy link
Contributor Author

ra1nb0w commented Aug 12, 2024

with latest commits the value is correct. But it is non constant like before the rework.
Instead, the mic is completely unusable now. It is constantly in underrun (tested on linux) but the value returned by tud_audio_write is stable and correct.
I hope these feedback are useful.

@HiFiPhile
Copy link
Collaborator

@hathach I'd a git bisect, it looks like d680424 breaks ISO IN transfer.

image

@hathach
Copy link
Owner

hathach commented Aug 12, 2024

thank @ra1nb0w and @HiFiPhile, sorry I indeeded didn't tested the ISO since I am still not familliar with audio yet. I mostly tested other examples running hil_test.py locally (also added an g0 (32-bit 2048 KB), f0 (1KB), ch32 (512B) to my hil pool to make sure my rework does not break things.

which example you are testing with ? I can use analyzer to capture but can you help a bit to write a python the script to read its payload count, that would make thing easier/faster. Will try to fix this asap.

@HiFiPhile
Copy link
Collaborator

@hathach
For H5 the fix is simple, just a buf_id thing.
https://github.com/HiFiPhile/tinyusb/tree/fsdev_iso_fix

which example you are testing with ? I can use analyzer to capture but can you help a bit to write a python the script to read its payload count, that would make thing easier/faster

It's the audio_4_channel_mic example, since the data is a stream it is a little tricky to validate the output with a script, but if you run plot_audio_samples.py and a get a clean output (sinus, square, saw wave) it should be good.
image


There is another tricky issue on older device, I'm trying to figure it out on L0.

@hathach
Copy link
Owner

hathach commented Aug 12, 2024

@HiFiPhile got it, running that script is good enough. No worries, I also have G0 and H5 which has 32-bit bus (2K USB SRAM). I will do some testing, bi-set and backtrack. It is probably the buffer toggle issue.

@HiFiPhile
Copy link
Collaborator

False alarm for L0, I just forgot disable DEBUG so the timing was all wrong...

@hathach since you are working on it you can integrate the changes if you want.

@hathach
Copy link
Owner

hathach commented Aug 13, 2024

thank you @HiFiPhile , it seems to fix the issue, it is easy to miss epreg bit manipulation :). Though on Linux, the plot does not look as good as your screenshot, but that is no problem, I will find a way to verify these value with a script later so that we could include them in the hil test.

image

@ra1nb0w
Copy link
Contributor Author

ra1nb0w commented Aug 13, 2024

fixed. Thank you very much guys!

the plot is probably the same, the issue is the (horizontal) time scale.

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.

3 participants