Skip to content

fix(config): support low performance device config#480

Merged
max-lvs merged 1 commit into
linuxdeepin:release/eaglefrom
Resurgamz:release/eagle
Jun 5, 2026
Merged

fix(config): support low performance device config#480
max-lvs merged 1 commit into
linuxdeepin:release/eaglefrom
Resurgamz:release/eagle

Conversation

@Resurgamz
Copy link
Copy Markdown

@Resurgamz Resurgamz commented Jun 5, 2026

Log: 支持通过配置标记低性能设备
Bug: https://pms.uniontech.com/bug-view-358869.html
Influence: 可通过配置直接识别低性能设备,优化低性能设备判断逻辑。

Summary by Sourcery

New Features:

  • Allow marking low-performance devices via DConfig so they can be identified directly by configuration.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Jun 5, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds configuration-based detection of low performance devices using DConfig before falling back to existing DBus-based logic.

Sequence diagram for updated isLowPerformanceBoard configuration check

sequenceDiagram
    participant GlobalUtils
    participant DConfig
    participant SystemInfoService

    GlobalUtils->>DConfig: DConfig::create(org.deepin.camera, org.deepin.movie.encode)
    alt [dconfig && isValid && keyList contains isLowPerformanceDevice]
        GlobalUtils->>DConfig: value(isLowPerformanceDevice)
        alt [value.toBool is true]
            GlobalUtils-->>GlobalUtils: return true
        else [value.toBool is false]
            GlobalUtils-->>DConfig: delete dconfig
            GlobalUtils-->>SystemInfoService: continue DBus-based detection
        end
    else [no config or key]
        GlobalUtils-->>DConfig: delete dconfig
        GlobalUtils-->>SystemInfoService: continue DBus-based detection
    end

    GlobalUtils->>SystemInfoService: QDBusInterface(com.deepin.system.SystemInfo)
    SystemInfoService-->>GlobalUtils: isValid / properties (existing logic)
Loading

File-Level Changes

Change Details Files
Add optional DConfig-based shortcut for marking low performance devices before running DBus checks.
  • Include DConfig header to enable configuration-based access
  • Create a DConfig instance for org.deepin.camera/org.deepin.movie.encode when DTKCORE_CLASS_DConfigFile is available
  • Check for presence of isLowPerformanceDevice key and return true early when it is set to true
  • Ensure the DConfig instance is deleted in all control paths before continuing to existing DBus-based detection logic
src/src/globalutils.cpp
Adjust encode configuration asset to support new low performance device flag.
  • Update org.deepin.camera.encode configuration JSON to add or prepare key for isLowPerformanceDevice flag, aligning assets with new DConfig lookup
src/assets/org.deepin.camera.encode.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@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.

Hey - I've found 1 issue, and left some high level feedback:

  • Consider managing the DConfig instance with RAII (e.g., std::unique_ptr or QScopedPointer) to avoid manual new/delete and reduce the risk of lifetime/cleanup issues, especially with early returns.
  • You can simplify the DConfig logic by avoiding keyList().contains(...) on every call and instead directly reading the value with a default (e.g., value("isLowPerformanceDevice", false).toBool()), which will be more efficient and concise.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider managing the `DConfig` instance with RAII (e.g., `std::unique_ptr` or `QScopedPointer`) to avoid manual `new`/`delete` and reduce the risk of lifetime/cleanup issues, especially with early returns.
- You can simplify the DConfig logic by avoiding `keyList().contains(...)` on every call and instead directly reading the value with a default (e.g., `value("isLowPerformanceDevice", false).toBool()`), which will be more efficient and concise.

## Individual Comments

### Comment 1
<location path="src/src/globalutils.cpp" line_range="61" />
<code_context>
 bool GlobalUtils::isLowPerformanceBoard()
 {
+#ifdef DTKCORE_CLASS_DConfigFile
+    DTK_CORE_NAMESPACE::DConfig *dconfig = DTK_CORE_NAMESPACE::DConfig::create("org.deepin.camera","org.deepin.movie.encode");
+    if(dconfig && dconfig->isValid() && dconfig->keyList().contains("isLowPerformanceDevice")){
+        if (dconfig->value("isLowPerformanceDevice").toBool()) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Possible mismatch between DConfig ID and the JSON asset name

The config is created with ID "org.deepin.movie.encode" but the added asset is `org.deepin.camera.encode.json`. If DConfig requires this argument to match the config ID / file name, the config may never load and the override will silently be ignored. Please confirm whether this ID should be "org.deepin.camera.encode" (or otherwise aligned with the asset name) so the config is actually used.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/src/globalutils.cpp Outdated
Log: 支持通过配置标记低性能设备
Bug: https://pms.uniontech.com/bug-view-358869.html
Influence: 可通过配置直接识别低性能设备,优化低性能设备判断逻辑。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码的主要目的是为相机应用引入一种基于 DConfig 的低性能设备判定机制,作为原有基于 DBus 读取硬件主板信息的补充。整体思路清晰,但在代码逻辑、性能、代码质量和安全性方面存在一些需要改进的地方。

以下是详细的审查意见:

1. 语法与逻辑

  • 逻辑顺序错误(严重):在 globalutils.cpploadCameraConf 函数中,你先将 m_IsLowPerformanceDevice 置为 false,然后再去读取 DConfig。如果 DConfig 读取失败或键值不存在,m_IsLowPerformanceDevice 将保持 false,这是符合预期的。但是,在 isLowPerformanceBoard 函数中,如果 m_IsLowPerformanceDevicetrue,直接 return true;,这也没问题。然而,这里存在一个初始化时序依赖问题:如果 isLowPerformanceBoard()loadCameraConf() 之前被调用,m_IsLowPerformanceDevice 默认为 false,可能会漏判。请确保在应用启动早期优先调用 loadCameraConf()
  • 条件编译宏的误用(严重)#ifdef DTKCORE_CLASS_DConfigFile 这个宏通常用于检查 DTK 的特定类是否存在,但作为预处理器宏,它只能判断字面量是否被定义(即 #define DTKCORE_CLASS_DConfigFile),无法判断运行时或链接时 DConfig 相关的库是否真正可用。如果项目通过 CMake 的 find_packagepkg-config 引入 DTKCore,建议在 CMakeLists.txt 中通过 find_package 检测,并生成自定义的宏(如 HAS_DCONFIG),或者直接假定引入了 DTKCore 就一定有 DConfig(因为 DConfig 是 DTKCore 的基础模块),去掉这个 #ifdef

2. 代码性能

  • 不必要的堆内存分配:在 isLowPerformanceBoard 函数中:
    QDBusInterface *monitorInterface = new QDBusInterface("com.deepin.system.SystemInfo", ...);
    使用了 new 在堆上创建 QDBusInterface,这不仅增加了开销,还需要手动管理内存(代码中未见 delete,存在内存泄漏)。建议直接在栈上创建:
    QDBusInterface monitorInterface("com.deepin.system.SystemInfo", "/com/deepin/system/SystemInfo", "org.freedesktop.DBus.Properties", QDBusConnection::systemBus());
    if (!monitorInterface.isValid()) { ... }
  • DConfig 的重复创建:在 loadCameraConf 中,每次调用都会通过 DConfig::create 创建一个新的 DConfig 实例。DConfig 内部涉及文件解析和 DBus 通信,创建开销较大。如果该函数只会在程序启动时调用一次尚可接受,但如果未来有可能多次调用,建议将 dconfig 提升为类的静态成员变量,或者使用单例模式复用。

3. 代码质量

  • 命名规范不一致:新增的 JSON 键名 isLowPerformanceDevice 使用了小驼峰命名法,而同文件中的其他键名(如 resolutionWidth, preferredResolution)也是小驼峰,这点保持了一致。但是,C++ 中的静态成员变量 m_IsLowPerformanceDevice 建议与已有的 m_LowPerformanceBoards 保持风格统一,虽然不影响编译,但 m_isLowPerformanceDevice 更符合常见的成员变量命名规范。
  • 硬编码的字符串"org.deepin.camera", "org.deepin.camera.encode", "isLowPerformanceDevice" 这些字符串直接硬编码在代码中。建议将其提取为常量,与上方的 CAMERA_CONF_PATH 保持一致的风格,便于后续维护。
  • 版权年份更新2020 ~ 2026 的修改没有问题,但请注意 SPDX 标签 SPDX-FileCopyrightText: 2023 UnionTech 中的年份未同步更新,建议保持一致。

4. 代码安全

  • DBus 调用缺乏超时控制monitorInterface->call("Get", ...) 默认使用 DBus 的超时时间(通常为25秒)。如果系统总线阻塞或 com.deepin.system.SystemInfo 服务无响应,会导致当前线程卡死。建议使用带有超时参数的 call 方法,例如 monitorInterface->call(QDBus::BlockWithGui, 3000, "Get", ...)(3秒超时)。
  • DBus 返回值缺乏有效性校验:通过 DBus 获取到的硬件信息直接进行了 toStringList() 转换和 contains 判断。如果 DBus 返回异常数据(如空值、非预期格式),可能导致逻辑错误。建议增加返回值的类型校验。
  • DConfig 权限与安全性:在 JSON 配置文件中,isLowPerformanceDevicepermissions 被设置为 "readwrite",且 visibility"private"。这意味着本地用户可以通过修改 ~/.config/deepin/... 下的用户级配置文件来覆盖系统默认值,将设备伪装成低性能设备或高性能设备。如果该标志位涉及核心功能的降级/升级,且不希望被用户随意篡改,建议将 permissions 改为 "read",仅允许系统级配置控制。

改进后的代码建议

1. 修改 JSON 配置 (org.deepin.camera.encode.json)

        "isLowPerformanceDevice": {
            "value": false,
            "serial": 0,
            "flags": ["global"],
            "name": "is low performance device",
            "name[zh_CN]": "是否是低性能设备",
            "description": "Is low performance device",
            "permissions": "read",  // 建议:如果不希望用户随意篡改,改为只读
            "visibility": "private"
        }

2. 修改 C++ 头文件

class GlobalUtils
{
private:
    static QStringList m_LowPerformanceBoards;
    static bool m_isLowPerformanceDevice; // 统一为小驼峰
    
    // 建议增加常量提取
    static const QString DCONFIG_APP_ID;
    static const QString DCONFIG_META_NAME;
    static const QString DCONFIG_KEY_LOW_PERF;
};

3. 修改 C++ 实现文件

// Copyright (C) 2020 ~ 2026 Uniontech Software Technology Co.,Ltd.
// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later

#include "globalutils.h"

#include <QDBusArgument>
#include <QDBusReply>

#ifdef HAS_DCONFIG // 建议通过 CMake 传入更准确的宏
#include <DConfig>
#include <memory>
#endif

// 相机配置文件
const QString CAMERA_CONF_PATH = "/usr/share/deepin-camera/camera.conf";
#define LOW_PERFORMANCE_BOARD "LowPerformanceBoard"

// 提取常量
const QString GlobalUtils::DCONFIG_APP_ID = "org.deepin.camera";
const QString GlobalUtils::DCONFIG_META_NAME = "org.deepin.camera.encode";
const QString GlobalUtils::DCONFIG_KEY_LOW_PERF = "isLowPerformanceDevice";

QStringList GlobalUtils::m_LowPerformanceBoards = QStringList();
bool GlobalUtils::m_isLowPerformanceDevice = false;

// ... (DMIInfo 及其流操作符保持不变) ...

bool GlobalUtils::isLowPerformanceBoard()
{
#ifdef HAS_DCONFIG
    if (m_isLowPerformanceDevice) {
        return true;
    }
#endif

    qDBusRegisterMetaType<DMIInfo>();
    // 优化:使用栈对象代替堆对象,避免内存泄漏
    QDBusInterface monitorInterface("com.deepin.system.SystemInfo", 
                                    "/com/deepin/system/SystemInfo", 
                                    "org.freedesktop.DBus.Properties", 
                                    QDBusConnection::systemBus());
    if (!monitorInterface.isValid())
        return false;

    // 优化:增加超时控制(例如 3000 毫秒),防止 DBus 阻塞
    QDBusMessage reply = monitorInterface.call(QDBus::BlockWithGui, 3000, "Get", 
                                               "com.deepin.system.SystemInfo", "DMIInfo");
    if (reply.type() == QDBusMessage::ReplyMessage) {
        QVariant variant = reply.arguments().at(0);
        if (variant.canConvert<QDBusVariant>()) {
            QVariant innerVar = variant.value<QDBusVariant>().variant();
            if (innerVar.canConvert<QDBusArgument>()) {
                QDBusArgument arg = innerVar.value<QDBusArgument>();
                DMIInfo info;
                arg >> info;
                return m_LowPerformanceBoards.contains(info.boardName);
            }
        }
    }
    
    return false;
}

void GlobalUtils::loadCameraConf()
{
    QSettings settings(CAMERA_CONF_PATH, QSettings::IniFormat);
    settings.beginGroup("Base");
    QString boards = settings.value(LOW_PERFORMANCE_BOARD).toString();
    settings.endGroup();

    if (!boards.isEmpty()) {
        m_LowPerformanceBoards = boards.split(",", QString::SkipEmptyParts);
    } else {
        m_LowPerformanceBoards = QStringList();
    }

#ifdef HAS_DCONFIG
    // 逻辑优化:无需提前置 false,因为默认值已经是 false,且创建失败时不应覆盖可能存在的上一次状态
    std::unique_ptr<DTK_CORE_NAMESPACE::DConfig> dconfig(
        DTK_CORE_NAMESPACE::DConfig::create(DCONFIG_APP_ID, DCONFIG_META_NAME));
        
    if (dconfig && dconfig->isValid()) {
        if (dconfig->keyList().contains(DCONFIG_KEY_LOW_PERF)) {
            m_isLowPerformanceDevice = dconfig->value(DCONFIG_KEY_LOW_PERF).toBool();
        }
    } else {
        qWarning() << "DConfig is invalid or create failed for" << DCONFIG_APP_ID;
    }
#endif
}

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@Resurgamz
Copy link
Copy Markdown
Author

/merge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented Jun 5, 2026

This pr cannot be merged! (status: unstable)

@max-lvs max-lvs merged commit b3e6a27 into linuxdeepin:release/eagle Jun 5, 2026
20 of 22 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