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

UAC2: Implement feedback by fifo counting. #2328

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

HiFiPhile
Copy link
Collaborator

@HiFiPhile HiFiPhile commented Nov 19, 2023

Describe the PR
Finally I had time to integrate my feedback work into UAC2 class :)

Compared to other methods the only thing needed is add a callback to set sample rate, all calculations are done inside the class.

void tud_audio_feedback_params_cb(uint8_t func_id, uint8_t alt_itf, audio_feedback_params_t* feedback_param)
{
  // Set feedback methode to fifo counting
  feedback_param->method = AUDIO_FEEDBACK_METHOD_FIFO_COUNT;
  feedback_param->sample_freq = current_sample_rate;
}

A new example uac2_speaker_fb has been added to show how the mechanism work with a stereo speaker. Both FS (STM32F429) and HS (LPC55S69) configs are tested and FIFO level is well maintained half filled.


A HID debug interface can be enabled by setting CFG_AUDIO_DEBUG=1 which export UAC class information and can be read by audio_debug.py.

image

@HiFiPhile
Copy link
Collaborator Author

Test welcomed @Protoxy22 @Lurcy38 @kf6gpe @PanRe

@Protoxy22
Copy link

Protoxy22 commented Nov 20, 2023

Wow great work, I will have time to test this next week on an STM32H743 Eval Board with USB HS

Thank you

@Hal9000Space
Copy link

This is interesting. I'm curious where can I find audio_debug.py. Any ideas? Thanks a lot!

@HiFiPhile
Copy link
Collaborator Author

This is interesting. I'm curious where can I find audio_debug.py. Any ideas? Thanks a lot!

Just in the PR examples/device/uac2_speaker_fb/src/audio_debug.py

@Hal9000Space
Copy link

This is interesting. I'm curious where can I find audio_debug.py. Any ideas? Thanks a lot!

Just in the PR examples/device/uac2_speaker_fb/src/audio_debug.py

Ah, so not in main yet. Ok, thanks!

@Lurcy38
Copy link
Contributor

Lurcy38 commented Dec 1, 2023

great :) , I will be very happy to test this implementation in my app, but i cannot reach the branch rx_fb , how to do ?

@HiFiPhile
Copy link
Collaborator Author

@Lurcy38 it's under my user space
https://github.com/HiFiPhile/tinyusb/tree/rx_fb
Or you can download PR by adding .patch
https://github.com/hathach/tinyusb/pull/2328.patch

@HiFiPhile
Copy link
Collaborator Author

@hathach After more tests this PR is ready.

@mastupristi
Copy link

Hi, what is the status of this PR? what is missing for it to be approved?

@Marcus2060
Copy link

Hi, great work,
very nice feedback method improvement on Tinyusb class Audio !
I Have implemented the feedback method FIFO_COUNTER (stereo 16 bit, 48KHz) on a my RP2040 hardware with I2S DAC with success .. but ...
I observe a good audio USB reproduction on Windows but the system crashes on MAC computer...???
If, instead, I remove only the feedback EP, the system It's starting to work on MAC too ! But obviously not sync ...

Could this be due to the fact that macOS does not accept this feedback method or some other reason such as incorrect descriptors for the MAC?

Thanks!

@mastupristi

This comment was marked as off-topic.

@mastupristi

This comment was marked as off-topic.

@mastupristi

This comment was marked as off-topic.

@Protoxy22

This comment was marked as off-topic.

@mastupristi

This comment was marked as off-topic.

@HiFiPhile
Copy link
Collaborator Author

Back from holiday...

I would add TUSB_ISO_EP_ATT_EXPLICIT_FB on the data endpoints, just to be sure that all OS are suggested to use the available feedback endpoint @HiFiPhile ?

No it's not how it works...

@HiFiPhile
Copy link
Collaborator Author

HiFiPhile commented Feb 27, 2024

I observe a good audio USB reproduction on Windows but the system crashes on MAC computer...???

@Marcus2060 What do you mean crash, does the kernel panic or having popping sound or something else ?

You can try to change

#define CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION 0 // 0 or 1

the choice of format is left to the caller and feedback argument is sent as-is. If CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION is set, then tinyusb
expects 16.16 format and handles the conversion to 10.14 on FS.
Note that due to a bug in its USB Audio 2.0 driver, Windows currently requires 16.16 format for _all_ USB 2.0 devices. 

From other people's experience it seems this format incompatibility issue exist.

On a FS interface, is there a good way to switch between the feedback formats for explicit feedback?

Windows expects 16.16. I get that. It looks very much like macOS and related Apple products adhere closer to the USB spec, and want 14.10 for the clock feedback.

I see there are 3 solutions:

  • Identify host OS on enumeration and change format correction on the fly : TODO, could be tricky to implement
  • Change the MCU and use high-speed
  • Ask Microsoft to fix their code

@Marcus2060
Copy link

Hi,
I continued working with a Full-Speed USB audio system RP2040 48KHz 16 bit by using feedback explicit endpoint.
(TinyUSB with FIFO counter feedback mechanism).

