Repository navigation
fix: move the scanner and camera work off the UI thread - #46
Conversation
Reviewer's GuideThe PR removes lengthy scanner, camera, image-processing, file-save, and PDF-rendering operations from the UI thread by routing device work through worker signals and using QtConcurrent for independent processing, while adding bounded waits, faster image paths, safer camera defaults, and timing diagnostics. Sequence diagram for asynchronous scanner enumeration and capturesequenceDiagram
participant UI as MainWindow or ScanWidget
participant Device as ScannerDevice
participant Worker as ScannerWorker thread
participant SANE as SANE backend
UI->>Device: requestAvailableDevices()
Device-->>Worker: triggerGetAvailableDevices()
Worker->>SANE: sane_get_devices()
SANE-->>Worker: device list
Worker-->>Device: availableDevicesReady(deviceNames)
Device-->>UI: availableDevicesReady(deviceNames)
UI->>Device: startScan()
Device-->>Worker: triggerStartScan()
Worker->>SANE: sane_start()
Worker->>SANE: sane_read()
alt QImage-compatible frame
Worker-->>Device: captureCompletedImage(image)
else other frame format
Worker-->>Device: captureCompleted(filePath)
end
Device-->>UI: imageCaptured or scan completion
Sequence diagram for background scan processing and file savingsequenceDiagram
participant UI as ScanWidget UI thread
participant Worker as QtConcurrent worker
participant File as Output file
UI->>Worker: saveScan(request)
Worker->>Worker: convertToBlackWhite(image)
Worker->>Worker: detectPaperSize(image, dpi)
Worker->>Worker: scaleToPaperSize(image, paperSize, dpi)
alt PDF output
Worker->>Worker: renderPdf(image, filePath, paperSize)
Worker->>File: write PDF
else image or OFD output
Worker->>File: save output
end
Worker-->>UI: ScanSaveResult
UI->>UI: update scan history
State diagram for bounded camera capture and reconfigurationstateDiagram-v2
[*] --> Initialized
Initialized --> Streaming: startStream()
Streaming --> Capturing: startCapturing()
Capturing --> Streaming: poll and VIDIOC_DQBUF succeeds
Capturing --> Streaming: FRAME_WAIT_TIMEOUT_MS expires
Streaming --> Reconfiguring: setResolution()
Reconfiguring --> Streaming: startStream()
Streaming --> Initialized: stopCapturing()
Reconfiguring --> Initialized: closeDevice() fails
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/ui/scanwidget.cpp" line_range="490" />
<code_context>
+ webcam->startPreview();
+ }
+ });
+ watcher->setFuture(QtConcurrent::run(&WebcamDevice::setResolution, webcam, width, height));
}
}
</code_context>
<issue_to_address>
**Device destroyed during reconfiguration**
When the application closes before the background resolution change completes, `QtConcurrent::run()` captures a raw `WebcamDevice*`, and `MainWindow::~MainWindow()` clears the last shared owner without waiting for the task. `setResolution()` then uses a destroyed device, causing use-after-free or concurrent teardown.
Keep the device alive until the task completes, or make application shutdown wait for the reconfiguration task.
</issue_to_address>
### Comment 2
<location path="src/ui/scanwidget.cpp" line_range="573" />
<code_context>
+ ScanSaveRequest request;
+ request.image = image;
+ request.filePath = QDir(getSaveDirectory())
+ .filePath(QString("%1.%2").arg(QDateTime::currentDateTime().toString("yyyyMMdd_hhmmss"))
+ .arg(FORMATS[m_imageSettings->format].toLower()));
+ request.formatIndex = m_imageSettings->format;
</code_context>
<issue_to_address>
**Concurrent saves overwrite scan files**
When two scans using the same output format and directory finish within the same second, `onScanFinished()` builds each `filePath` with only second-level timestamp precision, then starts `saveScan()` asynchronously without reserving the path. Both jobs write the same file, so one scan overwrites or corrupts the other and history can contain duplicate paths.
Assign each scan a unique output path before starting its asynchronous save.
Also at `src/ui/scanwidget.cpp:574-599`.
</issue_to_address>
### Comment 3
<location path="src/device/scannerdevice.cpp" line_range="882-883" />
<code_context>
break;
}
+ if (!writePng && linesRead >= parm.lines) {
+ break; // the image is complete
+ }
+
</code_context>
<issue_to_address>
**Completed scans are reported as failures**
When SANE supplies a positive line count and a frame format handled directly as a `QImage`, `scan_it` breaks while `status` is still `SANE_STATUS_GOOD`; its later EOF check therefore cancels the scan and returns an error, so `ScannerWorker::doStartScan` reports failure instead of emitting the completed image.
Treat reaching `parm.lines` as successful completion, or continue reading until SANE returns EOF before applying the EOF check.
</issue_to_address>
### Comment 4
<location path="src/device/webcamdevice.cpp" line_range="637" />
<code_context>
+ // Wait for a frame with a timeout instead of blocking in VIDIOC_DQBUF: the file
+ // descriptor stays in non-blocking mode, so a stalled stream cannot hang here.
+ struct pollfd pfd = { m_fd, POLLIN, 0 };
+ const int ready = poll(&pfd, 1, FRAME_WAIT_TIMEOUT_MS);
+ const bool gotFrame = (ready > 0) && (ioctl(m_fd, VIDIOC_DQBUF, &buf) != -1);
</code_context>
<issue_to_address>
**Camera capture freezes the UI**
When a camera frame is delayed or unavailable during capture, `ScanWidget::startScanning()` calls `startCapture()` synchronously on the UI thread, which reaches `captureImage()` and waits in `poll()` for up to two seconds per attempt. Three unsuccessful attempts can block the window for up to six seconds.
Run capture off the UI thread or otherwise ensure frame waits do not block the UI thread.
Also at `src/device/webcamdevice.cpp:638`.
</issue_to_address>
### Comment 5
<location path="src/device/webcamdevice.cpp" line_range="729-755" />
<code_context>
- size_t expectedSize = width * height * 2; // YUYV格式每个像素需要2字节
- if (expectedSize > 0) { // 合理的大小上限
- try {
- for (int y = 0; y < height; y++) {
- for (int x = 0; x < width; x += 2) {
- int i = y * width * 2 + x * 2;
-
- // Boundary check
- if (i + 3 >= expectedSize) {
- qCWarning(app) << "Frame data boundary check failed - possible incomplete data";
- continue;
- }
-
- int y0 = yuyv[i];
</code_context>
<issue_to_address>
**High-resolution preview still blocks UI**
When the camera preview is active at a resolution whose frame conversion takes appreciable time, `updatePreview()` invokes `frameToQImage()` from the UI-thread timer, so this per-pixel conversion still runs on the UI thread for every preview frame. At high resolutions, conversion stalls the window even though opening and reconfiguring the camera moved to workers.
Move preview frame dequeue and conversion off the UI thread, then deliver the converted frame to the widget asynchronously.
</issue_to_address>Opening and enumerating scanners, opening and capturing from a camera, processing and saving a scan and rendering the PDF all ran on the UI thread and froze the window for up to several seconds (about seven seconds for a SANE enumeration, one second for an A4 page at 300 dpi). They now run on worker threads, the fixed sleeps became waits with a timeout, the frame formats that map onto a QImage no longer go through a PNG temp file, and elapsed-time logs were added to tell the slow phase apart. The camera frame work followed: dequeuing a frame, converting it and waiting for a frame during a capture still ran on the UI thread, which froze the window for about a second on a stream that had to be started first and spent most of every preview frame on the per-pixel conversion. Both now run on a single per-device I/O thread that keeps the preview and the capture from touching the device at the same time, the frame wait is sliced so that a device that is being closed or reconfigured ends it within about 100 ms, the preview timer is only touched from the UI thread, the device-level operations are serialized, two scans that end within the same second no longer build the same file name, and the application waits for a camera reconfiguration before it releases the device. 扫描仪打开/枚举/扫描、相机打开/开流/采集/切换分辨率、扫描结果处理与保存、PDF 渲染原本都在 UI 线程执行,界面会冻结数秒(SANE 枚举约 7 秒,A4 300dpi PDF 渲染约 1 秒)。现全部移入工作线程, 固定睡眠改为带超时等待,可直接映射为 QImage 的帧格式不再经过 PNG 临时文件中转,并补充各阶段 耗时日志。 相机的取帧与转换随后也移入工作线程:预览取帧、逐像素转换和采集时等待首帧原本仍在 UI 线程执行, 需要现开流的采集会冻结界面约 1 秒,预览每帧的转换也几乎占满 UI 线程。现两者都在设备专属的单个 I/O 线程上串行执行;等待帧改成分片等待(设备关闭或重配置时约 100ms 内让出),预览定时器只在 UI 线程操作,设备级操作(打开/关闭/重配置)串行化,同一秒内结束的两次扫描不再生成同名文件,退出 应用时会等待进行中的相机重配置结束后再释放设备。 Log: 扫描与相机操作移出 UI 线程,扫描、保存、预览期间界面不再冻结 PMS: BUG-346173 Influence: 打开设备、开始扫描、保存结果、切换相机分辨率与预览期间界面保持响应;同一秒内连续两次扫描会生成不同文件名的结果;退出应用时若正在切换相机分辨率会等待该操作完成(最多数秒);默认相机分辨率由设备最大值改为 1080p(界面可改回);输出文件内容不变。
d5e00c3 to
2c84ae0
Compare
deepin pr auto review🤖 AI 代码审查报告📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 建议在 scan_it() 的 memcpy 后,当 len < image.bytesPerLine() 时对剩余字节清零,避免 QImage 中出现未初始化数据 2. 代码质量 ✅评价: 优秀 ✅ 通过 潜在问题:
建议: 建议将 captureImage() 中的重试循环逻辑提取为独立方法,如 tryCaptureFrame(),提升可读性和可维护性 3. 代码性能 ✅评价: 优秀 ✅ 通过 潜在问题: 建议: 无需额外优化建议,性能改进非常到位 4. 代码安全 🔒评价: 优秀 ✅ 通过
安全漏洞详情: 建议: 无需安全加固,代码不涉及用户输入处理、网络操作或敏感信息 💡 改进建议代码示例// scan_it() 中 memcpy 后对剩余字节清零
if (!writePng) {
const int copyLen = qMin(len, image.bytesPerLine());
memcpy(image.scanLine(linesRead), buffer, copyLen);
if (copyLen < image.bytesPerLine()) {
memset(image.scanLine(linesRead) + copyLen, 0,
image.bytesPerLine() - copyLen);
}
}本报告由 AI 代码审查工具自动生成 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, re2zero The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/merge |
fix: move the scanner and camera work off the UI thread
Opening and enumerating scanners, opening and capturing from a camera,
processing and saving a scan and rendering the PDF all ran on the UI thread and
froze the window for up to several seconds (about seven seconds for a SANE
enumeration, one second for an A4 page at 300 dpi). They now run on worker
threads, the fixed sleeps became waits with a timeout, the frame formats that
map onto a QImage no longer go through a PNG temp file, and elapsed-time logs
were added to tell the slow phase apart.
扫描仪打开/枚举/扫描、相机打开/开流/采集/切换分辨率、扫描结果处理与保存、PDF 渲染原本都在 UI
线程执行,界面会冻结数秒(SANE 枚举约 7 秒,A4 300dpi PDF 渲染约 1 秒)。现全部移入工作线程,
固定睡眠改为带超时等待,可直接映射为 QImage 的帧格式不再经过 PNG 临时文件中转,并补充各阶段
耗时日志。
Log: 扫描与相机操作移出 UI 线程,扫描、保存、加载期间界面不再冻结
PMS: BUG-346173
Influence: 打开设备、开始扫描、保存结果、切换相机分辨率期间界面保持响应;默认相机分辨率由设备最大值改为 1080p(界面可改回);输出文件内容不变。
Summary by Sourcery
Move scanner and camera operations, scan processing, and PDF rendering off the UI thread to keep the application responsive during device and file operations.
New Features:
Bug Fixes:
Enhancements: