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 15 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);

@HiFiPhile
Copy link
Collaborator Author

When (3 Bytes sent) && (MaxPacketBytes=3) -> No Sync, some clicks.

I agree it seems strange but I don't see any issue in TUSB side (maybe something wrong in RP2040 porting ?), could you use Wireshark USB capture to see what happened ?

@Marcus2060
Copy link

Hi,

I agree it seems strange but I don't see any issue in TUSB side (maybe something wrong in RP2040 porting ?), could you use Wireshark USB capture to see what happened ?

  1. Yes, thank you for your suggestion, I tried with Wiresharc and my board connected to Windows: The messages LOG shows that 3 bytes messages of feedback are sent from the TinyUsb stack to Host correctly on size and value !

  2. So the problem seems to be a different behavior of TinyUsb Audio Class 2.0 between Windows, Linux (16.16 ok) and MAC (10.14 ok ... but with the trick MAXPacketBytes=3 ) .... ??

  3. The Rp2040 is a full-speed device and I don't need large audio streaming traffic. So would it be possible with the TinyUsb stack to move from UAC2 (Audio 2.0) to class Audio 1.0 (USB Speaker)
    by changing only the descriptors or relatively little code on stack or on firmware ??

Thanks again for your attention.

@HiFiPhile
Copy link
Collaborator Author

@Marcus2060 I come back to this.

So the problem seems to be a different behavior of TinyUsb Audio Class 2.0 between Windows, Linux (16.16 ok) and MAC (10.14 ok ... but with the trick MAXPacketBytes=3 ) .... ??

No the stack works (tries to) the same way across all OSes, the issue is some OS doesn't respect the UAC specification as I mentioned earlier.

The Rp2040 is a full-speed device and I don't need large audio streaming traffic. So would it be possible with the TinyUsb stack to move from UAC2 (Audio 2.0) to class Audio 1.0 (USB Speaker)
by changing only the descriptors or relatively little code on stack or on firmware ??

Unfortunately the descriptor difference is pretty large, some major changes are needed.


To be honest I don't see a hassle-free way to make it work across all OSes, Windows can be detected as it ask for MS OS 2.0 descriptor, but it happens after the configuration descriptor where MaxPacketBytes is already set.

src/class/audio/audio_device.c Dismissed Show dismissed Hide dismissed
@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.

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.

None yet

7 participants