Tools: Add Cyber Recorder GUI Lite - #15816
Open
1012327963 wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/build blockers (task-buffer pop race in the consumer and CMake/BUILD packaging issues) that can cause runtime misbehavior or build failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces Cyber Recorder GUI Lite, a lightweight Qt-based GUI for inspecting and replaying Apollo Cyber .record files (single file or directory shards), including basic playback controls and republishing messages back onto Cyber channels.
Changes:
- Added Qt UI (MainWindow + custom sliders) for file/directory selection, metadata display, and play/pause/stop/seek controls.
- Implemented a playback pipeline (
Player+ producer/consumer/task buffer) to read records and publish messages back to their original channels. - Added build/packaging and docs assets (Bazel BUILD, CMakeLists, cyber launch, cyberfile.xml, EN/CN README).
File summaries
| File | Description |
|---|---|
| modules/tools/cyber_recorder_gui/record_playbar.h | New clickable progress slider widget (RecordPlaybar) |
| modules/tools/cyber_recorder_gui/record_playbar.cpp | RecordPlaybar mouse handling implementation |
| modules/tools/cyber_recorder_gui/README.md | English usage/build documentation for GUI Lite |
| modules/tools/cyber_recorder_gui/README_cn.md | Chinese usage/build documentation for GUI Lite |
| modules/tools/cyber_recorder_gui/player/player.h | Player API for GUI-driven playback |
| modules/tools/cyber_recorder_gui/player/player.cc | Threaded playback control and seek/reset implementation |
| modules/tools/cyber_recorder_gui/player/play_task.h | Playback task abstraction (message + writer + timestamps) |
| modules/tools/cyber_recorder_gui/player/play_task.cc | Playback task execution (writer publish) |
| modules/tools/cyber_recorder_gui/player/play_task_producer.h | Producer that reads record(s) and fills task buffer |
| modules/tools/cyber_recorder_gui/player/play_task_producer.cc | Record reading, writer creation, buffering logic |
| modules/tools/cyber_recorder_gui/player/play_task_consumer.h | Consumer that schedules tasks according to timestamps |
| modules/tools/cyber_recorder_gui/player/play_task_consumer.cc | Playback timing loop and pause handling |
| modules/tools/cyber_recorder_gui/player/play_task_buffer.h | Shared task buffer container |
| modules/tools/cyber_recorder_gui/player/play_task_buffer.cc | Task buffer implementation (multimap + mutex) |
| modules/tools/cyber_recorder_gui/player/play_param.h | Playback parameter struct |
| modules/tools/cyber_recorder_gui/mainwindow.ui | Qt Designer UI layout for the lite tool |
| modules/tools/cyber_recorder_gui/mainwindow.h | Main window controller declarations |
| modules/tools/cyber_recorder_gui/mainwindow.cpp | UI wiring, file picking, play/pause/seek orchestration |
| modules/tools/cyber_recorder_gui/main.cpp | Qt entry point + Cyber init/clear |
| modules/tools/cyber_recorder_gui/launch/cyber_recorder_gui_lite.launch | cyber_launch config for starting the binary |
| modules/tools/cyber_recorder_gui/info.h | Record metadata reader interface |
| modules/tools/cyber_recorder_gui/info.cc | Record metadata extraction/formatting |
| modules/tools/cyber_recorder_gui/doubleslider.h | Dual-handle range slider widget |
| modules/tools/cyber_recorder_gui/doubleslider.cc | Dual-handle slider rendering and interaction |
| modules/tools/cyber_recorder_gui/cyberfile.xml | Package metadata for installation |
| modules/tools/cyber_recorder_gui/CMakeLists.txt | CMake build definition for the GUI lite target |
| modules/tools/cyber_recorder_gui/BUILD | Bazel targets (binary + qt library) and packaging |
Review details
Suppressed comments (3)
modules/tools/cyber_recorder_gui/player/play_task_producer.cc:325
- The preload-mode nullptr check log text is grammatically incorrect and unclear ("should not nullptr"); it should state what is null and why that is invalid.
if (preload_fill_buffer_mode_ && !record_viewer_ptr_) {
AERROR << "Preload should not nullptr";
return;
}
modules/tools/cyber_recorder_gui/player/play_task_producer.cc:329
- Same issue as above: the non-preload check message ("No preload should nullptr") is unclear and grammatically incorrect.
if (!preload_fill_buffer_mode_ && record_viewer_ptr_) {
AERROR << "No preload should nullptr";
return;
}
modules/tools/cyber_recorder_gui/record_playbar.h:44
- The trailing
#endifcomment should match the updated header guard macro name to avoid confusion when navigating headers.
#endif // RECORDPLAYBAR_H
- Files reviewed: 27/27 changed files
- Comments generated: 10
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+74
to
+75
| apollo_package(enable_source=False) | ||
| apollo_package() |
Comment on lines
+11
to
+12
| set(CMAKE_CXX_STANDARD 11) | ||
| set(CMAKE_CXX_STANDARD_REQUIRED ON) |
Comment on lines
+17
to
+25
| set(PROJECT_SOURCES | ||
| main.cpp | ||
| mainwindow.cpp | ||
| mainwindow.h | ||
| mainwindow.ui | ||
| record_playbar.cpp | ||
| record_playbar.h | ||
|
|
||
| ) |
Comment on lines
+75
to
+79
| auto task = task_buffer_->Front(); | ||
| if (task == nullptr) { | ||
| std::this_thread::sleep_for(std::chrono::nanoseconds(kWaitProduceSleepNanoSec)); | ||
| continue; | ||
| } |
Comment on lines
+4
to
+8
| <description> | ||
| This is a demo package | ||
| </description> | ||
|
|
||
| <maintainer email="sample@sample.com">Apollo Developer</maintainer> |
| #include "doubleslider.h" | ||
|
|
||
| QT_BEGIN_NAMESPACE | ||
| using namespace apollo::goodman::record; |
Comment on lines
+66
to
+69
| if (!preload_fill_buffer_mode_ && node_ != nullptr) { | ||
| AERROR << "invalid param: nullptr node"; | ||
| node_ = nullptr; | ||
| } |
Comment on lines
+217
to
+218
| AINFO << "preload time is zero, we will use defalut value: " << kPreloadTimeSec << " seconds."; | ||
| play_param_.preload_time_s = kPreloadTimeSec; |
Comment on lines
+93
to
+96
| if (!is_stopped_.exchange(false)) { | ||
| AERROR << "player has been stopped."; | ||
| return false; | ||
| } |
Comment on lines
+1
to
+2
| #ifndef RECORDPLAYBAR_H | ||
| #define RECORDPLAYBAR_H |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds
cyber_recorder_gui_lite, a lightweight Qt-based graphical tool for replaying Apollo Cyber record files.The tool focuses on basic record inspection and playback. It is provided as a separate package from
modules/cyber_recorder_gui, allowing both versions to coexist without executable or installation conflicts.Main changes
.recordfile..recordand.record.*shards from a directory.