fix: handle null captured image retry#47
Conversation
bc91890 to
445e5e2
Compare
|
尽量不要使用sleep阻塞主线程 |
mhduiy
left a comment
There was a problem hiding this comment.
风险分析
1. capture() 循环链路变更 — 高风险 ⚠️
原代码在 processCapturedImage 末尾无条件调用 m_imageCapture->capture() 来维持采集循环。新代码删除了这个调用,改为:
- 空图片时 → 手动调
capture()重试 - 正常图片时 → 不再调用
capture()
这意味着正常图片处理后,下一次 capture() 的触发完全依赖 readyForCapture(true) 被外部调用。如果 readyForCapture 没有被及时触发,正常采集流程也会卡住。建议确认 readyForCapture 的调用时机和频率是否能保证采集链路不断。
2. msleep(100) 阻塞 — 中风险
processCapturedImage 是 Qt 信号槽回调,运行在接收者线程中。msleep(100) 会阻塞该线程 100ms。如果连续 10 次空图片,总阻塞约 1 秒。
另外 PR 描述写的 msleep(10),实际代码是 msleep(100),存在不一致。如果这个槽运行在主线程,会导致 UI 卡顿,建议确认该对象所在的线程。
3. 竞态条件 — 中风险
if (m_stopCapture) // check
return;
m_imageCapture->capture(); // act在 msleep(100) 之后、capture() 之前存在时间窗口,另一个线程可能设置了 m_stopCapture = true。虽然 sleep 后又检查了一次,但 check-then-act 不是原子的。不过 Qt 信号槽机制下实际触发概率较低。
4. m_bFirst 替代 id == 1 — 低风险
从依赖 QImageCapture 返回的 id 变为依赖布尔标志,行为有变更。如果 QImageCapture 的 id 编号机制有特殊情况,这个改动会改变行为,但更可能是修正了原来不够健壮的判断方式。
5. 超过 10 次重试后的行为 — 低风险
超过阈值后发出 FaceEnrollException 信号并 return,不再继续采集。需确认上层对该信号的处理是否完善(是否有用户提示、是否有重试入口)。
最需要关注的是第 1 点:建议确认正常采集路径中 readyForCapture(true) 的调用链路完整且时机正确,否则这个"修复空图片卡住"的 PR 可能反而导致正常场景下采集中断。
| int m_fileSocket; | ||
| bool m_bFirst; | ||
| bool m_checkDone; | ||
| int m_nullCount; |
Added m_nullCount member variable to track null image captures. Reset m_nullCount on thread start and when capture is stopped. In readyForCapture, added condition to check if capture is not stopped. In processCapturedImage, added logic to handle null previews: retry up to 10 times with sleep, else emit error. Changed first image condition from checking id to using m_bFirst flag. Removed unnecessary capture call after image processing. Influence: 1. Test capture functionality with normal and null image scenarios. 2. Verify that capture does not get stuck when images are null. 3. Test the retry mechanism and error emission after multiple failures. 4. Check that first image handling works correctly with the new flag. 5. Ensure capture stops properly when requested. 6. Test edge cases like rapid stop/start actions. 修复: 防止采集空图片时卡住 添加 m_nullCount 成员变量以跟踪空图片采集次数。 在线程启动和采集停止时重置 m_nullCount。 在 readyForCapture 中添加条件检查采集是否未停止。 在 processCapturedImage 中添加处理空预览的逻辑:最多重试10次并带延迟,否 则发出错误信号。 将首张图片条件从检查 id 改为使用 m_bFirst 标志。 移除处理图片后不必要的采集调用。 Influence: 1. 测试正常和空图片场景下的采集功能。 2. 验证当图片为空时采集不会卡住。 3. 测试重试机制和多次失败后的错误发出。 4. 检查使用新标志的首张图片处理是否正确。 5. 确保在请求时采集能正确停止。 6. 测试快速停止/启动操作等边界情况。 PMS: BUG-352653 Change-Id: I3a6964c991e6c8aef8385195ae9f54eb84455d41
deepin pr auto review你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。本次修改主要增加了对捕获图像为空( 整体来看,这些改进增强了代码的健壮性,但在语法逻辑、代码质量、性能和安全性方面,还有一些值得注意和改进的地方。以下是详细的审查意见: 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
改进后的代码建议基于以上分析,我为你提供一份优化后的 // workmodule.h
// 建议将重试次数定义为常量,并评估是否真的需要 atomic
private:
static constexpr int MAX_NULL_RETRY_COUNT = 10;
static constexpr int RETRY_DELAY_MS = 100;
// 如果只在同一线程访问,使用 int 即可;否则保留 std::atomic<int>
int m_nullCount;
bool m_needRetry; // 新增:重试标志位,避免在槽函数中直接触发捕获
// workmodule.cpp
void ErollThread::Start(QString actionId, int socket)
{
qDebug() << "ErollThread::Start thread:" << QThread::currentThreadId();
m_stopCapture = false;
m_nullCount = 0;
m_needRetry = false; // 初始化
m_actionId = actionId;
m_fileSocket = socket;
m_bFirst = true;
// ...
}
void ErollThread::readyForCapture(bool ready)
{
// 如果需要重试,且相机就绪,且未停止,则发起捕获
if (m_imageCapture && ready && !m_stopCapture) {
if (m_needRetry || m_bFirst /* 或其他需要捕获的条件 */) {
m_imageCapture->capture();
qDebug() << "ErollThread::readyForCapture";
}
}
}
void ErollThread::processCapturedImage(int id, const QImage &preview)
{
if (m_stopCapture) {
m_nullCount = 0;
m_needRetry = false;
return;
}
if (preview.isNull()) {
if (++m_nullCount > MAX_NULL_RETRY_COUNT) {
qWarning() << "captured image is null too many times, aborting";
m_nullCount = 0;
m_needRetry = false;
Q_EMIT processStatus(m_actionId, FaceEnrollException);
return;
}
qWarning() << "captured image is null, scheduling retry capture";
m_needRetry = true; // 标记需要重试
// 替换 QThread::msleep,使用异步定时器延时触发,不阻塞事件循环
// 注意:如果 readyForCapture 会自动触发,这里甚至不需要 Timer,直接 return 即可
QTimer::singleShot(RETRY_DELAY_MS, this, [this]() {
if (!m_stopCapture && m_imageCapture) {
m_imageCapture->capture();
}
});
return;
}
// 成功获取图像,重置状态
m_nullCount = 0;
m_needRetry = false;
QImage img;
if (preview.size() == QSize(800, 600)) {
img = preview;
} else {
img = preview.scaled(800, 600, Qt::IgnoreAspectRatio, Qt::FastTransformation);
}
if (m_bFirst) {
sendCapture(img);
m_bFirst = false;
} else {
// ... 原有逻辑
sendCapture(img);
}
// 继续下一次捕获
if (m_imageCapture) {
m_imageCapture->capture();
}
if (!m_checkDone) {
// ...
}
}总结:你的修改方向非常正确,解决了空帧卡死和首帧判断的Bug。主要的改进点在于避免在槽函数中进行阻塞睡眠,以及更安全地处理重试逻辑以避免信号堆积。希望这些建议对你有所帮助! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, mhduiy 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 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
|
/forcemerge |
|
This pr force merged! (status: unknown) |
ErollThread::processCapturedImageto detect when the capturedQImage previewis empty.QThread::msleep(10), and immediately triggerm_imageCapture->capture()again before returning.sendCapture(img), and the follow-up capture scheduling.Influence:
QImagefrom the capture source and verify the warning is printed and capture automatically retries.fix: 处理空采集图片重试
ErollThread::processCapturedImage中新增空图片判断,用于检测采集 到的QImage preview是否为空。QThread::msleep(10)进行短暂等 待,并立即重新调用m_imageCapture->capture()后返回。sendCapture(img)以及后续继续触发采集。Influence:
QImage的情况,验证会输出告警日志并自动重试 采集。PMS: BUG-352653
Change-Id: I3cc71b9c41b6b4ddef1c8c6638c3db15fa3a9407