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

feat(usb): Prepare embassy-usb for USB Audio, and add USB Audio Class 1.0 (playback only) #3212

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

elagil
Copy link
Contributor

@elagil elagil commented Jul 26, 2024

Hello,

I implemented a UAC 1.0 device, but this required some extensions and fixes to the implementation of embassy-usb.
Some changes are taken from or inspired by #2736.

Here is an example of where this is in use (still rough): https://github.com/blus-audio/firmware-rs
The feedback endpoint is not yet in use.

Features:

  • Added usage type and synchronization type settings for endpoints. This is only relevant for isochronous endpoints.
  • Added extra_fields that can be appended to an endpoint descriptor. This is required for UAC (e.g. bRefresh and bSynchAddress fields for UAC1.0)
  • Added functionality for allocating endpoints and writing their descriptors separately. This is required, because in UAC1.0, the synch endpoint must follow the audio streaming endpoint, but the descriptor of the latter must contain the endpoint index of the former. So I allocate the endpoints first, then write the descriptors with the information about the endpoints. Find an implementation of that approach here: https://github.com/blus-audio/firmware-rs/blob/main/firmware/src/uac1/mod.rs#L223

Fixes:

  • Set correct even/odd frame bit for isochronous endpoints. Otherwise, half of the packets are lost.
  • Consistent naming of register setters

@kalkyl
Copy link
Contributor

kalkyl commented Jul 26, 2024

Awesome!
Run cargo +nightly fmt to fix CI

Do you have a basic usage example to include?

@elagil
Copy link
Contributor Author

elagil commented Jul 26, 2024

Thanks! I just fixed formatting.

I could add an example for the STM32H7, because I can test that.
In my case, I stream USB->SAI->DAC.

However, such an example would still be a bit useless without explicit feedback. There will be jumps in the audio due to the rate discrepancies of SAI output and USB input. Measuring the value that has to be provided over the feedback endpoint is hardware dependent.

I will try to make something useful.

@elagil
Copy link
Contributor Author

elagil commented Jul 26, 2024

I guess I have to do this first, in order to create an example: embassy-rs/stm32-data#509

Maybe some details for the reasoning:
The fix allows v2 timers to trigger on the USB_SOF signal, which will be used for measuring the feedback value.
The feedback value expresses, how many samples were consumed by the audio device between two USB_SOF interrupts. It thus relates the rate of sample consumption to the rate at which input samples are provided. The feedback value is sent to the host, in order for it to adjust the rate at which it sends samples. It can be thought of as a control loop, where the USB audio device is the sensor.

@elagil elagil changed the title feat(usb): Prepare embassy-usb for USB Audio feat(usb): Prepare embassy-usb for USB Audio, and add USB Audio Class 1.0 (playback only) Jul 29, 2024
@elagil elagil marked this pull request as draft July 29, 2024 15:25
@elagil elagil force-pushed the feat_usb_prepare_for_uac branch 2 times, most recently from 1190b76 to 42905b2 Compare August 24, 2024 17:58
@elagil elagil marked this pull request as ready for review August 24, 2024 17:58
@elagil
Copy link
Contributor Author

elagil commented Aug 24, 2024

This should now be ready for review. You can go through the commits one-by-one:

  • Preparation of embassy-usb for USB audio
  • Implementation of UAC1
  • An example, based on STM32F4

The class is meant to be extended by more "applications" in the future. I implemented only a "speaker" application, where the device receives audio from the host. It has the following features:

  • Volume control
  • Mute control
  • Selectable sample rate
  • Selectable sample width
  • Implementation of the sample rate feedback mechanism (see example for details)
  • Separate streaming, control, and feedback instances

Disclaimer: I did not test the example on the STM32F4 platform, because I don't have that hardware. I only tested on my STM32H723, which does not exist as an example platform, and I feel like it makes more sense to use the more common F4.

Can someone please test it? For example, print the measured feedback value (in the usb_feedback_task) and received sample lengths (in the audio_receiver_task). Ideally, stream over SAI and play some music ;)

@elagil
Copy link
Contributor Author

elagil commented Sep 5, 2024

ISO endpoint support itself in the USB drivers was split off to #3314.

Therefore, this depends on the other PR and does not build without it.

@kalkyl
Copy link
Contributor

kalkyl commented Oct 21, 2024

Is this PR still blocked by something?

@elagil
Copy link
Contributor Author

elagil commented Oct 21, 2024

No, it should be fine. I am only waiting for review and maybe for someone to restart the build job.

I may also rebase it.

@kalkyl
Copy link
Contributor

kalkyl commented Oct 23, 2024

Looks great to me!

@vDorst
Copy link
Contributor

vDorst commented Oct 23, 2024

@elagil I am trying out your code on my Blackpill (STM32F401 with a PCM5102 DAC without MCLK (I2S3)).

I am the feeling that you are using the SOF capture not in the way it was intended.

Ideally you can run timer2 in normal mode and use channel 1 as a capture channel.
But your have to route the SOF signal to the trigger input.

One crucial part is setting the option registers.
But that is very microcontroller specific, you have to look in the Referance manuals what the options are.

// See RM0368 TIM2 option register (TIM2_OR)
    tim2.regs_gp32().or().write(|r| *r = 0b10 << 10);

This is how I currently have wire it.
vDorst/stm32_audio@2a8c20c

Other thing I see is that something is flaky.
I mean that I get different errors/panic while the Linux is setting-up the USB/soundcard.
If I press the reset, I may the an other error or it just works fine.

Like issue in USB driver

0.848297 INFO  Volume changed to Muted on channel LeftFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
0.848358 INFO  Volume changed to Muted on channel RightFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
0.851623 DEBUG USB set interface number InterfaceNumber(1) to alt setting 0.
└─ embassy_usb::class::uac1::speaker::{impl#10}::set_alternate_setting @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:745 
0.851776 DEBUG Got channel 1 mute state: true.
└─ embassy_usb::class::uac1::speaker::{impl#9}::interface_get_request @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:660 
0.851989 DEBUG Got channel 2 mute state: true.
└─ embassy_usb::class::uac1::speaker::{impl#9}::interface_get_request @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:660 
0.852386 ERROR panicked at /home/r/dev/rust/embassy/embassy-usb-synopsys-otg/src/lib.rs:46:9:
assertion failed: ep_num < ep_count

Index out of bound

0.681365 INFO  Volume changed to DeciBel(0.0) on channel LeftFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
0.681427 INFO  Volume changed to DeciBel(0.0) on channel RightFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
0.681701 INFO  endpoint_set_request: Request { direction: Out, request_type: Class, recipient: Endpoint, request: 1, value: 256, index: 1, length: 3 } []
└─ embassy_usb::class::uac1::speaker::{impl#9}::endpoint_set_request @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:588 
0.681823 ERROR panicked at /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:606:36:
index out of bounds: the len is 0 but the index is 0

BufferOverflow in USB

21.306182 WARN  control accept_in failed: BufferOverflow
└─ embassy_usb::{impl#1}::handle_control_in::{async_fn#0} @ /home/r/dev/rust/embassy/embassy-usb/src/lib.rs:383 

When it works and I change the volume, then I see the change in the probe-rs output.
again a assertion failed: ep_num < ep_count

16.083160 DEBUG Set channel 1 volume: -4608
└─ embassy_usb::class::uac1::speaker::{impl#9}::interface_set_volume @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:533 
16.083251 INFO  Volume changed to DeciBel(-18.0) on channel LeftFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
16.083343 INFO  Volume changed to DeciBel(-17.0) on channel RightFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
16.083435 INFO  interface_set_request: Request { direction: Out, request_type: Class, recipient: Interface, request: 1, value: 514, index: 512, length: 2 } [0, 238]
└─ embassy_usb::class::uac1::speaker::{impl#9}::interface_set_request @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:543 
16.083679 DEBUG Set channel 2 volume: -4608
└─ embassy_usb::class::uac1::speaker::{impl#9}::interface_set_volume @ /home/r/dev/rust/embassy/embassy-usb/src/class/uac1/speaker.rs:533 
16.083770 INFO  Volume changed to DeciBel(-18.0) on channel LeftFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
16.083862 INFO  Volume changed to DeciBel(-18.0) on channel RightFront.
└─ stm32_audio::__usb_control_task_task::{async_fn#0} @ src/main.rs:223 
16.130187 ERROR panicked at /home/r/dev/rust/embassy/embassy-usb-synopsys-otg/src/lib.rs:47:13:
ep num: 14 cnt 4
assertion failed: ep_num < ep_count

I have the feeling that the stm32-usb driver is buggy.

I am using embassy commit 8eb80c6
Os: OpenSUSE with kernel 6.11.3-2-default
Rust: rustc 1.82.0 (f6e511eec 2024-10-15)

@elagil
Copy link
Contributor Author

elagil commented Oct 24, 2024

I am the feeling that you are using the SOF capture not in the way it was intended.

Ideally you can run timer2 in normal mode and use channel 1 as a capture channel. But your have to route the SOF signal to the trigger input.

One crucial part is setting the option registers. But that is very microcontroller specific, you have to look in the Referance manuals what the options are.

Yes, you are right. By just reading the counter value in the interrupt, there will be some error due to delays. Using capture should fix that. I will also enable the missing routing of the trigger input for F4.

I have the feeling that the stm32-usb driver is buggy.

That looks like it, unfortunately. Do you have an idea what might cause it? I think I have some F401 board that I can test this on as well. The H7 (that I tested with) and the F4 use the same synopsis OTG driver, but there are some device-specific config-functions. I will have a look at those. Another idea is that there is some issue with the FIFO sizes.

@elagil
Copy link
Contributor Author

elagil commented Oct 25, 2024

@vDorst I can reproduce your issues with some custom F401 board that I have.

Some configuration values are not received correctly. For example, in dmesg I see

[ 1329.474212] usb 3-5.1: current rate 458752 is different from the runtime rate 48000
[ 1344.887822] usb 3-5.1: 2:0: failed to get current value for ch 2 (-110)

Some investigation will be required.

@vDorst
Copy link
Contributor

vDorst commented Oct 26, 2024

@elagil I have no idea what is causes it.

I have seen these dmesg items too.

I also looked at the USB description.
Simply dumping the builder.config_descriptor:

        let pos = builder.config_descriptor.position();
        let buf = &builder.config_descriptor.buf[0..pos];
        defmt::info!("USB Config Desc: [{}] {:02x}", pos, buf);

But I don´t see any strange values

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