openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

ui: synchronize model frame with camera frame to ensure consistent display

Open deanlee opened this issue 1 year ago • 3 comments

Issue

Previously, CameraView was repainted upon receiving a camera frame, while the model data (modelV2) was updated separately using a timer. This often resulted in mismatches between the camera and model frames. If we log the frame_id in CameraView::paintGL(), we can observe that the model's frame ID consistently lags behind the camera frame ID, and in some cases, model outputs are even skipped.:

camera frame id: 15031, model frame id: 15029 camera frame id: 15032, model frame id: 15031 (model skipped 2 frames, 15029->15031) camera frame id: 15033, model frame id: 15032

Resolution

This PR resolves this issue by triggering the camera frame repaint when modelV2 is updated, ensuring visual consistency between the model and the camera frames.

After this PR, the model frame and camera frame match perfectly:

camera frame id: 8801, model frame id: 8801 camera frame id: 8802, model frame id: 8802 camera frame id: 8803, model frame id: 8803

Key Changes

  1. Initialize SubMaster to poll exclusively for ModelV2 messages.
  2. Introduce a 10 ms delay in update_sockets() to ensure modelV2 is updated when onroad.
  3. Call update() in AnnotatedCameraWidget::updateState() to trigger painting once update_sockets() completes.

This PR is part of #33773. After this implementation, we can significantly simplify the CameraView class by refactoring it to a single-threaded design.

deanlee avatar Oct 12 '24 15:10 deanlee

UI Preview

All Screenshots

github-actions[bot] avatar Oct 12 '24 15:10 github-actions[bot]

I noticed if you pause replay, then the path disappears and the speed gets reset to 0.

sshane avatar Oct 14 '24 20:10 sshane

I noticed if you pause replay, then the path disappears and the speed gets reset to 0.

@sshane : This happens because we now check if messages are "alive" before drawing the path and speed. Pausing the replay makes the messages inactive, causing the path to disappear and speed to reset to 0.

https://github.com/commaai/openpilot/blob/9674c7b5af0b38837f0902e58e246d1c777df5f9/selfdrive/ui/qt/onroad/hud.cc#L16-L20 https://github.com/commaai/openpilot/blob/9674c7b5af0b38837f0902e58e246d1c777df5f9/selfdrive/ui/qt/onroad/model.cc#L19-L21

In the master branch, this behavior didn’t occur because the HUD and model were only repainted when a camera frame was received. During a pause (with no incoming camera frames), the path and HUD remained visible—although they would still disappear if you clicked the UI to show the sidebar, which triggered a repaint.

With the new approach, update_sockets triggers a repaint every time it's called, which causes the path to disappear during pauses.

Should we consider reverting to checking recv_frame > ui.scene.start_frame to maintain the display while paused? Alternatively, we could modify the SubMaster to always set alive to True when REPLAY environments are active. or on a PC environment.

~I've submitted a PR to add the REPLAY environment variable: https://github.com/commaai/openpilot/pull/33793. What do you think?~ after further consideration, I believe it's better to revert to the original frame comparison with scene.started_frame for now (https://github.com/commaai/openpilot/pull/33794). What are your thoughts?

deanlee avatar Oct 15 '24 02:10 deanlee

This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.

github-actions[bot] avatar Oct 28 '24 02:10 github-actions[bot]

This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.

github-actions[bot] avatar Nov 08 '24 01:11 github-actions[bot]

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

github-actions[bot] avatar Nov 10 '24 02:11 github-actions[bot]