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

Fix UAC encoding, update example. #2259

Merged
merged 9 commits into from
Oct 11, 2023
Merged

Conversation

HiFiPhile
Copy link
Collaborator

@HiFiPhile HiFiPhile commented Sep 19, 2023

Describe the PR
Currently interleaved copy in UAC2 is bugged with wrong stride, resulting samples become scrambled if encoding/decoding is enabled.

#2256, maybe #1722

Additional context
audio_4_channel_mic example use identical sample for all channels, makes the problems less obvious, now it has been updated to output different waveforms each channel.

image

@HiFiPhile HiFiPhile changed the title Fix UAC interleaved copy. Fix UAC encoding, update example. Oct 10, 2023
@HiFiPhile HiFiPhile force-pushed the uac_interl branch 2 times, most recently from c22e907 to 624ef66 Compare October 10, 2023 17:54
@HiFiPhile
Copy link
Collaborator Author

HiFiPhile commented Oct 10, 2023

@PanRe could you take a look when you are free ?

It's tested on stm32g0b1 and stm32f407.

Copy link
Collaborator

@PanRe PanRe left a comment

Choose a reason for hiding this comment

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

Thanks a lot for fixing the bug! Nice work as usual!

@@ -400,7 +420,17 @@ bool tud_audio_tx_done_pre_load_cb(uint8_t rhport, uint8_t itf, uint8_t ep_in, u
(void) ep_in;
(void) cur_alt_setting;

for (uint8_t cnt=0; cnt < CFG_TUD_AUDIO_FUNC_1_N_TX_SUPP_SW_FIFO; cnt++)

// In read world application data flow is driven by I2S clock,
Copy link
Collaborator

Choose a reason for hiding this comment

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

In real world ;)

@PanRe PanRe merged commit 118823c into hathach:master Oct 11, 2023
43 checks passed
@HiFiPhile
Copy link
Collaborator Author

HiFiPhile commented Oct 11, 2023

@PanRe Thanks for your quick return :)

I'll try to optimize IN transfer data flow in a following PR.

For TYPE-I format the allowed sample in each packet is navg+-1, for example 48kHz with a FS device gives 47~49 samples per packet.
image


Currently it always try to send all the data in FIFO. If we take example of a 48 sample length I2S buffer, who writes all samples to UAC FIFO every 1ms:

  • If I2S clock is quicker packet length will be 48 48 48 49 48 48... which works
  • If I2S clock is slower, packet length will be 48 48 48 0(or other low value) 48 48 48 48..., low value could be discarded by the host resulting gap in recording.
image

What I'm think is increase IN FIFO size to at least 4*packet size:

  • If FIFO level is less than 25%, use packet length of navg-1 if enough data, otherwise 0.
  • If FIFO level between 25%-75% use packet length of navg
  • If FIFO level above 75% use packet length of navg+1

SO the above example packet length becomes 48 48 48 47 48 48 48 47.

@PanRe
Copy link
Collaborator

PanRe commented Oct 12, 2023

Interesting idea, but isn't that what you already implemented in one of your projects? I remember you mentioned something like that for the feedback value calculation?

Nevertheless, I like the idea. However, it introduces a lag and some users might not want that. I know, it is hard to get it working without some kind of buffering. How about defining a minimum buffer fill level? The higher it is, the more robust it gets. For users which don't want a buffering, they can set the fill level to zero.

Do we need to need to modify the feedback value depending on the buffer settings?

@HiFiPhile
Copy link
Collaborator Author

Interesting idea, but isn't that what you already implemented in one of your projects? I remember you mentioned something like that for the feedback value calculation?

It was for OUT transfer, in that case user code has the responsibility to calculate feedback correctly. For IN transfer the feedback is implicite, simply determined by how much data is sent.

After thinking current code only support fixed sample rate, it's necessary to calculate navg based on sample rate to let multi-rate work. For example in 48k & 96k case the FIFO size will be 96+1, when 48k is chosen sent packet could be 96 0 96 0....

However, it introduces a lag and some users might not want that.

As the FIFO will be filled in to 50% in average so there will be an additional delay of 1ms for FS or 125us for HS. Compared to OS's latency it's mostly neageible.

For users which don't want a buffering, they can set the fill level to zero.

The only case is when I2S clock perfectly synced with USB clock, like current examples where data is simply looped back.

@PanRe
Copy link
Collaborator

PanRe commented Oct 12, 2023

I see! I think it is a great idea!

@hathach
Copy link
Owner

hathach commented Oct 18, 2023

awesome, great work as usual 👍 👍

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

Successfully merging this pull request may close these issues.

3 participants