Investigate PS4 Report Counter Latency

[arntsonl]

Prerequisites

Please check the following before posting an issue / bug report.

• [x] I am running the latest version of the firmware found HERE
• [x] I have checked the documentation HERE and found no answer
• [x] I checked to make sure that this issue has not already been filed HERE
• [x] I am reporting the issue to the correct repository

Context

Please provide all relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

• Firmware Version: 0.7.4-dev
• Name of device: Pico
• Link to where you purchased the device:
• Is this a custom build?:
• Operating System:
• Browser:

Expected Behavior

Adding report counter increment to PS4 data report should NOT increase latency

Current Behavior

Adding report counter increment to PS4 data report does increase latency????

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

1. Modify report counter in the PS4 report
2. Watch latency go up 1ms??

Screenshots & Files

This will require some USB capturing and looking at the values. Maybe the ++ was overflowing our 6-bit value, maybe this value is just never used? We really need to pull USB captures for third-party devices and see how they handle the report overflow issue.

[jfedor2]

Here's what the main loop looks like:

while True:
    button_state = fetch_button_state()
    update_input_report(button_state)
    if tud_hid_ready() and (report != previous_report):
        tud_hid_report(report)
        previous_report = report

This is executed multiple times per USB frame.

Once tuh_hid_report() is called, the report that's going to be sent to the host at the next start-of-frame packet is locked in. Anything that happens after that (even before the start-of-frame packet) will only be taken into consideration on the next frame. If you press button A at t+0.2ms and button B at t+0.6ms, an input report with A pressed will be sent at t+1ms and another input report with B pressed will be sent at t+2ms. As you can see this is already a problem even before we start considering the counter. With the counter the main loop looks like this:

while True:
    button_state = fetch_button_state()
    update_input_report(button_state)
    report.counter += 1
    if tud_hid_ready() and (report != previous_report):
        tud_hid_report(report)
        previous_report = report

The counter is something that makes the constructed report differ from the previously sent report every time, even if the button state doesn't change. This means that even when no buttons are pressed or released, at t+0.001ms a new report is constructed and tud_hid_report() is called. This means that if a button is now pressed at t+0.5ms, an input report with that button press will be sent at t+2ms, hence the observed latency increase (compared with no counter). This has nothing to do with actual contents of the counter, just the fact that it changes every time. If you absolutely insist on having the counter increment, I'm sure there are ways of avoiding the issues described above, but if you look at the code you'll realize that the counter doesn't even increment by 1 every frame. It increments by however many iterations the main loop did in a frame. So if your intent was to have a counter that increments by 1 every frame, we're already not doing that and the PlayStation doesn't care. Furthermore, in my tests, the PlayStation doesn't care even if the counter is always set to zero. A Razer Raion in PS4 mode doesn't have an incrementing counter.

[arntsonl]

Dogtopus mentioned this could be related to the IMU data. We'll have to dive into this more.

[vgf89]

I think I've got an idea on how to tackle this. If we store the time we sent a report, we can loop the gamepad processing steps until we reach t > last_report + some_μS, essentially letting changes accumulate for around 1ms, then check for changes and send the next report. This could be easily implemented at the top of send_report(...), though if we want to be sending an accurate report counter, this new timer and the memcmp that's currently in send_report would need to be implemented inside GP2040::run() so that we can defer get_report() until they're actually needed.

EDIT: This shouldn't affect IMU data or anything like that since those readings are just instantaneous measurements anyways, not cumulative between reports. So long as we're sending accurate timestamps it's all good (at the very least, I recall a Steam Input changelog mentioning using timestamps for gyro on PS4 controllers if it sees them). Also iirc I didn't even implement the report counter on my imu branch and the PS4 and pc drivers didn't seem to care at all, motion worked fine.

[jfedor2]

You can synchronize to the start-of-frame packet. The host sends it every 1ms just before it polls for inputs reports. usbd_class_driver_t has a .sof callback.

And I wouldn't worry about about the counter being "accurate" (if by that you mean it increases by 1 every frame). It never has been in GP2040 and still isn't. The motion data uses a separate field for its timestamps.

[vgf89]

https://github.com/OpenStickCommunity/GP2040-CE/pull/599