Skip to content

Conversation

@Ivy233
Copy link

@Ivy233 Ivy233 commented Feb 4, 2026

Filter invalid sink paths immediately before processing to avoid unnecessary delays. Remove the 200ms blocking sleep that was originally used for debouncing, as invalid signals are now filtered early and valid signals arrive with sufficient intervals (typically >250ms).

This change improves UI responsiveness by eliminating the blocking delay while maintaining stable device switching behavior.

优化音频设备切换性能

在处理前立即过滤无效的 sink 路径以避免不必要的延迟。移除原本用于
防抖的 200ms 阻塞延迟,因为无效信号现在会被提前过滤,而有效信号
到达的间隔已足够大(通常 >250ms)。

此改动通过消除阻塞延迟来提升 UI 响应速度,同时保持稳定的设备切换
行为。

Log: optimize audio device switching performance
PMS: BUG-295929

Summary by Sourcery

Optimize audio device switching responsiveness by removing unnecessary blocking delay and filtering invalid sink paths earlier.

Bug Fixes:

  • Prevent redundant and confusing audio device switches by ignoring invalid default sink paths before creating the sink interface.

Enhancements:

  • Eliminate the 200ms blocking sleep during default sink changes to improve UI responsiveness when switching audio devices.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Optimizes audio device default sink change handling by removing a blocking debounce delay and validating sink paths before creating the DBus sink interface, improving UI responsiveness while preserving stable behavior.

Sequence diagram for optimized default sink change handling

sequenceDiagram
    participant AudioBackend
    participant SoundController
    participant DBus
    participant DBusSink
    participant SoundModel

    AudioBackend->>SoundController: onDefaultSinkChanged(path)
    SoundController->>SoundController: check path.path() == "/" or isEmpty()
    alt invalid sink path
        SoundController->>SoundController: log warning and return
    else valid sink path
        SoundController->>SoundController: delete m_defaultSinkInter if not null
        SoundController->>DBus: create DBusSink with org.deepin.dde.Audio1 and path.path()
        DBus-->>SoundController: new DBusSink instance
        SoundController->>SoundModel: SoundModel::ref().setActivePort(DBusSink.card(), DBusSink.activePort().name)
    end
Loading

Class diagram for SoundController default sink handling

classDiagram
    class SoundController {
        - DBusSink* m_defaultSinkInter
        + void SetMute(bool in0)
        + void onDefaultSinkChanged(QDBusObjectPath path)
    }

    class DBusSink {
        + DBusSink(QString service, QString path, QDBusConnection connection, QObject parent)
        + int card()
        + Port activePort()
    }

    class Port {
        + QString name
    }

    class SoundModel {
        + static SoundModel& ref()
        + void setActivePort(int card, QString portName)
    }

    SoundController --> DBusSink : creates
    SoundController --> SoundModel : uses
    DBusSink --> Port : returns activePort
Loading

File-Level Changes

Change Details Files
Optimize default sink change handler by removing blocking debounce and early-filtering invalid sink paths before recreating the DBus sink interface.
  • Remove 200ms QThread::msleep-based debounce in the default sink change handler to avoid blocking the UI thread.
  • Add an early guard to skip handling when the sink DBus path is empty or root ("/"), logging a warning and returning immediately.
  • Retain and reorder cleanup of any existing default sink interface so it is deleted after validation but before creating a new DBusSink instance.
  • Keep subsequent logic that instantiates the DBusSink for the new path and updates the active audio port in the sound model unchanged.
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:

  • By moving the m_defaultSinkInter->deleteLater() call after the invalid-path check, an invalid sink path will now leave the existing m_defaultSinkInter untouched; please confirm whether the previous behavior (always deleting on signal) is still desired or if the deletion should happen before the early return to avoid a stale interface.
  • The new comment only mentions filtering invalid paths; consider briefly documenting in-code that the previous 200ms debounce was removed based on the assumption that valid sink change signals are already spaced sufficiently apart, so future readers understand why no explicit debouncing remains.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- By moving the `m_defaultSinkInter->deleteLater()` call after the invalid-path check, an invalid sink path will now leave the existing `m_defaultSinkInter` untouched; please confirm whether the previous behavior (always deleting on signal) is still desired or if the deletion should happen before the early return to avoid a stale interface.
- The new comment only mentions filtering invalid paths; consider briefly documenting in-code that the previous 200ms debounce was removed based on the assumption that valid sink change signals are already spaced sufficiently apart, so future readers understand why no explicit debouncing remains.

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.

Remove the 200ms blocking sleep (QThread::msleep) that was originally
used for debouncing multiple signals during device switching. Analysis
shows that valid signals arrive with sufficient intervals (>250ms),
making the blocking delay unnecessary and harmful to UI responsiveness.

移除音频设备切换中的阻塞延迟

移除原本用于防抖多个信号的 200ms 阻塞延迟(QThread::msleep)。
分析表明有效信号到达的间隔已足够大(>250ms),使得阻塞延迟既不必要
又损害 UI 响应速度。

Log: remove blocking delay in audio device switching
PMS: BUG-295929
@Ivy233 Ivy233 force-pushed the optimize/audio-switch-plan1 branch from 39e8cba to 3758e4a Compare February 4, 2026 09:55
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的改动是删除了 QThread::msleep(200); 这一行。这是一个非常好的改进。以下是对这段代码的详细审查和改进意见:

1. 语法逻辑

  • 审查意见:代码逻辑正确。删除该行不会破坏编译或运行逻辑。
  • 改进建议:无。

2. 代码质量

  • 审查意见
    • 原有问题QThread::msleep(200) 是一种强制当前线程(通常是主线程/GUI线程)休眠 200 毫秒的操作。在 GUI 应用程序中,阻塞主线程会导致界面冻结("卡顿"),用户无法进行任何操作,体验极差。
    • 改进效果:删除该行后,主线程不再被强制阻塞,界面响应流畅度得到提升。
  • 改进建议
    • 注释中提到"防止手动切换设备...获取到多个信号,设备切换多次"。这说明之前的休眠是为了"防抖"(Debounce),即等待信号稳定后再处理。虽然删除 sleep 解决了卡顿问题,但可能引入了新的逻辑风险:如果在短时间内连续收到多个 onDefaultSinkChanged 信号,可能会导致 m_defaultSinkInter 对象被频繁销毁和重建,或者业务逻辑被多次触发。
    • 替代方案:如果确实需要防止短时间内多次触发,建议使用 QTimer::singleShot 进行延时处理,或者使用信号防抖逻辑,而不是阻塞线程。

3. 代码性能

  • 审查意见
    • 原有问题:休眠 200ms 纯粹是浪费 CPU 时间片,且降低了系统的响应吞吐量。
    • 改进效果:移除休眠后,CPU 资源利用率更合理,函数立即返回,性能提升。
  • 改进建议:无。

4. 代码安全

  • 审查意见
    • 原有问题:在主线程使用 msleep 是不安全的,因为它阻止了事件循环的处理,可能导致网络超时、心跳丢失或其他依赖于事件循环的机制失效。
    • 改进效果:移除后消除了阻塞带来的潜在风险。
  • 改进建议
    • 需要注意 m_defaultSinkInter->deleteLater() 的调用。如果在极短时间内 onDefaultSinkChanged 被调用两次,第一次调用将旧对象标记为删除,第二次调用可能再次尝试删除它(虽然 deleteLater 对空指针是安全的,但需要确保 m_defaultSinkInter 的赋值逻辑是线程安全的,且在两次调用之间没有悬空指针的访问风险)。

总结与建议

结论:删除 QThread::msleep(200) 是一个正确的方向,解决了界面卡顿和阻塞主线程的问题。

进一步优化建议

鉴于注释中提到的需求(防止信号混乱),如果后续测试发现删除 sleep 后确实出现了设备快速切换导致的逻辑错误,建议采用以下非阻塞的"防抖"方案:

// 在头文件中添加成员变量
// QTimer *m_debounceTimer;

// 在构造函数中初始化
// m_debounceTimer = new QTimer(this);
// m_debounceTimer->setSingleShot(true);
// connect(m_debounceTimer, &QTimer::timeout, this, [this](){
//     // 实际的处理逻辑可以放在这里,或者通过标志位控制
//     // 这里仅仅是为了演示如何替代 sleep
// });

void SoundController::onDefaultSinkChanged(const QDBusObjectPath &path)
{
    // 防止手动切换设备,与后端交互时,获取到多个信号,设备切换多次,造成混乱
    // 停止之前的计时器(如果正在运行),重新开始计时
    if (m_debounceTimer->isActive()) {
        m_debounceTimer->stop();
    }
    
    // 延迟 200ms 执行,但如果在 200ms 内再次调用,则重新计时
    // 这里为了简单,我们假设逻辑是直接执行,实际可能需要将后续逻辑移到 timeout 槽函数中
    // 或者仅仅利用这个 timer 来屏蔽中间的信号
    
    // 如果只是想屏蔽中间的信号,可以这样做:
    // 1. 记录最后一次的 path
    // 2. 启动 timer
    // 3. timer 触发时,使用记录的最后一次 path 进行处理
    
    // 但由于当前代码结构是直接处理,我们可以简单地利用 timer 延迟处理
    // 为了不破坏现有结构太多,这里仅展示如何替换 sleep 的思路:
    
    // 如果必须保留原有的处理逻辑流,且不想大改结构,
    // 可以考虑使用 QEventLoop 稍微等待(虽然也不推荐,但比 msleep 稍好,因为能处理事件),
    // 但最好的方式还是重构为异步处理。

    // 以下是针对当前 diff 的最小侵入式修改建议(如果需要防抖):
    // 使用静态变量或成员变量记录上次处理时间
    // static qint64 lastTime = 0;
    // qint64 currentTime = QDateTime::currentMSecsSinceEpoch();
    // if (currentTime - lastTime < 200) {
    //     return; // 忽略 200ms 内的重复信号
    // }
    // lastTime = currentTime;

    if (m_defaultSinkInter)
        m_defaultSinkInter->deleteLater();
        
    // ... 后续代码
}

最终建议
如果删除 sleep 后系统运行稳定,则保持现状。如果出现信号抖动问题,请不要加回 sleep,而是采用时间戳检查QTimer 延迟处理的方式来实现防抖逻辑。

@Ivy233
Copy link
Author

Ivy233 commented Feb 4, 2026

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Feb 4, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit ee86167 into linuxdeepin:master Feb 4, 2026
8 of 10 checks passed
@Ivy233 Ivy233 deleted the optimize/audio-switch-plan1 branch February 4, 2026 10:06
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.

4 participants