feat: add play control button for local video playing - #67
Conversation
Walkthrough本次改动重构了本地视频捕获器的播放控制机制：将基于终端按键轮询的实现替换为基于 jthread 的后台服务线程，支持播放/暂停/步进；同时在 Web 播放页面新增对应的控制按钮与状态同步逻辑，并在 streamer 脚本中扩展 FIFO 路由与上下文读取以支持这些新接口。 Changes本地视频播放控制
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant WebUI as playing.html
participant Streamer as start-streamer
participant FIFO
participant Capturer as LocalVideo::Impl
User->>WebUI: 点击播放/暂停按钮
WebUI->>Streamer: POST /api/play_pause
Streamer->>FIFO: 写入 fifoPlayPause
FIFO->>Capturer: 后台服务读取动作
Capturer->>Capturer: 切换 playing 状态
Capturer-->>WebUI: 更新 latest_image（供轮询读取）
WebUI->>Streamer: GET /api/context
Streamer-->>WebUI: 返回 source/playing 状态
WebUI->>WebUI: updatePlaybackButtons() 刷新按钮文案
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/module/capturer/local_video.cpp (2)
138-143: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win重入配置前先停掉旧 service 线程。
Line 138 每次
configure()都会启动后台线程，Line 143 又允许connect()重新进入configure(config)。如果已有 service 正在运行，旧回调可能在新capturer构建和状态重置期间继续访问共享状态。建议在 location 校验通过后、capturer.emplace(...)前调用stop_service()。建议修复
if (_config.location.empty() || !std::filesystem::exists(_config.location)) { return std::unexpected { "Local video is not found or location is empty" }; } + stop_service(); + config = _config;🤖 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/module/capturer/local_video.cpp` around lines 138 - 143, `configure()` can be re-entered through `connect()`, which may start a new background service while an old one is still running and touching shared state. Update `configure` in `local_video.cpp` so that, after the location check succeeds and before `capturer.emplace(...)`, it first calls `stop_service()` to shut down any existing service thread. Use the `configure`, `connect`, `start_service`, and `stop_service` symbols to place the fix correctly.
165-173: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
allow_skipping=true时需要实际跳过落后的帧。当前落后时只是把
last_read_time调到Clock::now()，但capturer仍然只读取下一帧；处理耗时高于帧间隔时，本地视频会降速播放而不是追赶。建议按-wait_duration / interval_duration计算应跳过的帧数，并在读取前 seek 到对应帧；allow_skipping=false则继续严格逐帧读取。Based on learnings: 在
src/module/capturer中，allow_skipping=false应严格顺序播放所有帧，allow_skipping=true应保持预期播放速率并在落后时追赶，且两种模式都应有测试覆盖。Also applies to: 186-190
🤖 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/module/capturer/local_video.cpp` around lines 165 - 173, In local_video.cpp, the allow_skipping path in the capturer still only advances last_read_time and does not actually skip any frames, so a slow capture will drift instead of catching up. Update the read logic around the time_before_read/next_read_time_expected wait_duration check to compute how many frames were missed, seek the source to the correct frame before reading, and keep strict sequential reads when config.allow_skipping is false. Add or adjust tests for the local video capturer behavior in both modes.Source: Learnings
🤖 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/module/capturer/local_video.cpp`:
- Around line 40-51: `step_frame_locked()` and `wait_image()` are touching the
same `cv::VideoCapture` without serialization, so their `read`/`set` operations
can interleave and corrupt frame position. Add a shared mutex around all
`capturer` access in these paths, or introduce a dedicated `capturer_mutex`, and
make sure both the seeking logic in `step_frame_locked()` and the capture loop
in `wait_image()` use the same lock before calling `read()` or `set()`.
In `@tool/res/playing.html`:
- Around line 448-454: `sendPlaybackCommand()` only catches network errors and
still treats `4xx/5xx` responses as success; update this helper to inspect the
`fetch` response and throw or log when `response.ok` is false so FIFO write
failures are not silently swallowed. Keep the existing `console.error` path for
failures, and if needed in the same success/failure flow, trigger
`fetchRecordContext()` after the playback command to reduce UI lag while the 1s
polling continues to refresh state.
---
Outside diff comments:
In `@src/module/capturer/local_video.cpp`:
- Around line 138-143: `configure()` can be re-entered through `connect()`,
which may start a new background service while an old one is still running and
touching shared state. Update `configure` in `local_video.cpp` so that, after
the location check succeeds and before `capturer.emplace(...)`, it first calls
`stop_service()` to shut down any existing service thread. Use the `configure`,
`connect`, `start_service`, and `stop_service` symbols to place the fix
correctly.
- Around line 165-173: In local_video.cpp, the allow_skipping path in the
capturer still only advances last_read_time and does not actually skip any
frames, so a slow capture will drift instead of catching up. Update the read
logic around the time_before_read/next_read_time_expected wait_duration check to
compute how many frames were missed, seek the source to the correct frame before
reading, and keep strict sequential reads when config.allow_skipping is false.
Add or adjust tests for the local video capturer behavior in both modes.
🪄 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: 7ff34f1b-e3d4-424a-8e9c-6b0bed053f91
📒 Files selected for processing (3)
src/module/capturer/local_video.cpptool/res/playing.htmltool/res/start-streamer
| auto step_frame_locked(int offset) -> void { | ||
| if (!capturer.has_value()) return; | ||
|
|
||
| auto frame = cv::Mat { }; | ||
| if (offset < 0) { | ||
| const auto pos = static_cast<int>(capturer->get(cv::CAP_PROP_POS_FRAMES)); | ||
| if (pos < 2) return; | ||
| capturer->set(cv::CAP_PROP_POS_FRAMES, pos - 2); | ||
| if (!capturer->read(frame) || frame.empty()) return; | ||
| } else { | ||
| if (!capturer->read(frame) || frame.empty()) return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 核对 local_video.cpp 中所有 capturer 访问点是否都处在同一互斥保护策略下。
rg -nP 'capturer(->|\.)?(read|set|get|reset|emplace|has_value|isOpened)?' src/module/capturer/local_video.cpp -C 3Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 2767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 读取 local_video.cpp 相关区段，确认 capturer 与 mutex 的实际保护范围
sed -n '1,240p' src/module/capturer/local_video.cpp | cat -nRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 9712
用同一把锁串行化 capturer 访问。
step_frame_locked() 和 wait_image() 都在读写同一个 cv::VideoCapture，但现在只保护了 latest_image；wait_image() 里的 read/set 仍可能和步进回调交错，导致帧位置和返回图像不稳定。把 capturer 的定位/读取也纳入同一临界区，或单独引入 capturer_mutex。
🧰 Tools
🪛 Cppcheck (2.21.0)
[error] 41-41: Found an exit path from function with non-void return type that has missing return statement
(missingReturn)
[error] 47-47: Found an exit path from function with non-void return type that has missing return statement
(missingReturn)
🤖 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/module/capturer/local_video.cpp` around lines 40 - 51,
`step_frame_locked()` and `wait_image()` are touching the same
`cv::VideoCapture` without serialization, so their `read`/`set` operations can
interleave and corrupt frame position. Add a shared mutex around all `capturer`
access in these paths, or introduce a dedicated `capturer_mutex`, and make sure
both the seeking logic in `step_frame_locked()` and the capture loop in
`wait_image()` use the same lock before calling `read()` or `set()`.
| async function sendPlaybackCommand(url) { | ||
| try { | ||
| await fetch(url, { method: 'POST', body: '1' }); | ||
| } catch (error) { | ||
| console.error('Playback command failed:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '400,520p' tool/res/playing.htmlRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 4862
🏁 Script executed:
rg -n "fetchRecordContext|setInterval|sendPlaybackCommand|updatePlaybackButtons|isPlaying|isLocalVideo" tool/res/playing.htmlRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1358
🏁 Script executed:
sed -n '740,785p' tool/res/playing.htmlRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1286
补上 fetch 的 HTTP 状态检查 tool/res/playing.html:448-454
sendPlaybackCommand() 现在只会捕获网络异常，4xx/5xx 仍会被当作成功处理，FIFO 写入失败会被静默吞掉。这里应在 response.ok 为 false 时显式报错；状态刷新本身已由现有的 1s 轮询覆盖，必要时再额外触发一次 fetchRecordContext() 以减少点击后的 UI 延迟。
🤖 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 `@tool/res/playing.html` around lines 448 - 454, `sendPlaybackCommand()` only
catches network errors and still treats `4xx/5xx` responses as success; update
this helper to inspect the `fetch` response and throw or log when `response.ok`
is false so FIFO write failures are not silently swallowed. Keep the existing
`console.error` path for failures, and if needed in the same success/failure
flow, trigger `fetchRecordContext()` after the playback command to reduce UI lag
while the 1s polling continues to refresh state.
1 participant
本次变更为本地视频播放补充了完整的控制链路，并同步更新前后端交互。
主要内容：
src/module/capturer/local_video.cpp重构本地视频读取与控制逻辑，移除终端按键轮询方式，改为通过后台服务线程管理播放/暂停与逐帧控制；新增帧步进读取、latest_image同步缓存及相关并发保护。tool/res/start-streamer扩展服务端接口，新增/api/play_pause、/api/step_forward、/api/step_backward，将播放控制请求写入对应 FIFO；同时调整 context 读取逻辑，合并本地视频与录制状态信息。tool/res/playing.html增加本地视频控制按钮与交互状态展示，支持上一帧、播放/暂停、下一帧操作，并根据local_video状态动态启用/禁用按钮。