Skip to content

feat: Implement hardware acceleration driver configuration support#402

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
add-uos:release/eagle
Oct 11, 2025
Merged

feat: Implement hardware acceleration driver configuration support#402
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
add-uos:release/eagle

Conversation

@add-uos

@add-uos add-uos commented Oct 11, 2025

Copy link
Copy Markdown
Contributor
  • Added set/get functions for LIBVA_DRIVER_NAME and VDPAU_DRIVER environment variables
  • Integrated driver configuration loading from dconfig settings during application startup
  • Set environment variables for VAAPI and VDPAU hardware codec acceleration
  • Enhanced configuration file with corrected parameter names and descriptions
  • Added debug logging for driver configuration values and environment variables

Log: Add hardware acceleration driver configuration support
Bug: https://pms.uniontech.com/bug-view-335297.html

Summary by Sourcery

Implement hardware acceleration driver configuration support by introducing APIs for driver name management, loading settings from configuration, exporting environment variables, and adding debug logs

New Features:

  • Add APIs to set and get LIBVA and VDPAU driver names
  • Load hardware acceleration driver names from dconfig at application startup
  • Set LIBVA_DRIVER_NAME and VDPAU_DRIVER environment variables based on config

Enhancements:

  • Add debug logging for driver configuration values and environment variables
  • Correct parameter names and descriptions in the configuration file

- Added set/get functions for LIBVA_DRIVER_NAME and VDPAU_DRIVER
environment variables
- Integrated driver configuration loading from dconfig settings during
application startup
- Set environment variables for VAAPI and VDPAU hardware codec
acceleration
- Enhanced configuration file with corrected parameter names and
descriptions
- Added debug logging for driver configuration values and environment
variables

Log: Add hardware acceleration driver configuration support
Bug: https://pms.uniontech.com/bug-view-335297.html
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这段代码主要添加了对libva和libvdpau驱动名称的支持,我来分析一下代码的各个方面:

语法逻辑

  1. 代码语法正确,逻辑清晰
  2. 新增的函数和变量命名规范,符合项目风格
  3. 条件判断合理,空指针检查到位

代码质量

  1. 优点:

    • 添加了适当的注释说明函数用途
    • 使用了安全的字符串操作(strncpy并指定长度)
    • 实现了getter/setter模式,封装性良好
  2. 可改进之处:

    • 字符串数组大小(200)是硬编码的,建议定义为宏或常量
    • set函数中重复了相同的memset+strncpy逻辑,可以抽取为公共函数

代码性能

  1. 性能表现良好:

    • 字符串操作只在实际需要时进行
    • 使用了高效的内存操作函数memset和strncpy
  2. 可优化点:

    • 频繁的环境变量设置(qputenv)可能影响性能,可以考虑只在程序启动时设置一次

代码安全

  1. 安全措施得当:

    • 对输入参数进行了NULL检查
    • 使用strncpy防止缓冲区溢出
    • 正确处理了字符串终止符
  2. 安全建议:

    • 可以考虑添加输入长度检查,防止超长字符串被截断
    • 建议为驱动名称添加白名单验证,只允许特定的驱动名称

具体改进建议

  1. 添加常量定义:
#define MAX_DRIVER_NAME_LENGTH 200
static char libva_driver_name[MAX_DRIVER_NAME_LENGTH];
static char libvdpau_driver_name[MAX_DRIVER_NAME_LENGTH];
  1. 抽取公共函数:
static void set_driver_name(char *driver_name, const char *name, size_t max_len) {
    if (name != NULL) {
        memset(driver_name, 0, max_len);
        strncpy(driver_name, name, max_len - 1);
    } else {
        memset(driver_name, 0, max_len);
    }
}
  1. 添加输入验证:
void set_libva_driver_name(const char *name) {
    if (name != NULL && strlen(name) < MAX_DRIVER_NAME_LENGTH - 1) {
        set_driver_name(libva_driver_name, name, MAX_DRIVER_NAME_LENGTH);
    } else {
        memset(libva_driver_name, 0, MAX_DRIVER_NAME_LENGTH);
    }
}
  1. 环境变量设置优化:
    可以在程序启动时一次性设置环境变量,而不是每次检查都设置:
void set_environment_variables() {
    if (!libVaDriverName.isEmpty()) {
        qputenv("LIBVA_DRIVER_NAME", libVaDriverName.toLocal8Bit());
    }
    if (!libVdpauDriverName.isEmpty()) {
        qputenv("VDPAU_DRIVER", libVdpauDriverName.toLocal8Bit());
    }
}

总体来说,这段代码的实现是合理的,但可以通过上述建议进一步提高代码的健壮性和可维护性。

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src/main.cpp:146` </location>
<code_context>
         set_project_id(projectId.toUtf8().constData());
     }
+
+    QString libVaDriverName = "";
+    if (dconfig && dconfig->isValid() && dconfig->keyList().contains("libVaDriverName")) {
+        libVaDriverName = dconfig->value("libVaDriverName").toString();
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring the repeated driver configuration logic into a single helper function to reduce duplication and centralize future changes.

You can collapse all of the repeated “read‐key → call C-API → setenv → log” logic into a single helper. For example:

```cpp
// place this helper near the top of your function (or in an anonymous/local namespace)
auto configureDriver = [&](const char* configKey,
                          void (*setter)(const char*),
                          const char* envVarName)
{
    if (dconfig && dconfig->isValid() && dconfig->keyList().contains(configKey)) {
        const QString name = dconfig->value(configKey).toString();
        setter(name.toUtf8().constData());
        if (!name.isEmpty())
            qputenv(envVarName, name.toLocal8Bit());
        qInfo() << QString("%1 value is:").arg(configKey) << name
                << envVarName << qgetenv(envVarName);
    }
};
```

Then replace your two blocks with:

```cpp
configureDriver("libVaDriverName",    set_libva_driver_name,    "LIBVA_DRIVER_NAME");
configureDriver("libVdpauDriver",     set_libvdpau_driver_name, "VDPAU_DRIVER");
```

This keeps all functionality intact, removes duplication, and centralizes any future tweaks to logging, env-setting or config-access.
</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.

@sourcery-ai

sourcery-ai Bot commented Oct 11, 2025

Copy link
Copy Markdown

Reviewer's Guide

This PR adds support for configuring hardware acceleration drivers by introducing new driver name accessors, loading settings from dconfig at startup to set LIBVA_DRIVER_NAME and VDPAU_DRIVER environment variables (with debug logging), and correcting parameter names in the application's configuration file.

Sequence diagram for driver configuration loading and environment setup at startup

sequenceDiagram
    participant App as "Application Startup"
    participant DConfig as "dconfig (settings)"
    participant CamView as "camview (driver accessors)"
    participant Env as "Environment"
    App->>DConfig: Load configuration
    DConfig-->>App: Return driver names (libVaDriverName, libVdpauDriver)
    App->>CamView: set_libva_driver_name(libVaDriverName)
    App->>CamView: set_libvdpau_driver_name(libVdpauDriverName)
    App->>Env: Set LIBVA_DRIVER_NAME
    App->>Env: Set VDPAU_DRIVER
    App->>App: Log driver names and env variables
Loading

Class diagram for new hardware driver name accessors

classDiagram
    class camview {
        +void set_libva_driver_name(const char *name)
        +const char* get_libva_driver_name(void)
        +void set_libvdpau_driver_name(const char *name)
        +const char* get_libvdpau_driver_name(void)
    }
Loading

File-Level Changes

Change Details Files
Introduce libVA and VDPAU driver name accessors in the camview API
  • Declared set_libva_driver_name/get_libva_driver_name and set_libvdpau_driver_name/get_libvdpau_driver_name in the header
  • Added static buffers and implemented safe string copy logic for driver names in the source
libcam/libcam/camview.h
libcam/libcam/camview.c
Load driver configuration and set environment variables with debug logging
  • Read libVaDriverName and libVdpauDriver keys from dconfig in main()
  • Invoke the new set_* functions and call qputenv for LIBVA_DRIVER_NAME and VDPAU_DRIVER
  • Added qInfo statements to log driver names and the resulting environment variable values
src/main.cpp
Correct configuration parameters in the encode JSON schema
  • Fixed parameter names and updated descriptions in the org.deepin.camera.encode.json file
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

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, 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

@add-uos

add-uos commented Oct 11, 2025

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit 973d0cc into linuxdeepin:release/eagle Oct 11, 2025
18 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