I've done other experiments and I get the impression that the usbd_edpt_xfer() function on low-level stack doesn't work well when we try to send 3 bytes.
These are my two scenarios:

MAC:

  1. The system works fine with 10.14 feedback (CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION=1) but only if I set MaxPacketBytes=3 in the feedback descriptor.
  2. The system doesn't work if I try to send 3 Bytes (10.14) via usbd_edpt_xfer() but with MaxPacketBytes=4 ??? (MaxPacketBytes means Max number of bytes transfer on the endpoint ??...)
  3. The system doesn't work with 16.16 feedback (4 bytes sent) because seems that macOS does not accept 16.16 for full speed devices.

Windows:

  1. The system works well with 16.16 feedback (4 bytes sent).
  2. The system doesn't work with 10.14 format (CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION=1) when I try to send 3 bytes by usbd_edpt_xfer() (MaxPacketBytes = 4 in the descriptor) ... I listen some click audio due, as we know, with more or less bytes on Audio buffer respect to the DAC buffer...
  3. If I use, instead, the same code of MAC (as above MaxPacketBytes = 3 in the descriptor) unfortunately the system stops working because the Audio class it is not enumerate on WIndows and I no longer see the card ... ???

Also:
I tested the pico-playground usb_sound_card project (Pico board + DAC board) and it works well both Windows and MAC. If we see the code (usb_sound_card.c) seems that feedback routine uses 10.14 format putted in 3 bytes.

So, could the problem be on the low level of the Tinyusb stack when we try to send 3 bytes feedback via usbd_edpt_xfer() to Host ???

Thanks!

@HiFiPhile
Copy link
Collaborator Author

So, could the problem be on the low level of the Tinyusb stack when we try to send 3 bytes feedback via usbd_edpt_xfer() to Host ???

No it can't be, you also tested 3 bytes works on MAC.

If I use, instead, the same code of MAC (as above MaxPacketBytes = 3 in the descriptor) unfortunately the system stops working because the Audio class it is not enumerate on WIndows and I no longer see the card ... ???

It's true.

I tested the pico-playground usb_sound_card project (Pico board + DAC board) and it works well both Windows and MAC. If we see the code (usb_sound_card.c) seems that feedback routine uses 10.14 format putted in 3 bytes.

Pico use UAC 1.0 while TUSB uses UAC 2.0, you can see their descriptors are quite different.

@Marcus2060
Copy link

Hi,
thank you for reply and sorry if I try to clarify better:

No it can't be, you also tested 3 bytes works on MAC.

No, my previous comment is precisely because I tested that the 3 bytes don't seem to work (see my pont 1, 2 about MAC):

  1. The system works fine with 10.14 feedback (CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION=1) but only if I set MaxPacketBytes=3 in the feedback descriptor.
  2. The system doesn't work if I try to send 3 Bytes (10.14) via usbd_edpt_xfer() but with MaxPacketBytes=4 (MaxPacketBytes means Max number of bytes transfer on the endpoint ??...)

I highlighted the 3 bytes sent problem because:
Point 1 could it be a mechanism where the low-level USB stack forces anyway the 3 bytes ??
Point 2 why doesnt'work while point 1 works ??

Thanks anyway for your attention.

@HiFiPhile
Copy link
Collaborator Author

HiFiPhile commented Mar 4, 2024

@Marcus2060

The system doesn't work if I try to send 3 Bytes (10.14) via usbd_edpt_xfer() but with MaxPacketBytes=4 (MaxPacketBytes means Max number of bytes transfer on the endpoint ??...)

Sorry I've some problem understanding, you mean 3 Bytes && MaxPacketBytes=4 doesn't work ? How about 3 Bytes && MaxPacketBytes=3 ?

@Marcus2060
Copy link

Sorry I've some problem understanding, you mean 3 Bytes && MaxPacketBytes=4 doesn't work ? How about 3 Bytes && MaxPacketBytes=3 ?

Below 4 different tests on MAC using 10.14 format:

  1. When (4 Bytes sent) && (MaxPacketBytes=4) -> Wrong Audio (I think the macOS recognize 16.16).
  2. When (3 Bytes sent) && (MaxPacketBytes=4) -> No Sync, some clicks.
  3. When (3 Bytes sent) && (MaxPacketBytes=3) -> No Sync, some clicks.
  4. When (4 Bytes sent) && (MaxPacketBytes=3) -> Works fine, no click!

Below the function to send feedback (feedback value is an uint32_t) :
usbd_edpt_xfer(audio->rhport, audio->ep_fb, (uint8_t *) &value, 4);

@lijunru-hub
Copy link
Contributor

Testing this feature with ESP32-S3 has been very successful. The device plays sound without any noise, and the FIFO consistently maintains a stable intermediate level.

@HiFiPhile
Copy link
Collaborator Author

HiFiPhile commented May 11, 2024

@Marcus2060 @rhysmorgan134
I've setup up a Hackintosh Ventura and tested 3 bytes feedback (3 Bytes sent) && (MaxPacketBytes=3)and it works very well.
Zero click & pop during more than one hour listening:
image

So I'm curious about why it doesn't work in your cases.
Only thing I come up is when 4 bytes are sent 2ms is needed (3+1) instead of 1ms to finish one feedback transfer, maybe it makes OSX's rate adjusting speed different ?
Maybe I2S DMA buffer is too big ? It should be at least smaller than 1/4 of UAC FIFO size to avoid underflow/overflow when 2 packets are popping/pushing from/to the buffer consecutively.


Anyone interested can find the code:
https://gist.github.com/HiFiPhile/9ed6f4f65bf1dc5bc0c7626fb7700d8d

@rhysmorgan134
Copy link

@Marcus2060 @rhysmorgan134 I've setup up a Hackintosh Ventura and tested 3 bytes feedback (3 Bytes sent) && (MaxPacketBytes=3)and it works very well. Zero click & pop during more than one hour listening: image

So I'm curious about why it doesn't work in your cases. Only thing I come up is when 4 bytes are sent 2ms is needed (3+1) instead of 1ms to finish one feedback transfer, maybe it makes OSX's rate adjusting speed different ? Maybe I2S DMA buffer is too big ? It should be at least smaller than 1/4 of UAC FIFO size to avoid underflow/overflow when 2 packets are popping/pushing from/to the buffer consecutively.

Anyone interested can find the code: https://gist.github.com/HiFiPhile/9ed6f4f65bf1dc5bc0c7626fb7700d8d

Thanks for looking into this so much! I will take a look though your implementation and match buffer sizes and then feedback.

@rhysmorgan134
Copy link

@HiFiPhile Seems you are right, setting my DMA buffers to 64 samples, 128bytes in my case, works fine with 3 bytes set in the transmit function and maxPacketBytes==3.

Are you using the tusb_config.h and descriptors from the example? If not would you mind sharing your ones you used for your test, would be nice for me to just sanity check mine.

Also did you run into any difficulties getting the python script running on OSX?

@HiFiPhile
Copy link
Collaborator Author

HiFiPhile commented May 12, 2024

Are you using the tusb_config.h and descriptors from the example

Yes they are from stack example, only difference is enable correction and set wMaxPacketSize=3:

diff --git a/examples/device/uac2_speaker_fb/src/tusb_config.h b/examples/device/uac2_speaker_fb/src/tusb_config.h
index 1cc1031c1..1a1cebca8 100644
--- a/examples/device/uac2_speaker_fb/src/tusb_config.h
+++ b/examples/device/uac2_speaker_fb/src/tusb_config.h
@@ -123,7 +123,7 @@ extern "C" {
 #define CFG_TUD_AUDIO_FUNC_1_DESC_LEN                                TUD_AUDIO_SPEAKER_STEREO_FB_DESC_LEN

 // Enable if Full-Speed on OSX, also set feedback EP size to 3
-#define CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION              0
+#define CFG_TUD_AUDIO_ENABLE_FEEDBACK_FORMAT_CORRECTION              1

 // Audio format type I specifications
 #if defined(__RX__)
diff --git a/examples/device/uac2_speaker_fb/src/usb_descriptors.c b/examples/device/uac2_speaker_fb/src/usb_descriptors.c
index fa307674d..5d84024ea 100644
--- a/examples/device/uac2_speaker_fb/src/usb_descriptors.c
+++ b/examples/device/uac2_speaker_fb/src/usb_descriptors.c
@@ -150,7 +150,7 @@ uint8_t const desc_configuration[] =
     TUD_CONFIG_DESCRIPTOR(1, ITF_NUM_TOTAL, 0, CONFIG_TOTAL_LEN, 0x00, 100),

     // Interface number, string index, byte per sample, bit per sample, EP Out, EP size, EP feedback, feedback EP size,
-    TUD_AUDIO_SPEAKER_STEREO_FB_DESCRIPTOR(0, 4, CFG_TUD_AUDIO_FUNC_1_N_BYTES_PER_SAMPLE_RX, CFG_TUD_AUDIO_FUNC_1_RESOLUTION_RX, EPNUM_AUDIO_OUT, CFG_TUD_AUDIO_FUNC_1_EP_OUT_SZ_MAX, EPNUM_AUDIO_FB | 0x80, 4),
+    TUD_AUDIO_SPEAKER_STEREO_FB_DESCRIPTOR(0, 4, CFG_TUD_AUDIO_FUNC_1_N_BYTES_PER_SAMPLE_RX, CFG_TUD_AUDIO_FUNC_1_RESOLUTION_RX, EPNUM_AUDIO_OUT, CFG_TUD_AUDIO_FUNC_1_EP_OUT_SZ_MAX, EPNUM_AUDIO_FB | 0x80, 3),

 #if CFG_AUDIO_DEBUG
     // Interface number, string index, protocol, report descriptor len, EP In address, size & polling interval

Also did you run into any difficulties getting the python script running on OSX?

There are some dependencies needs install:

sudo pip3 install hid
sudo pip3 install matplotlib

@HiFiPhile
Copy link
Collaborator Author

Last commit optimized debug interface fifo count, it's not the real count inside UAC class but a replication, it was less accurate.

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

Successfully merging this pull request may close these issues.

None yet

8 participants