changes proposal to audio feedback computation #1463

hathach · 2022-05-13T15:56:17Z

Describe the PR
Proposal update to #1381

src/class/audio/audio_device.c

HiFiPhile · 2022-05-15T13:29:06Z

Since the focus has changed from SOF to UAC feedback, I think we could add something to facilite feedback calculation based on FIFO count.

It's rather a hardware question, let's take an example of 72MHz STM32F1 & 12.288MHz MCLK.

Because MCLK frequency is relatively high to system clock, timer capture & compare shouldn't be used to measure MCLK frequency otherwise the obtained result will be horrible (72/6=12MHz ~ 72/5=14MHz).

Only option is drive the timer with MCLK and count timer_cnt in sof_irq, in this case we have to "press the stop watch" precisely in the irq. DMA can't help as the transfer need to be triggered somehow, I don't aware any MCU which USB SOF can trigger a DMA transfer.

As I said earlier it's impossible to measure MCLK precisely with a MCU, also with @geekbozu's experimentation he also confirmed feedback value need to be tweaked with actual FIFO count.

What do you think ?

PanRe · 2022-05-15T13:51:59Z

I think that would complete all possibilities to determine the feedback value! I think that would be awsome! Do you mean to add some option like FEEDBACK_COMPUTE_FIFO_COUNT, which does not need the SOF interrupt?

HiFiPhile · 2022-05-15T14:16:12Z

Do you mean to add some option like FEEDBACK_COMPUTE_FIFO_COUNT, which does not need the SOF interrupt?

Actually I'm not 100% sure if it should be based on SOF or not. All my tests were based on High-Speed which is easier to regulate the FIFO level, because frame interval is 125us instead of 1ms and less samples are arrived each packet.

It's better to test with FS to see if my method works.

Basically I took a FIFO size of 8*packet size and regulate the level to half full, which gives me a latency of 4*125us=0.5ms which is negligible. In FS the latency will be 4ms which should be okay, but the packet size will be 8 times larger.

hathach · 2022-05-16T09:15:07Z

@HiFiPhile do you have an example code on how to calculate feedback on fifo count work ? I will try to find a way to fit it as well or give some option to do so.

HiFiPhile · 2022-05-16T10:27:42Z

@HiFiPhile do you have an example code on how to calculate feedback on fifo count work ? I will try to find a way to fit it as well or give some option to do so.

Full code is in https://github.com/HiFiPhile/i2s_bridge, UAC related code is in audio_callbacks.c

I defined a feedback min/max of 0.1%, in USB HS spec clock deviation is 0.05% so 0.1% should be good if same type of oscillator is used for MCLK.
https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L101
Actual min/max value is updated each time sample frequency is changed: https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L179
Each time a packet is arrived I update a filtered FIFO count value with fixed-point math:
https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L210
Playback starts after FIFO is half full buffer_ready == true to avoid starving on startup.
Feedback value is updated in tud_audio_fb_done_cb
https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L237

My 0.1% magic number maybe need more proof, only thing close is in UAC spec:

In the case of an adaptive isochronous data endpoint that support only a discrete number o
sampling frequencies, the endpoint must at least tolerate ±1000 PPM inaccuracy on the reported
Sampling Frequency Control values to accommodate sample clock inaccuracies.

hathach · 2022-05-17T09:52:48Z

thanks @HiFiPhile for sharing your code, I currently switch (yes) to another works. Give me a few days, I will try to update this PR-to-PR to include your feedback-on-fifo-count method.

- also add interval_log2 to isr callback - also rename other variables

src/class/audio/audio_device.c

src/class/audio/audio_device.h

hathach · 2022-05-19T09:12:25Z

src/class/audio/audio_device.h

+enum {
+  AUDIO_FEEDBACK_METHOD_DISABLED,
+  AUDIO_FEEDBACK_METHOD_FREQUENCY_FIXED,
+  AUDIO_FEEDBACK_METHOD_FREQUENCY_FLOAT,
+  AUDIO_FEEDBACK_METHOD_FREQUENCY_POWER_OF_2,
+  AUDIO_FEEDBACK_METHOD_FIFO_COUNT_FIXED,
+  AUDIO_FEEDBACK_METHOD_FIFO_COUNT_FLOAT
+};
+
+typedef struct {
+  uint8_t method;
+  union {
+    struct {
+        uint32_t sample_freq;
+        uint32_t mclk_freq;
+    }frequency;
+
+    struct {
+        uint32_t nominal;
+        uint32_t threshold;
+        // variation etc ..
+    }fifo_count;
+  };
+}audio_feedback_params_t;
+
+
+// TU_ATTR_WEAK void tud_audio_feedback_params_cb(uint8_t func_id, uint8_t alt_itf, audio_feedback_params_t* feedback_param);


@HiFiPhile I made a draft idea on how to add the fifo count method, the params_cb with struct for input. Fifo count method seems to reply on a few params: std (nominal) value, variation, and buffer threshold. The buffer threshold is a bit tricky, since it requires application to respect this. The rest can be computed within the stack. Let me know if I miss something before adding the actual code.

I like the idea of union.

The buffer threshold is a bit tricky, since it requires application to respect this.

Yes indeed. It can be calculated in the stack because normally it's N*PacketSize, or add a macro for easier threshold calculation in application.

https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L186
Here 8000 is for HS (125us), FS need 1000.

Glad you like the idea. I am off for a few days, will put code together next week or so

@HiFiPhile I have a silly question, fifo_count_flt stands for float/fraction part in 16.16 format right ?

https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L113

It's "filtered" fifo count. As fifo count is jumpy each time a packet is arrived / consumed, it's hard to regulate the fifo level based on raw count. So I make an average of fifo count to get a stabilized value.

Basically fifo_count_flt = (fifo_count_old * 255 + fifo_count) / 256, I shifted the whole value <<8 to let the result has enough precision so float is not required.

This value is well tested on HS for sample rate from 44.1kHz to 384kHz, but for FS another value may needed.

thanks @HiFiPhile , I kind of getting the idea, filtered ~ average here

filtered = 255 * old + raw_count feedback = nominal - (filtered/256 - threshold)

out of curiosity, how would you come up with the filtered/avg formula and how does it depend on sample rate, FS/HS link. Sorry for more question, I am trying to figure out the big picture and normalize the argument for this compute method.

out of curiosity, how would you come up with the filtered/avg formula and how does it depend on sample rate, FS/HS link.

I used J-Scope to watch FIFO level, it's kind of trial and error :)

FIFO is filled by USB packets and drained by I2S.

For the same sample rate, since HS packets arrive 8x faster every packet is 8x smaller, FIFO level variation is much smaller so easier to filter.

The higher the sample rate the higher the variation, in theory a higher filter coefficient is needed to achieve a same smoothed result.
But since feedback_std is also proportional to sample rate, which cancelled the need of a higher filter coefficient.
So no need to change filter coefficient on sample rate change.

thanks for the explanation, unfortunately, I still pretty much have no ideas how to generalize this into generic code. Apparently, there is no formula to fit all, I am just hoping to make some skeleton and allow user to simply tweaking a parameter or two with actual hardware. However, this is really out of my knowledge, and don't actually have any real application on hardware to test these out. This is purely due to my inexperience with audio

Since this has been pending for quite some time, I would like to merge this as it is and come back later to this later on. The good news is we have an decent API with place holders union for fifo count method (or any new method we could find afterwards). Should you have time feel free to make an PR to enhance this :)

Protoxy22 · 2023-09-08T07:57:07Z

@HiFiPhile do you have an example code on how to calculate feedback on fifo count work ? I will try to find a way to fit it as well or give some option to do so.

Full code is in https://github.com/HiFiPhile/i2s_bridge, UAC related code is in audio_callbacks.c

1. I defined a feedback min/max of 0.1%, in USB HS spec clock deviation is 0.05% so 0.1% should be good if same type of oscillator is used for MCLK.
   https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L101
   Actual min/max value is updated each time sample frequency is changed: https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L179

2. Each time a packet is arrived I update a filtered FIFO count value with fixed-point math:
   https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L210
   Playback starts after FIFO is half full `buffer_ready == true` to avoid starving on startup.

3. Feedback value is updated in `tud_audio_fb_done_cb`
   https://github.com/HiFiPhile/i2s_bridge/blob/1d6a24fdd0f70d150a88d85d63ed4b583b3b6b30/audio_callbacks.c#L237

My 0.1% magic number maybe need more proof, only thing close is in UAC spec:

In the case of an adaptive isochronous data endpoint that support only a discrete number o
sampling frequencies, the endpoint must at least tolerate ±1000 PPM inaccuracy on the reported
Sampling Frequency Control values to accommodate sample clock inaccuracies.

Hi @HiFiPhile, I would be really interested to see your i2s_bridge repository for the fifo count feedback method.
Is there a way I can access to it ?
Thanks

HiFiPhile · 2023-09-08T08:17:21Z

@Protoxy22 just sent you a invite:)

Protoxy22 · 2023-09-08T08:30:17Z

@Protoxy22 just sent you a invite:)

Thank you !

changes proposal to audio feedback computation

c5ba1ea

hathach requested a review from pigrew May 13, 2022 15:56

fix ci build

20b810d

hathach mentioned this pull request May 13, 2022

Add SOF IRQ Handler #1381

Merged

hathach commented May 13, 2022

View reviewed changes

src/class/audio/audio_device.c Outdated Show resolved Hide resolved

hathach commented May 13, 2022

View reviewed changes

src/class/audio/audio_device.c Outdated Show resolved Hide resolved

PanRe reviewed May 15, 2022

View reviewed changes

src/class/audio/audio_device.c Show resolved Hide resolved

hathach added 2 commits May 19, 2022 13:44

rename tud_audio_sof_isr() to tud_audio_feedback_interval_isr()

5766c9a

- also add interval_log2 to isr callback - also rename other variables

prototype for feedback method

e5113a1

hathach commented May 19, 2022

View reviewed changes

src/class/audio/audio_device.c Outdated Show resolved Hide resolved

hathach commented May 19, 2022

View reviewed changes

src/class/audio/audio_device.h Outdated Show resolved Hide resolved

hathach commented May 19, 2022

View reviewed changes

hathach added 3 commits May 27, 2022 12:27

wrap feedback and compute to its own struct/union

15aa593

adding feedback fifo count (WIP)

dce2ad4

comment out fifo count method for now

f292667

hathach merged commit 223aaea into add-sof-isr May 31, 2022

hathach deleted the sof-isr-update branch May 31, 2022 13:57

HiFiPhile mentioned this pull request Nov 12, 2022

stm32_fsdev isochronous support #1249

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

changes proposal to audio feedback computation #1463

changes proposal to audio feedback computation #1463

hathach commented May 13, 2022

HiFiPhile commented May 15, 2022

PanRe commented May 15, 2022

HiFiPhile commented May 15, 2022 •

edited

hathach commented May 16, 2022

HiFiPhile commented May 16, 2022 •

edited

hathach commented May 17, 2022

hathach May 19, 2022

HiFiPhile May 19, 2022 •

edited

hathach May 19, 2022

hathach May 27, 2022 •

edited

HiFiPhile May 27, 2022 •

edited

hathach May 31, 2022

HiFiPhile May 31, 2022

hathach May 31, 2022 •

edited

Protoxy22 commented Sep 8, 2023

HiFiPhile commented Sep 8, 2023

Protoxy22 commented Sep 8, 2023

changes proposal to audio feedback computation #1463

changes proposal to audio feedback computation #1463

Conversation

hathach commented May 13, 2022

HiFiPhile commented May 15, 2022

PanRe commented May 15, 2022

HiFiPhile commented May 15, 2022 • edited

hathach commented May 16, 2022

HiFiPhile commented May 16, 2022 • edited

hathach commented May 17, 2022

hathach May 19, 2022

Choose a reason for hiding this comment

HiFiPhile May 19, 2022 • edited

Choose a reason for hiding this comment

hathach May 19, 2022

Choose a reason for hiding this comment

hathach May 27, 2022 • edited

Choose a reason for hiding this comment

HiFiPhile May 27, 2022 • edited

Choose a reason for hiding this comment

hathach May 31, 2022

Choose a reason for hiding this comment

HiFiPhile May 31, 2022

Choose a reason for hiding this comment

hathach May 31, 2022 • edited

Choose a reason for hiding this comment

Protoxy22 commented Sep 8, 2023

HiFiPhile commented Sep 8, 2023

Protoxy22 commented Sep 8, 2023

HiFiPhile commented May 15, 2022 •

edited

HiFiPhile commented May 16, 2022 •

edited

HiFiPhile May 19, 2022 •

edited

hathach May 27, 2022 •

edited

HiFiPhile May 27, 2022 •

edited

hathach May 31, 2022 •

edited