-
Notifications
You must be signed in to change notification settings - Fork 31
perf(sound): optimize audio device switching performance #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
perf(sound): optimize audio device switching performance #423
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideOptimizes 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 handlingsequenceDiagram
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
Class diagram for SoundController default sink handlingclassDiagram
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
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this 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 existingm_defaultSinkInteruntouched; 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.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
39e8cba to
3758e4a
Compare
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
deepin pr auto review这段代码的改动是删除了 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结与建议结论:删除 进一步优化建议: 鉴于注释中提到的需求(防止信号混乱),如果后续测试发现删除 // 在头文件中添加成员变量
// 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();
// ... 后续代码
}最终建议: |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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:
Enhancements: