Skip to content

fix: 修复摄像头重连和重启后分辨率恢复默认值#476

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
Resurgamz:release/eagle
May 28, 2026
Merged

fix: 修复摄像头重连和重启后分辨率恢复默认值#476
deepin-bot[bot] merged 1 commit into
linuxdeepin:release/eaglefrom
Resurgamz:release/eagle

Conversation

@Resurgamz
Copy link
Copy Markdown

ON 阶段 setData 触发 DTK widget 重建时读到 OFF 阶段清零的 value=0,
中间态的 resolutionchanged 异步将相机切到错误分辨率,settingDialog 重新打开时
以相机当前错误分辨率覆盖了已保存的用户选择。

  • ON 分支用 m_updatingResolution 标志抑制 setData/setOption 的中间态信号, QTimer::singleShot 在所有排队信号处理后发射一次正确值
  • settingDialog 优先读 option 已保存值,设备分辨率作为 fallback
  • switchCameraSuccess 连接 setNewResolutionList,启动和重连均触发 ON 分支恢复
  • switchCameraSuccess 无条件发射,构造函数路径也能进入 ON 分支

Log: 修复摄像头重连和应用重启后分辨率恢复为默认值
Bug: https://pms.uniontech.com/bug-view-363077.html

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.

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

Please try again later or upgrade to continue using Sourcery

@Resurgamz Resurgamz force-pushed the release/eagle branch 2 times, most recently from 1ad550e to 7d263c9 Compare May 28, 2026 07:51
  ON 阶段 setData 触发 DTK widget 重建时读到 OFF 阶段清零的 value=0,
  中间态的 resolutionchanged 异步将相机切到错误分辨率,settingDialog 重新打开时
  以相机当前错误分辨率覆盖了已保存的用户选择。

  - ON 分支用 m_updatingResolution 标志抑制 setData/setOption 的中间态信号,
    QTimer::singleShot 在所有排队信号处理后发射一次正确值
  - settingDialog 优先读 option 已保存值,设备分辨率作为 fallback
  - switchCameraSuccess 连接 setNewResolutionList,启动和重连均触发 ON 分支恢复
  - switchCameraSuccess 无条件发射,构造函数路径也能进入 ON 分支

  Log: 修复摄像头重连和应用重启后分辨率恢复为默认值
  Bug: https://pms.uniontech.com/bug-view-363077.html
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要涉及分辨率列表更新的逻辑优化、信号发射时机的控制(防抖/抑制中间态)、摄像头切换时的状态恢复,以及部分代码的健壮性修复

整体来看,这次修改解决了一些潜在的崩溃问题(如数组越界)和信号重复发射的问题,方向是正确的。但在语法逻辑、代码质量和性能方面,还有一些可以进一步改进和探讨的地方。

以下是详细的审查意见:

一、 语法与逻辑

  1. Lambda捕获潜在的生命周期问题(悬垂指针/引用)

    • 位置Settings.cpp 中的 QTimer::singleShot Lambda表达式。
    • 问题:Lambda按值捕获了 thisresolutionDatabasedefres。虽然 resolutionDatabasedefres 是按值捕获的(安全),但 this 是按值捕获的指针。如果 Settings 对象在 QTimer::singleShot 触发前被销毁,Lambda执行时访问 this->m_updatingResolutionemit 将导致崩溃。
    • 建议:使用 QPointer 来守护 this,或者在确保 Settings 生命周期长于事件循环的前提下,使用 this 的弱引用方式(如 QObject 的上下文传递)。更安全的写法如下:
      QTimer::singleShot(0, this, [this, resolutionDatabase, defres]() {
          // Qt内部会在this被销毁时自动断开连接,因为传递了context='this'
          // 但为了代码防御性编程,依然建议检查或使用QPointer
          m_updatingResolution = false;
          if (defres >= 0 && defres < resolutionDatabase.size()) {
              emit resolutionchanged(resolutionDatabase[defres]);
          }
      });
      注:由于 QTimer::singleShot 的第三个参数传递了 this 作为 context,Qt 的信号槽机制会保证如果 this 被销毁,该 Lambda 不会被执行。因此当前写法在 Qt 框架下是安全的,但建议加上注释说明这一点,以免后续维护者误以为存在悬垂指针风险。
  2. switchCameraSuccess 信号发射逻辑的语义变化

    • 位置videowidget.cpp
    • 旧代码if (devName && strlen(devName)) { emit switchCameraSuccess(devName); }
    • 新代码emit switchCameraSuccess(devName ? devName : "");
    • 问题:旧逻辑只有在设备名非空时才发信号,新逻辑无论设备名是否为空都会发信号。如果 devName 为空字符串,mainwindow.cppSettings.cpp 中的槽函数 onSwitchCameraSuccesssetNewResolutionList 是否能正确处理空字符串?如果空字符串代表异常状态,可能会引发后续逻辑错误。
    • 建议:确认下游槽函数对空字符串的容忍度。如果空字符串属于非法值,建议保留拦截:
      if (devName && devName[0] != '\0') { // 比strlen更高效
          emit switchCameraSuccess(devName);
      }

二、 代码质量

  1. 代码重复

    • 位置Settings.cppmainwindow.cpp 中关于分辨率匹配的逻辑完全重复了:
      if (resolutiontemp.size() >= 2 &&
              v4l2core_get_frame_width(get_v4l2_device_handler()) == resolutiontemp[0].toInt() &&
              v4l2core_get_frame_height(get_v4l2_device_handler()) == resolutiontemp[1].toInt())
    • 建议:违反了 DRY (Don't Repeat Yourself) 原则。建议在 Settings 类或工具类中提取一个公共函数,例如 int findMatchingResolutionIndex(const QStringList& db, int width, int height),两处统一调用,降低后续维护成本。
  2. 变量命名不规范

    • 位置Settings.h
    • 问题:新增成员变量 m_updatingResolution 符合 m_ 前缀的私有成员规范,但在 Qt 习惯中,布尔类型的标志位往往用 m_isUpdatingResolution 命名,语义更清晰。
    • 建议:将 m_updatingResolution 改为 m_isUpdatingResolution
  3. 类成员变量声明位置

    • 位置Settings.h
    • 问题bool m_updatingResolution = false; 被声明在 public slots: 区域内。虽然 C++ 的访问修饰符不影响内存布局,但从语义上看,这应该是一个内部状态变量,不应该暴露给外部。
    • 建议:将其移至 private: 区域。

三、 代码性能

  1. QString::split 的性能开销
    • 位置Settings.cppmainwindow.cpp
    • 问题resolutionDatabase[i].split("x") 每次都会在堆上分配内存创建 QStringList。如果 resolutionDatabase 辺界较大,在循环中频繁分配/释放内存会有性能损耗。
    • 建议:由于分辨率格式固定为 宽x高(如 1920x1080),可以使用 indexOfmid 来避免内存分配:
      const QString &resStr = resolutionDatabase[i];
      int sepIndex = resStr.indexOf('x');
      if (sepIndex > 0 && sepIndex < resStr.length() - 1) {
          int w = resStr.leftRef(sepIndex).toInt();
          int h = resStr.midRef(sepIndex + 1).toInt();
          if (v4l2core_get_frame_width(get_v4l2_device_handler()) == w &&
              v4l2core_get_frame_height(get_v4l2_device_handler()) == h) {
              defres = i;
              break;
          }
      }
      (注:如果分辨率列表通常只有几项到十几项,现有的 split 写法可读性更好,性能影响可忽略不计;如果列表可达上百项,则建议优化)

四、 代码安全

  1. devName 的空指针解引用风险

    • 位置videowidget.cpp
    • 旧代码if (devName && strlen(devName))
    • 新代码emit switchCameraSuccess(devName ? devName : "");
    • 分析:新代码通过三元运算符避免了空指针传给 QString 构造函数的问题(QString 构造函数接受 const char* 时,传 nullptr 是未定义行为或直接崩溃,所以 devName ? devName : "" 是正确且安全的做法)。
  2. 并发与重入风险

    • 位置Settings.cpp 中的 m_updatingResolution 标志位。
    • 问题m_updatingResolution = true; 是在普通函数中同步设置的,而 m_updatingResolution = false; 是在 QTimer::singleShot(0, ...) 的异步事件中重置的。如果 setNewResolutionList 在极短时间内被连续调用两次,第一次调用设置了 true 并投入定时器,第二次调用又设置了 true 并投入新的定时器。此时第一个定时器触发将 m_updatingResolution 设为 false,这会导致第二次的中间态拦截失效。
    • 建议:如果存在快速重入的可能,建议将 m_updatingResolution 改为整数计数器(引用计数),或者确保 setNewResolutionList 不会被并发/连续调用。如果是单线程 GUI 操作,通常不会重入,当前逻辑尚可,但需注意。

总结与修改建议代码示例

针对以上审查,我为你提供一份优化后的代码片段供参考:

Settings.h:

// 将 m_updatingResolution 移到 private,并改名
private:
    bool m_isUpdatingResolution = false;

Settings.cpp:

// ... 前面代码不变
for (int i = 0; i < resolutionDatabase.size(); i++) {
    // 优化:避免循环内频繁 split 分配内存,且更安全
    const QString &resStr = resolutionDatabase[i];
    int sepIndex = resStr.indexOf('x');
    if (sepIndex > 0 && sepIndex < resStr.length() - 1) {
        int w = resStr.leftRef(sepIndex).toInt();
        int h = resStr.midRef(sepIndex + 1).toInt();
        if (v4l2core_get_frame_width(get_v4l2_device_handler()) == w &&
                v4l2core_get_frame_height(get_v4l2_device_handler()) == h) {
            defres = i;
            break;
        }
    }
}

// 抑制中间态的 resolutionchanged, 仅在最后发射一次正确值
m_isUpdatingResolution = true;
resolutionmodeFamily->setData("items", resolutionDatabase);
m_settings->setOption(QString("outsetting.resolutionsetting.resolution"), defres);

// 注意:因为 context 传了 this,如果 this 销毁,该 lambda 不会执行,生命周期安全
QTimer::singleShot(0, this, [this, resolutionDatabase, defres]() {
    m_isUpdatingResolution = false;
    if (defres >= 0 && defres < resolutionDatabase.size()) {
        emit resolutionchanged(resolutionDatabase[defres]);
    }
});
// ... 后面代码不变

videowidget.cpp:

// 更安全的空指针判断,且保持原有"空名称不发信号"的语义
if (devName && devName[0] != '\0') {
    emit switchCameraSuccess(devName);
}

@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 May 28, 2026

This pr cannot be merged! (status: unstable)

@Resurgamz
Copy link
Copy Markdown
Author

/merge

@deepin-bot deepin-bot Bot merged commit c985023 into linuxdeepin:release/eagle May 28, 2026
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