feat: record video while autoaim is running#58
Conversation
Walkthrough此PR添加了视频录制功能,包括FIFO命名管道的IPC服务框架、录制状态管理和状态查询接口、配置参数支持,以及Capturer中的原子请求驱动录制控制流程,允许外部通过命名管道动态开启/停止视频录制。 ChangesVideo Recording Functionality
Sequence Diagram(s)sequenceDiagram
participant External as 外部控制端
participant ServiceThread as Service线程<br/>(cap:record)
participant MainLoop as Capturer主循环
participant Recorder as VideoRecorder
External->>ServiceThread: 向FIFO写入请求字符串
ServiceThread->>ServiceThread: 解析字符串,设置record_request
MainLoop->>MainLoop: 读取原子record_request(±1)
alt record_request == +1
MainLoop->>Recorder: recorder.start()
else record_request == -1
MainLoop->>Recorder: recorder.stop()
end
MainLoop->>Recorder: recorder.tick(frame)推送新帧
Recorder->>Recorder: 检查是否达到max_duration
alt 达到时长上限
Recorder->>Recorder: 设置stop_reason记录信息
Recorder->>Recorder: 自动停止录制
end
MainLoop->>MainLoop: 重置record_request为0
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Infer (1.2.0)src/kernel/capturer.cppIn file included from src/kernel/capturer.cpp:1: ... [truncated 1138 characters] ... ang/install/lib/clang/18/include" src/utility/image/recorder.cppIn file included from src/utility/image/recorder.cpp:1: ... [truncated 1141 characters] ... tall/lib/clang/18/include" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/kernel/capturer.cpp (1)
70-77: ⚡ Quick win考虑简化字符串解析逻辑。
当前的字符串处理使用了多个 ranges 视图组合(
drop_while、reverse、transform),虽然功能正确,但可读性较低。可以考虑使用更直接的方法。♻️ 简化建议
- const auto lower = std::ranges::to<std::string>(data - | std::views::drop_while([](char c) { return c == '\n' || c == '\r'; }) - | std::views::reverse - | std::views::drop_while([](char c) { return c == '\n' || c == '\r'; }) - | std::views::reverse | std::views::transform(::tolower)); + auto trimmed = data; + // 去除前后空白字符 + const auto not_space = [](char c) { return c != '\n' && c != '\r' && c != ' '; }; + auto start = std::ranges::find_if(trimmed, not_space); + auto end = std::ranges::find_if(trimmed | std::views::reverse, not_space).base(); + auto lower = std::string(start, end); + std::ranges::transform(lower, lower.begin(), ::tolower); const auto equal = [&](std::string_view target) { return std::ranges::equal(lower, target); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/kernel/capturer.cpp` around lines 70 - 77, The current construction of lower using chained std::views (in capturer.cpp) is hard to read; replace it with a straightforward approach: copy data into a std::string (e.g., named lower), trim leading/trailing '\n' and '\r' via find_first_not_of/find_last_not_of or simple while loops, then apply std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower); keep the equal lambda that compares this normalized lower to the target. Update the code around the const auto lower and const auto equal declarations to use this simpler sequence.src/utility/service.hpp (1)
34-34: 💤 Low value考虑固定缓冲区大小是否足够。
256 字节的固定缓冲区可能无法容纳较长的命令或数据。如果消息超过 256 字节,将被截断,可能导致命令解析失败。
建议根据实际使用场景评估是否需要更大的缓冲区(如 1024 或 4096 字节),或在读取时处理部分读取的情况。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utility/service.hpp` at line 34, 当前在 service.hpp 中使用固定数组声明 char buf[256] 可能导致超长消息被截断;请将该固定缓冲区替换为可变长度的缓冲策略(例如使用 std::string 或 std::vector<char> 并按需 resize 到 1024/4096 或根据实际消息长度分配),并在所有使用 buf 的函数处(定位到对 char buf[256] 的读/写调用)改为循环读取/追加直到接收完整消息或遇到分隔符/长度限制以正确处理部分读取情况;如果保留固定缓冲区,至少将大小调整为更安全的值(如 1024 或 4096)并在读取后检查是否发生截断,记录/返回错误。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/kernel/capturer.cpp`:
- Around line 6-9: The file uses ::tolower in capturer.cpp but does not include
the <cctype> header; add an explicit `#include` <cctype> alongside the existing
includes (e.g., near "utility/image/image.details.hpp",
"utility/image/recorder.hpp") so ::tolower is defined and portable across
standard library implementations. Ensure the include is added at the top of
src/kernel/capturer.cpp so the function call to ::tolower compiles correctly.
In `@src/utility/service.hpp`:
- Around line 52-53: The call to ::open(...) can return -1 but the result
assigned to fd is not checked; update the code around the open call to check if
fd == -1 and handle the error (e.g., log the errno string, set an error
state/flag on the object, or return/throw an error) so callers (and spin_once())
see the failure; specifically modify the open() branch that sets fd (and any
constructor or init function that calls it) to detect -1, record or propagate
the error (using strerror(errno) in logs) and ensure spin_once() observes the
failed-open state rather than silently proceeding.
- Around line 60-63: The code calls ::read(...) into buf and only checks n > 0
before calling callback, which fails to handle n == -1 (error) and n == 0 (EOF);
update the read-handling logic around the local variable n so that when ::read
returns -1 you do not construct a std::string_view from an invalid size —
instead handle the error (e.g., log/return/throw) and only call callback when n
> 0, and treat n == 0 as EOF (no callback). Locate the read call and the
callback invocation (the variables fd, buf, n and the callback invocation) and
implement the checks to avoid creating std::string_view with a negative length.
- Around line 78-80: The constructor for Service currently unconditionally calls
fs::remove_all(service_location) (and OnExit::~OnExit() repeats removal), which
creates a race with other instances; update Service and OnExit to stop
unconditional removal: instead implement one of (a) single-instance/lockfile
semantics keyed by kNameView (create and hold a lock file or flock in Service
ctor, fail if taken), or (b) make service_location include the process PID or a
generated instance id so instances do not share /tmp/autoaim/{kNameView}/, or
(c) only create the directory and FIFOs (use mkfifo with EEXIST) and track which
files this instance actually created and only remove those in OnExit; modify the
Service constructor, the variable service_location, and OnExit::~OnExit()
accordingly and add lock/file-tracking helpers to ensure safe cleanup without
interfering with other Named<"cap"> instances.
---
Nitpick comments:
In `@src/kernel/capturer.cpp`:
- Around line 70-77: The current construction of lower using chained std::views
(in capturer.cpp) is hard to read; replace it with a straightforward approach:
copy data into a std::string (e.g., named lower), trim leading/trailing '\n' and
'\r' via find_first_not_of/find_last_not_of or simple while loops, then apply
std::transform(lower.begin(), lower.end(), lower.begin(), ::tolower); keep the
equal lambda that compares this normalized lower to the target. Update the code
around the const auto lower and const auto equal declarations to use this
simpler sequence.
In `@src/utility/service.hpp`:
- Line 34: 当前在 service.hpp 中使用固定数组声明 char buf[256]
可能导致超长消息被截断;请将该固定缓冲区替换为可变长度的缓冲策略(例如使用 std::string 或 std::vector<char> 并按需 resize
到 1024/4096 或根据实际消息长度分配),并在所有使用 buf 的函数处(定位到对 char buf[256]
的读/写调用)改为循环读取/追加直到接收完整消息或遇到分隔符/长度限制以正确处理部分读取情况;如果保留固定缓冲区,至少将大小调整为更安全的值(如 1024 或
4096)并在读取后检查是否发生截断,记录/返回错误。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2cabf2a1-c066-48f0-8bc2-b3e349a1c675
📒 Files selected for processing (6)
config/config.yamlsrc/kernel/capturer.cppsrc/utility/image/recorder.cppsrc/utility/image/recorder.hppsrc/utility/service.hppsrc/utility/string.hpp
| #include "utility/image/image.details.hpp" | ||
| #include "utility/image/recorder.hpp" | ||
| #include "utility/logging/printer.hpp" | ||
| #include "utility/service.hpp" |
There was a problem hiding this comment.
缺少 <cctype> 头文件。
第 74 行使用了 ::tolower,但文件未包含 <cctype> 头文件。虽然某些标准库实现可能通过其他头文件间接包含它,但应该显式包含以确保可移植性。
🔧 建议的修复
`#include` "utility/logging/printer.hpp"
`#include` "utility/service.hpp"
+#include <cctype>
`#include` "utility/singleton/running.hpp"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/kernel/capturer.cpp` around lines 6 - 9, The file uses ::tolower in
capturer.cpp but does not include the <cctype> header; add an explicit `#include`
<cctype> alongside the existing includes (e.g., near
"utility/image/image.details.hpp", "utility/image/recorder.hpp") so ::tolower is
defined and portable across standard library implementations. Ensure the include
is added at the top of src/kernel/capturer.cpp so the function call to ::tolower
compiles correctly.
| fd = ::open(path.c_str(), O_RDONLY | O_NONBLOCK); | ||
| } |
There was a problem hiding this comment.
检查 open() 的返回值。
open() 调用可能失败并返回 -1,但代码未检查返回值。如果打开失败,fd 将为 -1,后续的 spin_once() 虽然会提前返回,但不会向调用者报告错误。
🔍 建议的修复
fd = ::open(path.c_str(), O_RDONLY | O_NONBLOCK);
+if (fd < 0) {
+ throw std::runtime_error { std::format("无法打开管道: {}", path) };
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fd = ::open(path.c_str(), O_RDONLY | O_NONBLOCK); | |
| } | |
| fd = ::open(path.c_str(), O_RDONLY | O_NONBLOCK); | |
| if (fd < 0) { | |
| throw std::runtime_error { std::format("无法打开管道: {}", path) }; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utility/service.hpp` around lines 52 - 53, The call to ::open(...) can
return -1 but the result assigned to fd is not checked; update the code around
the open call to check if fd == -1 and handle the error (e.g., log the errno
string, set an error state/flag on the object, or return/throw an error) so
callers (and spin_once()) see the failure; specifically modify the open() branch
that sets fd (and any constructor or init function that calls it) to detect -1,
record or propagate the error (using strerror(errno) in logs) and ensure
spin_once() observes the failed-open state rather than silently proceeding.
| auto n = ::read(fd, buf, sizeof(buf)); | ||
| if (n > 0) { | ||
| callback(std::string_view { buf, static_cast<std::size_t>(n) }); | ||
| } |
There was a problem hiding this comment.
检查 read() 的错误返回值。
read() 返回 -1 表示错误,但代码只检查了 n > 0。当 n == -1 时,会传递一个无效的 std::string_view { buf, static_cast<std::size_t>(-1) } 给回调函数,可能导致未定义行为。
🛡️ 建议的修复
auto n = ::read(fd, buf, sizeof(buf));
-if (n > 0) {
+if (n > 0) {
callback(std::string_view { buf, static_cast<std::size_t>(n) });
+} else if (n < 0 && errno != EAGAIN && errno != EWOULDBLOCK) {
+ // 读取错误(非阻塞情况除外)
+ throw std::runtime_error { std::format("读取管道失败: errno={}", errno) };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| auto n = ::read(fd, buf, sizeof(buf)); | |
| if (n > 0) { | |
| callback(std::string_view { buf, static_cast<std::size_t>(n) }); | |
| } | |
| auto n = ::read(fd, buf, sizeof(buf)); | |
| if (n > 0) { | |
| callback(std::string_view { buf, static_cast<std::size_t>(n) }); | |
| } else if (n < 0 && errno != EAGAIN && errno != EWOULDBLOCK) { | |
| // 读取错误(非阻塞情况除外) | |
| throw std::runtime_error { std::format("读取管道失败: errno={}", errno) }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utility/service.hpp` around lines 60 - 63, The code calls ::read(...)
into buf and only checks n > 0 before calling callback, which fails to handle n
== -1 (error) and n == 0 (EOF); update the read-handling logic around the local
variable n so that when ::read returns -1 you do not construct a
std::string_view from an invalid size — instead handle the error (e.g.,
log/return/throw) and only call callback when n > 0, and treat n == 0 as EOF (no
callback). Locate the read call and the callback invocation (the variables fd,
buf, n and the callback invocation) and implement the checks to avoid creating
std::string_view with a negative length.
| if (fs::exists(service_location)) { | ||
| fs::remove_all(service_location); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查是否存在其他创建类似服务路径的位置
rg -nP --type=cpp '\bService\s*\{' -A3Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 553
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect the exact constructor code around the reported lines
sed -n '1,140p' src/utility/service.hpp | nl -ba | sed -n '50,120p'
# 2) Find where the service directory (/tmp/autoaim/{name}/) is constructed/used
rg -n --hidden --no-ignore-vcs "tmp/autoaim|autoaim|service_location|fifo" src/utility/service.hpp src/kernel/capturer.cpp src -S
# 3) List all Service creations to see whether names are unique per process/instance
rg -n --type=cpp "\bService\s*\{" -S src
# 4) Trace what filesystem/FIFO operations happen for the service path
rg -n --type=cpp "mkfifo|fifo|remove_all|exists\(|service_location" -S src/utility/service.hpp src/kernel/capturer.cpp srcRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect constructor code near the reported lines
sed -n '1,160p' src/utility/service.hpp | cat -n | sed -n '50,110p'
# 2) Search for service path / fifo / deletion operations
rg -n --hidden --no-ignore-vcs "tmp/autoaim|autoaim|service_location|fifo|remove_all" src -S
# 3) List all Service creation sites
rg -n --type=cpp "\bService\s*\{" -S src
# 4) Find the definition of service_location / path composition (if any)
rg -n --type=cpp "service_location|/tmp/autoaim|autoaim/\{|\{name\}|Named<|kNameView" src/utility/service.hpp src -SRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 8042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Service creation sites (broader)
rg -n --type=cpp "\bService\s*<|Service\s*\{" src -S
# Inspect remaining part of service.hpp for on_exit removal and FIFO behavior
sed -n '1,200p' src/utility/service.hpp | cat -n
# Inspect capturer.cpp where Service is instantiated
sed -n '1,130p' src/kernel/capturer.cpp | cat -nRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 10425
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find all instantiations of Service template (to see which Name values share the same directory)
rg -n --type=cpp "auto service = Service\s*\{" src -S
rg -n --type=cpp "Named<\"[^\"]+\">\s*\{" src -S
# 2) Find all FIFO open/write usages (writers vs readers)
rg -n --type=cpp "O_WRONLY|O_RDWR|::open\\(.*O_WRONLY|::open\\(.*O_RDWR|mkfifo\\(" src -S
# 3) Find any other direct usage of /tmp/autoaim
rg -n --hidden --no-ignore-vcs "/tmp/autoaim" src -S
# 4) Specifically find where the "record" action FIFO is used (since Service<"cap", Action<"record"...>> is opened for reading)
rg -n --type=cpp "Named<\"record\">|record\"\\s*\)|record\\b" src -SRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 681
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect recorder code that references /tmp/autoaim
sed -n '1,120p' src/utility/image/recorder.hpp | cat -n
sed -n '1,200p' src/utility/image/recorder.cpp | cat -n
# Search for reading the actions list file and/or opening FIFO paths listed there
rg -n --type=cpp "ifstream|std::ifstream|open\\(.*actions|actions_path|/tmp/autoaim/.*/actions" src -S
rg -n --type=cpp "/tmp/autoaim/" src/utility src/kernel -S
# Search for any other Service instantiation patterns and other Named<"..."> usage
rg -n --type=cpp "Service<" src -S
rg -n --type=cpp "Service\s*\{" src -S
# Search for O_WRONLY opens to FIFOs (writers) and for the specific FIFO name "record"
rg -n --type=cpp "::open\\(.*O_WRONLY|::open\\(.*O_RDWR|O_WRONLY" src -S
rg -n --type=cpp "autoaim.*/record|\"record\"|Named<\"record\"|kNameView" src -SRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 10064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all Named<"..."> occurrences (to infer Service directory names)
rg -n --type=cpp "Named<\"[^\"]+\">" src -S
# Find any other Service instantiation besides capturer.cpp
rg -n --type=cpp "auto service = Service\s*\{" src -S
rg -n --type=cpp "Service\s*\{" src -S
# Find any other usage of /tmp/autoaim/<name>/ and specifically "cap/record" FIFO usage patterns
rg -n --type=cpp "/tmp/autoaim/[^\"']+/[^\"']+|/tmp/autoaim/.*record|autoaim.*record|\"cap\".*record|Named<\"record\">" src -S
# Search for FIFO open flags; Action::open does O_RDONLY, we need if anything does O_WRONLY on the same paths
rg -n --type=cpp "::open\\([^\\)]*O_WRONLY|::open\\([^\\)]*O_RDWR" src -SRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 433
/tmp/autoaim 目录在 Service 构造/析构中无条件 remove_all 存在竞态风险(会干扰同一路径的其它实例/写入方)
在 src/utility/service.hpp 的 Service 构造函数里,当 service_location = std::format("/tmp/autoaim/{}/", kNameView) 对应到 capturer.cpp 中的 Named<"cap"> 时,会对 /tmp/autoaim/cap/ 执行 fs::remove_all(service_location);这会删除该目录下的 FIFO(例如 /tmp/autoaim/cap/record)及 actions 文件。若系统中存在另一正在运行/启动中的实例(或外部写入方按该路径打开 FIFO),可能导致对方读到旧 FIFO 或无法继续使用新数据流。
同时 Service 的 OnExit::~OnExit() 也会再次 fs::remove_all(to_remove),进一步放大同一路径的竞争窗口。
建议:
- 增加锁文件/单实例机制,确保同一
kNameView仅有一个实例运行 - 或将进程 PID/随机实例号纳入服务路径(避免所有实例共享
/tmp/autoaim/{name}/) - 或避免对
service_location做无条件remove_all:仅创建目录/FIFO(保留现有的mkfifo+EEXIST逻辑),并仅清理“本实例明确创建的”条目
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utility/service.hpp` around lines 78 - 80, The constructor for Service
currently unconditionally calls fs::remove_all(service_location) (and
OnExit::~OnExit() repeats removal), which creates a race with other instances;
update Service and OnExit to stop unconditional removal: instead implement one
of (a) single-instance/lockfile semantics keyed by kNameView (create and hold a
lock file or flock in Service ctor, fail if taken), or (b) make service_location
include the process PID or a generated instance id so instances do not share
/tmp/autoaim/{kNameView}/, or (c) only create the directory and FIFOs (use
mkfifo with EEXIST) and track which files this instance actually created and
only remove those in OnExit; modify the Service constructor, the variable
service_location, and OnExit::~OnExit() accordingly and add lock/file-tracking
helpers to ensure safe cleanup without interfering with other Named<"cap">
instances.
视频录制功能集成
概述
本PR为自瞄系统添加了在自瞄运行期间录制视频的功能,通过新增配置项、集成视频录制器、以及建立进程间通信服务框架来实现。
主要变更
配置管理 (config/config.yaml)
新增录制相关配置项:
record_enable: 录制开关(默认关闭)saving_pathes: 多个视频保存路径列表(支持/home/root/autoaim/、/home/ubuntu/autoaim/、/tmp/autoaim/)max_duration_seconds: 单个视频最大录制时长(60秒)record_fps: 录制帧率(24 fps)max_videos_size_gb: 最大视频文件容量限制(50 GB)图像采集器核心改动 (src/kernel/capturer.cpp)
cap:record指令,支持启动/停止录制的原子信号流转recorder.tick()处理录制逻辑record_request的状态(+1/-1)驱动recorder.start()和recorder.stop()操作进程间通信框架 (src/utility/service.hpp)
新增通用的FIFO基础Service框架:
Action<Name, Callback>: 为每个命名动作创建/管理命名管道,支持非阻塞轮询读取Service<Name, Actions...>: 统一管理多个动作,在/tmp/autoaim/{name}/目录下维护FIFO文件列表,提供spin()轮询驱动接口字符串工具 (src/utility/string.hpp)
新增
Named<StaticString>模板结构体,在编译期将静态字符串的视图与数组指针导出为常量,用于配合Service框架进行类型安全的动作名称管理。视频录制器增强 (src/utility/image/recorder.cpp/hpp)
autoaim_{:%Y-%m-%d_%H-%M-%S}.aviSession::duration()改为返回截断到秒级的std::chrono::secondsstop_reason成员用于记录录制停止的原因(超时/手动停止等)status() const -> std::string公开方法,返回当前录制状态:未开始/录制中(含时长)/已停止(含停止原因)技术特点
代码体量