fix: fix audio device switching and mute synchronization issues#428
fix: fix audio device switching and mute synchronization issues#428robertkill merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts how the dock sound controller manages the default sink DBus interface, fixes device activation timing by refreshing card info before activating, and revises mute/volume synchronization so volume==0 enforces mute while removing the previous auto‑unmute-on-volume-change behavior. Sequence diagram for updated default sink change handlingsequenceDiagram
participant DBusDaemon
participant SoundController
participant DBusSink
participant SoundModel
DBusDaemon->>SoundController: onDefaultSinkChanged(path)
alt previous m_defaultSinkInter exists
SoundController->>DBusSink: disconnect(this)
SoundController->>SoundModel: clear previous active sink state
end
SoundController->>DBusSink: create new DBusSink(path)
SoundController->>SoundModel: setCardsInfo(m_audioInter.cardsWithoutUnavailable())
SoundController->>SoundModel: setActivePort(card, activePort)
SoundController->>SoundModel: setMute(m_defaultSinkInter.mute())
alt existActiveOutputDevice
SoundController->>SoundModel: setVolume(m_defaultSinkInter.volume())
else no active output device
SoundController->>SoundModel: setVolume(0)
end
DBusSink-->>SoundController: MuteChanged(value)
SoundController->>SoundModel: setMute(m_defaultSinkInter.mute())
SoundController->>SoundModel: setVolume(m_defaultSinkInter.volume())
DBusSink-->>SoundController: VolumeChanged(value)
alt value == 0 and SoundModel.isMute() is false
SoundController->>SoundController: SetMute(true)
SoundController->>SoundModel: setVolume(0)
else value != 0 or already muted
SoundController->>SoundModel: setVolume(value)
end
DBusSink-->>SoundController: ActivePortChanged(port)
SoundController->>SoundModel: setActivePort(card, port.name)
Class diagram for updated sound controller, DBus sink, and sound modelclassDiagram
class SoundController {
- DBusSink m_defaultSinkInter
- AudioInterface m_audioInter
+ static SoundController ref()
+ void SetMute(bool in0)
+ void onDefaultSinkChanged(QDBusObjectPath path)
}
class DBusSink {
+ DBusSink(QString service, QString path, QDBusConnection connection, QObject parent)
+ int card()
+ AudioPort activePort()
+ bool mute()
+ double volume()
+ void SetMuteQueued(bool mute)
+ void disconnect(QObject receiver)
<<signal>> void MuteChanged(bool value)
<<signal>> void VolumeChanged(double value)
<<signal>> void ActivePortChanged(AudioPort port)
}
class SoundModel {
+ static SoundModel ref()
+ void setCardsInfo(QList~AudioCard~ cards)
+ void setActivePort(int cardId, QString portName)
+ void setMute(bool mute)
+ void setVolume(double volume)
+ bool isMute()
}
class AudioInterface {
+ QList~AudioCard~ cardsWithoutUnavailable()
}
class AudioPort {
+ QString name
}
class QDBusObjectPath {
+ QString path()
}
SoundController --> DBusSink : uses m_defaultSinkInter
SoundController --> SoundModel : updates state
SoundController --> AudioInterface : uses m_audioInter
SoundController --> QDBusObjectPath : onDefaultSinkChanged parameter
DBusSink --> AudioPort : returns activePort
AudioInterface --> AudioCard : returns list
SoundModel --> AudioCard : stores cards
SoundModel --> AudioPort : stores active port
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
onDefaultSinkChanged,m_defaultSinkInter->disconnect(this);only disconnects slots onthis, but the previousm_defaultSinkInteris also connected toSoundModel::ref(); if the intent is to avoid duplicate signals on subsequent connections, consider callingm_defaultSinkInter->disconnect();or explicitly disconnecting theSoundModel-related connections as well. - The mute-on-zero-volume check uses
qFuzzyCompare(0.0, value), which is slightly unconventional for testing a value against zero; usingqFuzzyIsNull(value)or an explicitvalue <= epsilonwould be clearer and avoid relying onqFuzzyCompare's symmetric-comparison semantics.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `onDefaultSinkChanged`, `m_defaultSinkInter->disconnect(this);` only disconnects slots on `this`, but the previous `m_defaultSinkInter` is also connected to `SoundModel::ref()`; if the intent is to avoid duplicate signals on subsequent connections, consider calling `m_defaultSinkInter->disconnect();` or explicitly disconnecting the `SoundModel`-related connections as well.
- The mute-on-zero-volume check uses `qFuzzyCompare(0.0, value)`, which is slightly unconventional for testing a value against zero; using `qFuzzyIsNull(value)` or an explicit `value <= epsilon` would be clearer and avoid relying on `qFuzzyCompare`'s symmetric-comparison semantics.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Added proper disconnection of previous DBus sink interface before creating new one to prevent signal conflicts 2. Fixed device activation timing issue by manually refreshing device list before setting active port 3. Changed mute handling logic to automatically set mute when volume reaches 0 4. Removed automatic unmute on volume change to prevent inconsistent state Log: Fixed audio device switching issues and improved mute/volume synchronization Influence: 1. Test audio device switching between different output devices 2. Verify mute state synchronization when volume is set to 0 3. Test volume adjustment while muted/unmuted 4. Check for any signal conflicts during device switching 5. Verify device list refresh works correctly on specific hardware models fix: 修复音频设备切换和静音同步问题 1. 在创建新的DBus接收器接口前正确断开之前的连接,防止信号冲突 2. 通过手动刷新设备列表再设置活动端口,修复设备激活时序问题 3. 修改静音处理逻辑,当音量达到0时自动设置静音 4. 移除音量变化时的自动取消静音,防止状态不一致 Log: 修复音频设备切换问题并改进静音/音量同步 Influence: 1. 测试不同输出设备之间的音频设备切换 2. 验证音量设为0时的静音状态同步 3. 测试静音/取消静音状态下的音量调整 4. 检查设备切换过程中是否有信号冲突 5. 验证特定硬件型号上的设备列表刷新功能 PMS: BUG-350057
deepin pr auto review这段代码修改主要针对音频设备切换时的时序问题和信号处理进行了优化。以下是对代码的详细审查和改进建议: 1. 语法逻辑审查优点:
问题:
2. 代码质量审查优点:
改进建议:
3. 代码性能审查优点:
改进建议:
4. 代码安全审查问题:
综合改进后的代码示例:void SoundController::onDefaultSinkChanged(const QDBusObjectPath &path)
{
if (path.path().isEmpty() || !m_audioInter) {
qWarning() << "Invalid input parameters";
return;
}
// 防抖动处理
static QTimer debounceTimer;
if (debounceTimer.isActive()) {
return;
}
debounceTimer.start(300);
if (m_defaultSinkInter) {
m_defaultSinkInter->disconnect(this);
m_defaultSinkInter->deleteLater();
}
m_defaultSinkInter = new DBusSink("org.deepin.dde.Audio1", path.path(), QDBusConnection::sessionBus(), this);
auto &soundModel = SoundModel::ref();
// 事务处理状态更新
soundModel.beginUpdate();
soundModel.setCardsInfo(m_audioInter->cardsWithoutUnavailable());
soundModel.setActivePort(m_defaultSinkInter->card(), m_defaultSinkInter->activePort().name);
soundModel.setMute(m_defaultSinkInter->mute());
soundModel.setVolume(existActiveOutputDevice() ? m_defaultSinkInter->volume() : 0);
soundModel.endUpdate();
// 使用Qt::QueuedConnection避免阻塞
connect(m_defaultSinkInter, &DBusSink::MuteChanged, &soundModel, [this] (bool value) {
Q_UNUSED(value)
if (m_defaultSinkInter) {
soundModel.setMute(m_defaultSinkInter->mute());
soundModel.setVolume(m_defaultSinkInter->volume());
}
}, Qt::QueuedConnection);
connect(m_defaultSinkInter, &DBusSink::VolumeChanged, &soundModel, [this] (double value) {
if (m_defaultSinkInter) {
soundModel.setVolume(value);
}
}, Qt::QueuedConnection);
connect(m_defaultSinkInter, &DBusSink::ActivePortChanged, this, [this](AudioPort port) {
if (m_defaultSinkInter) {
soundModel.setActivePort(m_defaultSinkInter->card(), port.name);
}
}, Qt::QueuedConnection);
}这些改进可以显著提高代码的健壮性、可维护性和性能,同时解决潜在的时序问题和安全问题。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, robertkill 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 |
Log: Fixed audio device switching issues and improved mute/volume synchronization
Influence:
fix: 修复音频设备切换和静音同步问题
Log: 修复音频设备切换问题并改进静音/音量同步
Influence:
PMS: BUG-350057
Summary by Sourcery
Improve audio output device switching reliability and mute/volume state synchronization in the dock sound controller.
Bug Fixes:
Enhancements: