Best practice for task and/or callback handling

[LouDnl]

Hello everyone,
I have been using TinyUSB with great pleasure for quite some time now but am sort of struggling to find best practice on how to handle incoming data. My project has a working implementation of CDC, Midi and WebUSB(Vendor) combined based on the many great examples.
At some point I figured out I could use the rx_cb functions for these classes instead of having a dedicated _task function in my main loop, this even seemed to improve incoming buffer handling for all 3 class types.
Enter Vendor class, this class includes the previous buffer in the callback, which at first I found strange but accepted as is.

My project is an audio chip driver that converts incoming data to 8 bit parallel writes using an RPi Pico. This requires very small timings to make this work perfect.
The issue I am having is with WebUSB/Vendor class, this seems much slower then CDC due to async await overhead and the receive confirmation packet back to the host not being sent until the complete 64 byte packet is processed, I think.
I figured out that this was also mainly due to the incoming packets being processed from within the Vendor class callback function. After moving this back into a _task function and calling it from the main loop the timing improved a lot.

My 2 questions are:

1. Should incoming packets be handled from a _task function in the main loop, an _rx_cb function or even both?
   In my local codebase I am now running both _task functions in the main loop and _rx_cb functions both using the same code, this seems to work but has me questioning its use.
2. Is it possible to have the Vendor class send the receive confirmation (URB I think?) before I send off the packet buffer to project code?

Thanks in advance!
LouD

Links to mentioned code

main loop:
https://github.com/LouDnl/USBSID-Pico/blob/0bd346452f8e332d6bbf3ab42dad17e3d7160469/src/usbsid.c#L691-L693

vendor callback:
https://github.com/LouDnl/USBSID-Pico/blob/0bd346452f8e332d6bbf3ab42dad17e3d7160469/src/usbsid.c#L421-L439

vendor task example:

void vendor_task(void)
{
  if (web_serial_connected) { /* vendor class has no connect check, thus use this */
    if (tud_vendor_available()) {
      wusb_itf = WUSB_ITF; /* Since there's only 1 vendor interface, we know it's 0 */
      usbdata = 1, dtype = wusb;
      webread = tud_vendor_read(&read_buffer, MAX_BUFFER_SIZE);
      memcpy(sid_buffer, read_buffer, webread);
      process_buffer(wusb_itf, &webread);
      tud_vendor_read_flush();
      /* memset(read_buffer, 0, count_of(read_buffer)); */
      /* memset(sid_buffer, 0, count_of(sid_buffer)); */
      return;
    }
    return;
  }
  return;
}

[HiFiPhile]

Enter Vendor class, this class includes the previous buffer in the callback, which at first I found strange but accepted as is.Should incoming packets be handled from a _task function in the main loop, an _rx_cb function or even both

There is no standard answer, if data processing is complexe or with a RTOS, processing data in a task give you more flexibility. Otherwise processing in the rx callback has less latency and needs less code.

For audio I usually put tud_task_ext() inside ISR so I can do other things in the main loop.

Enter Vendor class, this class includes the previous buffer in the callback, which at first I found strange but accepted as is.

Vendor class is different than CDC where data received could be individual packets instead of a stream, in which case a fifo backend is less practical since once put into a fifo it's hard to split them.
You can read more in tusb_fifo.c.

To deal with packets you can set CFG_TUD_VENDOR_RX_BUFSIZE=0 to disable the fifo and read data directly in the rx callback. It also saves the time of fifo data copy and flush.

Is it possible to have the Vendor class send the receive confirmation (URB I think?) before I send off the packet buffer to project code?

The next transfer will be scheduled once there is enough space in the rx fifo or fifo is disabled. But bulk transfer (used by vendor or cdc class)
is not suitable for audio since the latency is not deterministic, if there are more devices under the root hub you will expect delay.
You can learn more in https://www.beyondlogic.org/usbnutshell/usb4.shtml#Bulk

[LouDnl]

Thanks for your in depth answer! It figures that there's no standard answer. I didn't know that you could disable the fifo for Vendor class, this is definitely worth a try.
I am not using audio, so that should be fine for now I guess. The midi class is handling sysex packets which in the end are also data to the same device.

[LouDnl]

Reading the link you provided I was wondering if it's possible to send the ACK command from the ISR before I send of the data into program code?

[HiFiPhile]

Reading the link you provided I was wondering if it's possible to send the ACK command from the ISR before I send of the data into program code?

You need a hardware which support double buffer, like esp32, higher tier stm32 or atmel MCUs, I don't use Pi but I don't think it does.

In the rx callback, you can also only do the data copy and put the processing in a separate lower priority ISR like PendSV. #1376 (reply in thread)

[LouDnl]

I think I read something about double buffering for rp2040 but cant seem to find if and how I can configure that.
Using no buffering is a lot faster indeed but not as fast as cdc.
Cant I just redirect webusb to use cdc and its buffering?

[LouDnl]

Setting CFG_TUD_VENDOR_RX_BUFSIZE to 0 and then using the callback buffer and bufsize indeed improves speed a lot, it's not perfect yet but better atleast.

For cdc and midi I now have the same routing in the callback and in a _task which also improves speed, it just seems a bit of overkill but okay.