fix(audio): honor manual output selection#1146
Merged
Merged
Conversation
Reviewer's GuideImplements tracking and completion of pending manual audio port changes across card profile switches, ensuring manual output selections are honored even when automatic port switching is disabled, by adding state for pending manual switches and routing card-ready event handling through a new completion helper. Sequence diagram for manual port switch across card profile changesequenceDiagram
actor User
participant Audio
participant Card
User ->> Audio: setPort(cardId, portName, direction, auto=false)
Audio ->> Card: ActiveProfile
alt profile matches
Audio ->> Audio: setPortDirectly(cardId, portName, direction)
Audio ->> Audio: clearPendingManualPort(cardId, portName, direction)
else profile differs
Audio ->> Audio: setPendingManualPort(cardId, portName, direction)
Audio ->> Card: SetProfile(profile)
end
Note over Card,Audio: After profile switch and card becomes ready
Card ->> Audio: handleCardEvent/handleSinkAdded(...)
Audio ->> Audio: checkCardIsReady(cardId)
Audio ->> Audio: switchPortAfterCardReady(cardId)
alt pending manual switch exists
Audio ->> Audio: completePendingManualPort(cardId)
Audio ->> Audio: setPortDirectly(cardId, portName, direction)
else no pending manual switch
Audio ->> Audio: autoSwitchPort()
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
Audiostruct only tracks a singlependingManualPort, so concurrent/manual switches on different cards will overwrite each other; consider storing pending switches keyed bycardIdto avoid losing earlier requests when multiple cards are involved. - In
completePendingManualPort, there is a gap between unlocking and later relockinga.muwherependingManualPortcan be updated by another goroutine; although you guard the final clear with a pointer comparison, it may be clearer and less error‑prone to capture the values you need under the lock and have a separate method to perform the switch without touching shared state.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `Audio` struct only tracks a single `pendingManualPort`, so concurrent/manual switches on different cards will overwrite each other; consider storing pending switches keyed by `cardId` to avoid losing earlier requests when multiple cards are involved.
- In `completePendingManualPort`, there is a gap between unlocking and later relocking `a.mu` where `pendingManualPort` can be updated by another goroutine; although you guard the final clear with a pointer comparison, it may be clearer and less error‑prone to capture the values you need under the lock and have a separate method to perform the switch without touching shared state.
## Individual Comments
### Comment 1
<location path="audio1/audio.go" line_range="1071-1081" />
<code_context>
+ }
+}
+
+func (a *Audio) clearPendingManualPort(cardId uint32, portName string, direction int) {
+ a.mu.Lock()
+ defer a.mu.Unlock()
+
+ if a.pendingManualPort != nil &&
+ a.pendingManualPort.cardId == cardId &&
+ a.pendingManualPort.portName == portName &&
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clearing the pending manual port only on full match may leave stale state around.
`clearPendingManualPort` only clears when `cardId`, `portName`, and `direction` all match. If a user quickly issues multiple manual switches on the same card with different ports/directions, an older `pendingManualPort` can remain and later be applied by `completePendingManualPort`, which only checks `cardId`. To avoid using stale choices, either clear by `cardId` only, or also match `portName`/`direction` in `completePendingManualPort`.
```suggestion
func (a *Audio) clearPendingManualPort(cardId uint32, portName string, direction int) {
a.mu.Lock()
defer a.mu.Unlock()
// Keep parameters in the signature for compatibility; ignore them for clearing logic.
_ = portName
_ = direction
if a.pendingManualPort != nil &&
a.pendingManualPort.cardId == cardId {
a.pendingManualPort = nil
}
}
```
</issue_to_address>
### Comment 2
<location path="audio1/audio.go" line_range="1083-1092" />
<code_context>
+ }
+}
+
+func (a *Audio) completePendingManualPort(cardId uint32) bool {
+ a.mu.Lock()
+ pending := a.pendingManualPort
+ if pending == nil || pending.cardId != cardId {
+ a.mu.Unlock()
+ return false
+ }
+ a.mu.Unlock()
+
+ err := a.setPortDirectly(pending.cardId, pending.portName, pending.direction)
+ if err != nil {
+ logger.Warning(err)
+ return false
+ }
+
+ a.mu.Lock()
+ if a.pendingManualPort == pending {
+ a.pendingManualPort = nil
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a copied pointer without re-validating the intended port/direction can apply a stale manual selection.
In `completePendingManualPort`, you snapshot `pending := a.pendingManualPort`, unlock, then call `setPortDirectly` with that snapshot. If another goroutine calls `setPendingManualPort` for the same `cardId` but a different `portName`/`direction` after the snapshot, this function will still apply the old selection, so the final state may not match the latest user choice. Consider either re-validating that `pending` still matches the current desired port/direction under the lock immediately before calling `setPortDirectly`, or attaching a monotonic sequence/timestamp to `pendingManualPort` and only acting on the newest entry.
</issue_to_address>
### Comment 3
<location path="audio1/audio_events.go" line_range="277-279" />
<code_context>
a.autoSwitchInputPort()
}
+func (a *Audio) switchPortAfterCardReady(cardId uint32) {
+ if !a.completePendingManualPort(cardId) {
+ a.autoSwitchPort()
+ }
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Falling back to auto-switch after a failed manual completion may override user intent.
In `switchPortAfterCardReady`, if `completePendingManualPort` returns false because `setPortDirectly` failed, we immediately call `a.autoSwitchPort()`. This turns a transient failure in applying the manual choice into an automatic override of explicit user intent. It would be better to distinguish "no pending manual for this card" from "pending exists but failed to apply" and, in the latter case, avoid `autoSwitchPort` (e.g., leave the current port or retry the manual selection).
Suggested implementation:
```golang
func (a *Audio) switchPortAfterCardReady(cardId uint32) {
handledPending, applied := a.completePendingManualPort(cardId)
// If there is no pending manual selection for this card, fall back to auto.
if !handledPending {
a.autoSwitchPort()
return
}
// If there *was* a pending manual selection but it failed to apply,
// do not auto-switch: keep the current port to respect user intent.
if !applied {
return
}
}
```
To support this change you will also need to:
1. Update the signature of `completePendingManualPort` to return two booleans, e.g.:
- `handledPending bool` – true if there was a pending manual selection for this `cardId` (regardless of success in applying it).
- `applied bool` – true if the pending manual selection was successfully applied.
Example: `func (a *Audio) completePendingManualPort(cardId uint32) (handledPending bool, applied bool)`.
2. Adjust the implementation of `completePendingManualPort` to:
- Return `(false, false)` when there is no pending manual port for the given `cardId`.
- Return `(true, true)` when a pending manual selection exists and `setPortDirectly` (or equivalent) succeeds.
- Return `(true, false)` when a pending manual selection exists but applying it fails.
3. Update all other call sites of `completePendingManualPort` in the codebase to handle the new `(bool, bool)` return values appropriately (most likely checking `handledPending` or both flags depending on context).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
mhduiy
force-pushed
the
sink
branch
2 times, most recently
from
2a250db to
6cd54af
Compare
Contributor
Author
|
/test all |
1. Track pending manual port switches when a profile change is required. 2. Complete manual output device selection after the card becomes ready. 3. Avoid losing manual port selection when automatic port switching is disabled. Log: Fix manual output device selection not taking effect when automatic port switching is disabled. fix(audio): 修复手动输出设备选择失效 1. 在需要切换配置文件时记录待处理的手动端口切换。 2. 在声卡就绪后继续完成手动输出设备选择。 3. 避免关闭自动端口切换后丢失手动端口选择。 Log: 修复关闭自动端口切换后设置输出设备不生效的问题。
deepin pr auto review★ 总体评分：95分（0-100之间的整数）■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 diff --git a/audio1/audio_events.go b/audio1/audio_events.go
index e5ee4fe37..c12345678 100644
--- a/audio1/audio_events.go
+++ b/audio1/audio_events.go
@@ -277,8 +277,9 @@ func (a *Audio) switchPortAfterCardReady(cardId uint32) {
if !handledPending {
a.autoSwitchPort()
return
}
if !applied {
logger.Warningf("pending manual switch for cardId=%d was not applied, fallback to auto switch", cardId)
+ a.autoSwitchPort()
return
}
}
diff --git a/audio1/audio.go b/audio1/audio.go
index d60ea98e3..d87654321 100644
--- a/audio1/audio.go
+++ b/audio1/audio.go
@@ -1083,6 +1083,14 @@ func (a *Audio) completePendingManualPort(cardId uint32) (handled bool, applied
if err != nil {
logger.Warningf("failed to complete pending manual port: cardId=%d, portName=%s, direction=%d, err=%v",
pending.cardId, pending.portName, pending.direction, err)
+
+ // 执行失败时清除残留状态，防止后续事件被永久阻塞
+ a.pendingManualPortMu.Lock()
+ if a.pendingManualPort != nil &&
+ a.pendingManualPort.cardId == pending.cardId &&
+ a.pendingManualPort.portName == pending.portName &&
+ a.pendingManualPort.direction == pending.direction {
+ a.pendingManualPort = nil
+ }
+ a.pendingManualPortMu.Unlock()
return true, false
} |
fly602
approved these changes
Jun 17, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602, mhduiy 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: Fix manual output device selection not taking effect when automatic port switching is disabled.
fix(audio): 修复手动输出设备选择失效
Log: 修复关闭自动端口切换后设置输出设备不生效的问题。
Summary by Sourcery
Ensure manual audio output selection is correctly applied even when profile changes are required and automatic port switching is disabled.
Bug Fixes: