Conversation
1. 优化了音频初始化配置设置 2. 优化了音频切换端口检查的逻辑 3. 优化了音频音量静音的设置 Log: 音频优化 PMS: BUG-350057 Influence: audio
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fly602 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 |
Reviewer's GuideRefactors audio initialization and auto-switch logic so that configuration is loaded earlier, port selection iterates robustly over available ports, input/output auto-switch checks are unified, and mute/volume handling on sinks is made more consistent and stateful. Sequence diagram for updated output auto-switch logicsequenceDiagram
participant Audio
participant PriorityManager
participant Cards
participant Ctx
participant PulseCard
Audio->>Audio: checkAutoSwitchOutputPort()
Audio->>Audio: canAutoSwitchPort()?
alt cannot auto switch
Audio-->>Audio: return false,0,""
else can auto switch
Audio->>Audio: read currentCardName,currentPortName from defaultSink
loop iterate available output ports
Audio->>PriorityManager: LoopAvaiablePort(DirectionSink,pos)
PriorityManager-->>Audio: prefer, pos
alt no more ports or invalid
Audio-->>Audio: break loop
else valid prefer
Audio->>Cards: getByName(prefer.CardName)
alt card not found
Cards-->>Audio: error
Audio-->>Audio: continue
else card found
Cards-->>Audio: card
Audio->>Ctx: GetCard(card.Id)
alt pulse card not found
Ctx-->>Audio: error
Audio-->>Audio: continue
else pulse card found
Ctx-->>Audio: pc
Audio->>PulseCard: Ports.Get(prefer.PortName,DirectionSink)
alt port not found
PulseCard-->>Audio: error
Audio-->>Audio: continue
else port found
PulseCard-->>Audio: port
Audio->>Audio: mode = ConfigKeeper.GetMode(card,prefer.PortName)
alt different from current sink
Audio-->>Audio: return true,card.Id,prefer.PortName
else same as current sink
Audio-->>Audio: return false,0,""
end
end
end
end
end
end
Audio-->>Audio: return true,0,"" %% exhausted ports without mismatch
end
Sequence diagram for updated input auto-switch logicsequenceDiagram
participant Audio
participant PriorityManager
participant Cards
participant Ctx
participant PulseCard
Audio->>Audio: checkAutoSwitchInputPort()
Audio->>Audio: canAutoSwitchPort()?
alt cannot auto switch
Audio-->>Audio: return false,0,""
else can auto switch
Audio->>Audio: read currentCardName,currentPortName from defaultSource
loop iterate available input ports
Audio->>PriorityManager: LoopAvaiablePort(DirectionSource,pos)
PriorityManager-->>Audio: prefer, pos
alt no more ports or invalid
Audio-->>Audio: break loop
else valid prefer
Audio->>Cards: getByName(prefer.CardName)
alt card not found
Cards-->>Audio: error
Audio-->>Audio: continue
else card found
Cards-->>Audio: card
Audio->>Ctx: GetCard(card.Id)
alt pulse card not found
Ctx-->>Audio: error
Audio-->>Audio: continue
else pulse card found
Ctx-->>Audio: pc
Audio->>PulseCard: Ports.Get(prefer.PortName,DirectionSource)
alt port not found
PulseCard-->>Audio: error
Audio-->>Audio: continue
else port found
PulseCard-->>Audio: port
Audio->>Audio: check card.ActiveProfile not nil
alt profile exists and port supports profile
Audio-->>Audio: compare currentCardName,currentPortName
alt different from current
Audio-->>Audio: return true,card.Id,prefer.PortName
else same as current
Audio-->>Audio: return false,0,""
end
else profile missing or unsupported
Audio-->>Audio: continue
end
end
end
end
end
end
Audio-->>Audio: return true,0,"" %% exhausted ports without mismatch
end
Sequence diagram for updated sink volume and mute handlingsequenceDiagram
participant Client
participant Sink
participant Audio
participant Ctx
participant ConfigKeeper
Client->>Sink: SetVolume(value,isPlay)
alt value == 0
Sink->>Sink: value = 0.001
Sink->>Sink: setMute(true)
else value > 0
Sink->>Sink: setMute(ConfigKeeper.Mute.MuteOutput)
end
Sink->>Sink: update channel volume (cVolume.SetAvg)
Sink->>Ctx: SetSinkVolumeByIndex
Client->>Sink: SetMute(value)
Sink->>Sink: setMute(value)
alt setMute returns error
Sink-->>Client: dbus.Error
else success
Sink->>ConfigKeeper: SetMuteOutput(value)
Sink-->>Client: ok
end
rect rgba(200,200,200,0.2)
note right of Sink: internal setMute(value)
Sink->>Sink: CheckPort()
alt error
Sink-->>Sink: return error
else ok
Sink->>Sink: setPropMute(value)
alt prop changed
Sink->>Ctx: SetSinkMuteByIndex
alt value == false
Sink->>Sink: playFeedback()
end
else no change
end
Sink-->>Sink: return nil
end
end
Updated class diagram for audio auto-switch and mute logicclassDiagram
class Audio {
+init() error
+autoSwitchPort()
+autoSwitchOutputPort() bool
+autoSwitchInputPort() bool
+checkAutoSwitchOutputPort() (auto bool, cardId uint32, portName string)
+checkAutoSwitchInputPort() (auto bool, cardId uint32, portName string)
+resumeSinkConfig(s *Sink)
+resumeSourceConfig(s *Source)
+updateDefaultSink(sinkName string)
+LoadNullSinkModule()
+moveSinkInputsToSink(s *Sink)
+refresh()
+canAutoSwitchPort() bool
+getCardNameById(id uint32) string
-defaultSink *Sink
-defaultSource *Source
-cards Cards
-ctx Context
-Mono bool
-PropsMu Mutex
-setPropCards(v string)
}
class PriorityManager {
+SetTheFirstPort(cardName string, portName string, direction int)
+GetTheFirstPort(direction int) (*PriorityPort, *Position)
+LoopAvaiablePort(direction int, pos *Position) (*PriorityPort, *Position)
-availablePort(mapKey string) bool
-Input PriorityPortList
-Output PriorityPortList
}
class Position {
+tp int
+index int
}
class PriorityPort {
+CardName string
+PortName string
+PortType int
}
class Sink {
+Name string
+index uint32
+SetVolume(value float64, isPlay bool) *dbus.Error
+SetMute(value bool) *dbus.Error
+CheckPort() error
+playFeedback()
-audio *Audio
-cVolume CVolume
-PropsMu Mutex
-setMute(value bool) error
-setPropMute(value bool) bool
}
class Source {
+Name string
}
class Cards {
+getByName(name string) (*Card, error)
+string() string
}
class Card {
+Id uint32
+ActiveProfile *Profile
+Ports PortCollection
-core CoreCard
}
class Profile {
+Name string
}
class PortCollection {
+Get(portName string, direction int) (*Port, error)
}
class Port {
+Profiles ProfileSet
}
class ProfileSet {
+Exists(name string) bool
}
class Context {
+GetCard(id uint32) (*PulseCard, error)
+GetDefaultSink() string
+GetDefaultSource() string
+SetDefaultSink(name string)
+SetDefaultSource(name string)
+SetSinkMuteByIndex(index uint32, value bool)
+SetSinkVolumeByIndex(index uint32, volume CVolume)
}
class PulseCard {
+Ports PortCollection
}
class ConfigKeeper {
+Load()
+GetMode(card *Card, portName string) string
+SetMuteOutput(value bool)
+Mute MuteConfig
}
class MuteConfig {
+MuteOutput bool
}
Audio --> PriorityManager : uses for port selection
Audio --> Cards : uses for logical cards
Audio --> Context : uses for PulseAudio context
Audio --> ConfigKeeper : uses for mode and mute
Audio "1" o-- "many" Sink : manages
Audio "1" o-- "many" Source : manages
PriorityManager --> PriorityPort : manages
PriorityManager --> Position : iteration state
Cards --> Card
Card --> Profile
Card --> PortCollection
PortCollection --> Port
Port --> ProfileSet
Sink --> Audio
Sink --> Context
Sink --> ConfigKeeper
Context --> PulseCard
ConfigKeeper --> MuteConfig
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto reviewGit Diff 代码审查报告总体评价这次提交主要对音频模块的初始化流程、自动切换端口逻辑以及静音处理进行了重构。代码整体结构有所改进，逻辑更加清晰，但存在一些潜在的性能和并发安全问题。 详细审查意见1. 语法与逻辑问题audio1/audio.go
audio1/audio_events.go
audio1/sink.go
2. 代码质量问题
3. 性能问题
4. 安全问题
改进建议
总结这次提交改进了音频模块的初始化流程和自动切换端口逻辑，代码结构更加清晰，错误处理更加完善。但需要注意性能优化、并发安全和错误处理的改进。建议在合并前进行充分的测试，特别是边界条件和并发场景。 |
![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 1 issue, and left some high level feedback:
- In
resumeSinkConfig, the mute setting now ignores!portConfig.Enabledand relies only onMuteOutput; if a port is disabled this may leave it unmuted—double‑check whether the disabled state should still influence mute when resuming sink configuration. - The new
LoopAvaiablePorthelper inPriorityManagerhas a spelling issue in its name (Avaiable), and it might be worth tightening its contract with callers (e.g., documenting or encapsulating thenil/PortTypeInvalidreturn behavior) to make the termination conditions in thecheckAutoSwitch*Portloops clearer. - Both
checkAutoSwitchOutputPortandcheckAutoSwitchInputPortnow loop over ports and repeatedly callGetCard/Ports.Get; if this runs frequently, consider caching or reducing the amount/level of debug logging (Debugf) inside the loop to avoid performance and log‑noise issues on systems with many ports.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `resumeSinkConfig`, the mute setting now ignores `!portConfig.Enabled` and relies only on `MuteOutput`; if a port is disabled this may leave it unmuted—double‑check whether the disabled state should still influence mute when resuming sink configuration.
- The new `LoopAvaiablePort` helper in `PriorityManager` has a spelling issue in its name (`Avaiable`), and it might be worth tightening its contract with callers (e.g., documenting or encapsulating the `nil`/`PortTypeInvalid` return behavior) to make the termination conditions in the `checkAutoSwitch*Port` loops clearer.
- Both `checkAutoSwitchOutputPort` and `checkAutoSwitchInputPort` now loop over ports and repeatedly call `GetCard`/`Ports.Get`; if this runs frequently, consider caching or reducing the amount/level of debug logging (`Debugf`) inside the loop to avoid performance and log‑noise issues on systems with many ports.
## Individual Comments
### Comment 1
<location> `audio1/sink.go:107-108` </location>
<code_context>
+ } else {
+ return false, 0, ""
}
- } else {
- return true, 0, ""
}
</code_context>
<issue_to_address>
**issue (bug_risk):** SetVolume now updates hardware mute without updating the stored mute config, which can desync state.
In the non-zero volume path, `SetVolume` now calls `s.setMute(GetConfigKeeper().Mute.MuteOutput)` directly instead of using `SetMute`, so `GetConfigKeeper().SetMuteOutput` is never invoked when mute changes via volume. This can leave `MuteOutput` out of sync with the actual sink mute and cause an incorrect mute state to be reapplied on resume. Please either route this through `SetMute` or explicitly update `MuteOutput` whenever `setMute` is called here.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+107
to
+108
| } else { | ||
| s.setMute(GetConfigKeeper().Mute.MuteOutput) |
There was a problem hiding this comment.
issue (bug_risk): SetVolume now updates hardware mute without updating the stored mute config, which can desync state.
In the non-zero volume path, SetVolume now calls s.setMute(GetConfigKeeper().Mute.MuteOutput) directly instead of using SetMute, so GetConfigKeeper().SetMuteOutput is never invoked when mute changes via volume. This can leave MuteOutput out of sync with the actual sink mute and cause an incorrect mute state to be reapplied on resume. Please either route this through SetMute or explicitly update MuteOutput whenever setMute is called here.
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
2 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: 音频优化
PMS: BUG-350057
Influence: audio
Summary by Sourcery
Refine audio initialization and automatic port switching behavior to better handle dynamic device availability and mute/volume settings.
Bug Fixes:
Enhancements: