Skip to content

fix: Add DConfig for preview with no delay.#455

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle
Mar 4, 2026
Merged

fix: Add DConfig for preview with no delay.#455
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle

Conversation

@lichaofan2008
Copy link
Contributor

Add DConfig for preview with no delay.
DConfig中增加关于预览无延迟的配置。

Bug: https://pms.uniontech.com/bug-view-351345.html

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @lichaofan2008, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Add DConfig for preview with no delay.
DConfig中增加关于预览无延迟的配置。

Bug: https://pms.uniontech.com/bug-view-351345.html
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码主要实现了一个通过配置项(previewNoDelay)来控制相机预览是否延迟的功能。当previewNoDelaytrue时,取消原有的msleep(33)延迟,旨在实现"无延迟"预览。

以下是对该代码的审查意见,涵盖语法逻辑、代码质量、性能和安全方面:

1. 语法与逻辑审查

  • 逻辑正确性

    • 代码逻辑基本正确。通过dconfig读取配置,存入单例DataManager,并在渲染线程中读取该配置来决定是否休眠。
    • 潜在逻辑缺陷msleep(33)意味着将帧率限制在约 30 FPS。移除这个休眠意味着帧率将不再受此代码限制,而是取决于硬件性能和上游数据源的速度。如果上游数据源(如摄像头驱动或解码器)产生数据的速度极快(例如 120 FPS),移除休眠可能导致 UI 线程或渲染线程过载,反而造成界面卡顿或 CPU 占用飙升。代码中仅判断了m_nVdWidth(宽度),并未判断高度或分辨率负载,逻辑略显单薄。
  • 命名规范

    • previewNoDelay:命名清晰,符合布尔变量通常的命名习惯。
    • isTrue:作为参数名略显泛泛,建议改为更具体的名称,如 enablednoDelay

2. 代码质量审查

  • 线程安全

    • 严重问题DataManager 是一个单例,m_isPreviewNoDelay 是一个普通的成员变量。main.cpp(主线程)在初始化时写入该变量,而 MajorImageProcessingThread(工作线程)在 run() 循环中读取该变量。
    • 虽然在 main 函数启动时写入通常被认为是"初始化阶段",但 bool 类型的读写在 C++ 中并非原子操作(尽管在大多数现代架构上通常是原子的),且缺乏内存屏障。如果系统支持在运行时动态修改配置(DConfig 通常支持热更新),这就构成了典型的数据竞争
    • 建议:应使用 std::atomic<bool> 来声明 m_isPreviewNoDelay,或者确保配置的监听和更新机制是线程安全的。
  • 代码一致性

    • datamanager.h 中,isSupportCameraSwitch 等函数没有使用 const 修饰符,新增的 isPreviewNoDelay() 也没有使用。虽然保持了风格一致,但这不是良好的 C++ 实践。
    • 建议bool isPreviewNoDelay() const { return m_isPreviewNoDelay; }
  • 配置检查冗余

    • main.cpp 中检查 dconfig->keyList().contains("previewNoDelay") 略显冗余。通常 JSON 配置文件是版本控制的,如果 key 不存在,dconfig->value("previewNoDelay") 通常会返回默认值(根据后端实现,可能是 QVariant 的默认值)。如果是为了防止配置文件未更新的情况,保留是可以的,但建议增加日志输出,提示用户使用了默认值。

3. 性能审查

  • 性能影响
    • previewNoDelaytrue 时,循环中移除了休眠。这会显著增加 CPU 占用率,因为线程会尽可能快地处理数据。
    • 如果摄像头输出帧率很高,GPU 渲染压力也会增大。
    • 建议:考虑是否真的需要"完全无延迟"。通常"低延迟"比"无延迟"更可控。或许可以将 msleep(33) 改为 msleep(16)(约 60 FPS)或者更小的值,而不是完全移除,以平衡性能和流畅度。

4. 安全审查

  • 输入验证
    • dconfig->value("previewNoDelay").toBool() 会将输入转换为布尔值。这相对安全,不会导致崩溃。
  • 资源耗尽风险
    • 如性能审查所述,取消休眠可能导致 CPU 100% 占用。在嵌入式设备或低性能 PC 上,这可能导致系统发热严重或其他进程响应变慢。

改进建议代码

1. datamanager.h (改进线程安全和 const 修饰)

#include <atomic>

class DataManager: public QObject
{
    // ... 其他代码 ...
public:
    /**
     * @brief 获取是否预览无延迟
     * @return 
     */
    bool isPreviewNoDelay() const { return m_isPreviewNoDelay; }; // 添加 const

    /**
     * @brief 设置是否预览无延迟
     * @param noDelay 改名以更清晰
     */
    void setPreviewNoDelay(bool noDelay) { m_isPreviewNoDelay = noDelay; };

private:
    // 使用 atomic 保证线程安全
    std::atomic<bool> m_isPreviewNoDelay {false}; 
};

2. majorimageprocessingthread.cpp (优化逻辑)

void MajorImageProcessingThread::run()
{
    // ... 前置代码 ...
    while (!isInterruptionRequested()) {
        // ... 处理图像逻辑 ...

        // 改进:如果开启无延迟模式,可以设置一个更小的休眠时间(例如 10ms,约 100 FPS)
        // 或者根据分辨率动态调整,而不是完全不休眠,防止 CPU 空转
        if (!DataManager::instance()->isPreviewNoDelay()) {
            if (m_nVdWidth <= 1920) {
                msleep(33); // 普通模式限制 ~30FPS
            }
        } else {
            // 无延迟模式下,给极其微小的让出时间片机会,防止完全饿死其他同级线程
            // 或者根据实际硬件能力,例如限制在 60FPS (16ms)
            if (m_nVdWidth > 1920) {
                // 高分辨率下即使无延迟,也适当限制一下保护系统
                msleep(10); 
            }
            // 低分辨率下完全不休眠,追求极速
        }
    }
}

3. main.cpp (增加日志)

    if (dconfig && dconfig->isValid()) {
        // 读取配置,如果 key 不存在,value() 通常返回 QVariant()
        // 如果 JSON 中定义了默认值 false,这里也会读到 false
        bool noDelay = dconfig->value("previewNoDelay", false).toBool();
        DataManager::instance()->setPreviewNoDelay(noDelay);
        
        // 可选:打印日志,方便调试
        qDebug() << "Preview No Delay mode set to:" << noDelay;
    }

总结

这段代码实现了基本功能,但在多线程安全性上存在隐患(非原子变量读写),在性能控制上较为激进(完全移除休眠可能导致 CPU 负载过高)。建议使用 std::atomic 修饰布尔标志,并考虑在"无延迟"模式下保留极短时间的休眠或根据分辨率动态调整策略,以获得更好的系统稳定性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, max-lvs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@lichaofan2008
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 4, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 452f400 into linuxdeepin:release/eagle Mar 4, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants