Skip to content

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Jan 6, 2026

Fixed the existActiveOutputDevice() method to include an additional check for empty device name. The previous logic could incorrectly identify an active output device when the default sink interface exists but has an empty name. This ensures that devices with empty names are not considered active output devices, improving the accuracy of device detection in sound controller.

Influence:

  1. Test sound output device detection when no physical devices are connected
  2. Verify that the dock sound plugin correctly shows/hides based on active device presence
  3. Test with virtual audio devices that might have empty or null names
  4. Verify compatibility with cloud platforms that may not have audio ports

fix: 修复活动输出设备检测逻辑

修复了existActiveOutputDevice()方法,增加了对空设备名称的检查。之前的逻
辑在默认音频输出接口存在但名称为空时可能会错误地识别为活动输出设备。此修
复确保名称为空的设备不会被识别为活动输出设备,提高了声音控制器中设备检测
的准确性。

Influence:

  1. 测试在没有物理设备连接时的声音输出设备检测
  2. 验证Dock声音插件是否根据活动设备存在正确显示/隐藏
  3. 测试可能具有空名称或null名称的虚拟音频设备
  4. 验证与可能没有音频端口的云平台的兼容性

Summary by Sourcery

Bug Fixes:

  • Prevent devices with empty names from being treated as active output devices when only a default sink interface exists.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 6, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts the logic for detecting an active output device in the dock sound controller by adding a guard that ignores default sink interfaces with empty names, preventing false positives when no real audio device is present.

Class diagram for updated SoundController existActiveOutputDevice logic

classDiagram
class SoundController {
  +bool existActiveOutputDevice() const
  -SinkInterface m_defaultSinkInter
}

class SoundModel {
  +static SoundModel ref()
  +QList~Port~ ports()
}

class SinkInterface {
  +QString name()
}

SoundController --> SoundModel : uses
SoundController --> SinkInterface : holds
Loading

Flow diagram for updated existActiveOutputDevice decision logic

flowchart TD
  A[existActiveOutputDevice] --> B{SoundModel_ref_ports_isEmpty}
  B -- no --> N[return false]
  B -- yes --> C{m_defaultSinkInter_not_null}
  C -- no --> N
  C -- yes --> D{name_startsWith_auto_null}
  D -- yes --> N
  D -- no --> E{name_contains_bluez}
  E -- yes --> N
  E -- no --> F{name_isEmpty}
  F -- yes --> N
  F -- no --> P[return true]
Loading

File-Level Changes

Change Details Files
Tighten active output device detection to exclude default sink interfaces with empty or invalid names.
  • Extend the existActiveOutputDevice check to require that the default sink interface name is not empty, in addition to existing checks for auto_null and bluez virtual devices.
  • Preserve the existing condition that active devices are only considered when there are no ports, a default sink interface exists, and its name does not indicate auto_null or bluez backends.
plugins/dde-dock/sound/soundcontroller.cpp

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

@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 left some high level feedback:

  • Consider caching m_defaultSinkInter->name() in a local const auto &name variable to avoid repeated method calls and make the condition more readable.
  • You might want to check !m_defaultSinkInter->name().isEmpty() earlier in the condition (before startsWith/contains) to short-circuit quickly when the name is empty.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider caching `m_defaultSinkInter->name()` in a local `const auto &name` variable to avoid repeated method calls and make the condition more readable.
- You might want to check `!m_defaultSinkInter->name().isEmpty()` earlier in the condition (before `startsWith`/`contains`) to short-circuit quickly when the name is empty.

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.

@mhduiy mhduiy force-pushed the sound branch 2 times, most recently from b13041a to c638d4b Compare January 6, 2026 06:11
@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个diff进行详细的代码审查:

  1. 语法逻辑改进:
  • 在onDefaultSinkChanged函数中添加了对path有效性的检查,这是很好的防御性编程实践。
  • 在existActiveOutputDevice函数中,将直接访问m_defaultSinkInter->name()改为先检查m_defaultSinkInter是否存在,避免了潜在的空指针访问。
  1. 代码质量改进:
  • 使用了有意义的变量名sinkName,提高了代码可读性。
  • 添加了有意义的警告日志,便于调试和问题追踪。
  • 代码结构更清晰,将复杂的条件判断拆解成更容易理解的形式。
  1. 代码性能改进:
  • 使用QString的引用(const QString&)来避免不必要的字符串拷贝。
  • 提前检查path的有效性,避免创建不必要的DBusSink对象。
  1. 代码安全改进:
  • 防止了空指针解引用的风险。
  • 增加了对DBus路径有效性的验证,防止无效路径导致的问题。

建议进一步改进的地方:

  1. 可以考虑将path.path() == "/" || path.path().isEmpty()这个条件提取为一个单独的辅助函数,比如isValidPath(),以提高代码的可维护性。
  2. 在existActiveOutputDevice函数中,可以考虑将复杂的条件判断拆分成多个有意义的布尔变量,使代码更易读。
  3. 可以考虑为DBus相关的操作添加异常处理,以应对可能的DBus通信错误。

总体来说,这些改动提高了代码的健壮性和可维护性,是一个很好的改进。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

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

The condition for checking if an active output device exists was
incomplete. Previously, it only checked if the default sink interface
existed and its name didn't start with "auto_null" or contain "bluez".
However, it didn't verify that the name was non-empty, which could lead
to false positives when the name is empty.

Added an additional check `!m_defaultSinkInter->name().isEmpty()` to
ensure the device name is not empty before considering it as an active
output device. This prevents incorrectly identifying empty-named devices
as active output devices, which is important for proper audio device
detection and UI state management in the dock sound plugin.

Influence:
1. Test sound device detection when no physical audio devices are
connected
2. Verify dock sound plugin correctly shows/hides audio controls based
on available devices
3. Test scenarios with virtual audio devices that might have empty names
4. Verify Bluetooth audio device detection still works correctly
5. Test audio switching functionality between different output devices

fix: 修复活动输出设备检测逻辑

活动输出设备存在的检查条件不完整。之前仅检查默认输出接口是否存在且其名称
不以"auto_null"开头或不包含"bluez"。但未验证名称是否非空,这可能导致名称
为空时产生误判。

添加了额外的检查条件 `!m_defaultSinkInter->name().isEmpty()`,确保在将设
备视为活动输出设备之前,设备名称不为空。这可以防止将空名称的设备错误识别
为活动输出设备,对于dock声音插件中的音频设备检测和UI状态管理非常重要。

Influence:
1. 测试无物理音频设备连接时的声音设备检测
2. 验证dock声音插件根据可用设备正确显示/隐藏音频控制
3. 测试虚拟音频设备可能具有空名称的情况
4. 验证蓝牙音频设备检测仍正常工作
5. 测试不同输出设备之间的音频切换功能

PMS: BUG-345945
@mhduiy
Copy link
Contributor Author

mhduiy commented Jan 6, 2026

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Jan 6, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 20fe385 into linuxdeepin:master Jan 6, 2026
6 of 10 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