Skip to content

fix(audio): 设置端口时, 调整端口的优先级#1051

Open
fly602 wants to merge 1 commit intolinuxdeepin:masterfrom
fly602:fix-audio
Open

fix(audio): 设置端口时, 调整端口的优先级#1051
fly602 wants to merge 1 commit intolinuxdeepin:masterfrom
fly602:fix-audio

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Mar 7, 2026

之前做音频端口优先级优化时, 忽略了产品需要, 要求将设置的端口放到端口类型优先级队首.

Log: 音频端口优先级策略调整
PMS: BUG-351629
Influence: audio priority

Summary by Sourcery

Adjust audio port priority handling so user-selected ports are treated as highest priority and persisted as per-card defaults.

New Features:

  • Persist per-card default input and output ports in configuration and reuse them when refreshing priorities.

Bug Fixes:

  • Ensure manually selected audio ports move to the front of the priority list regardless of port type and remain preferred on subsequent refreshes.
  • Avoid selecting unavailable or removed ports by pruning non-existent ports from priority lists when cards/ports change.

Enhancements:

  • Simplify priority policy APIs by removing explicit available-port tracking and relying on cleaned priority lists.
  • Reorder and arrange card ports by PulseAudio priority and configured defaults before inserting them into priority policies.

@deepin-ci-robot
Copy link

[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.

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

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 7, 2026

Reviewer's Guide

Adjusts audio port priority behavior so that user-selected ports move to the front of their type queue, introduces per-card default input/output ports in config, refactors priority iteration to no longer depend on dynamic availability lists, and adds logic to sort/rearrange card ports based on priority and stored defaults while cleaning up removed ports.

Sequence diagram for setting default audio port priority

sequenceDiagram
    actor User
    participant Audio
    participant PriorityManager
    participant PriorityPolicy
    participant ConfigKeeper

    User->>Audio: selectPort(cardName, portName, direction)
    Audio->>PriorityManager: SetTheFirstPort(cardName, portName, direction)

    alt direction is sink
        PriorityManager->>PriorityPolicy: Output.SetTheFirstPort(cardName, portName)
    else direction is source
        PriorityManager->>PriorityPolicy: Input.SetTheFirstPort(cardName, portName)
    end

    PriorityPolicy->>PriorityPolicy: FindPort(cardName, portName)
    PriorityPolicy-->>PriorityManager: bool success

    alt port found
        PriorityPolicy->>PriorityPolicy: reorder Types and Ports
        PriorityManager->>ConfigKeeper: SetDefaultPort(cardName, portName, direction)
        ConfigKeeper->>ConfigKeeper: update CardConfig.DefaultOutputPort or DefaultInputPort
        ConfigKeeper->>ConfigKeeper: Save()
        PriorityManager->>PriorityManager: Print()
        PriorityManager->>PriorityManager: Save()
    else port not found
        PriorityPolicy-->>PriorityManager: false
    end
Loading

Updated class diagram for audio priority and config models

classDiagram
    class PriorityManager {
        -string file
        +PriorityPolicy Output
        +PriorityPolicy Input
        +NewPriorityManager(path string) PriorityManager
        +Init(cards CardList) void
        +refreshPorts(cards CardList) void
        +removeNonExistentPorts(policy PriorityPolicy, currentPorts map[string]map[string]bool) void
        +arrangeCardPorts(card Card) void
        +moveCardPortToFirst(ports pulse.CardPortInfos, portName string) void
        +GetTheFirstPort(direction int) PriorityPort, Position
        +SetTheFirstPort(cardName string, portName string, direction int) void
        +LoopAvaiablePort(direction int, pos Position) PriorityPort, Position
        +Print() void
        +Save() void
    }

    class PriorityPolicy {
        +map[int][]PriorityPort Ports
        +PriorityTypeList Types
        +InsertPort(card pulse.Card, port pulse.CardPortInfo) void
        +insertByPriority(newPort PriorityPort, tp int) void
        +GetTheFirstPort() PriorityPort, Position
        +GetNextPort(pos Position) PriorityPort, Position
        +SetTheFirstPort(cardName string, portName string) bool
        +FindPort(cardName string, portName string) Position
    }

    class PriorityPort {
        +string CardName
        +string PortName
        +int PortType
        +uint32 Priority
    }

    class Position {
        +int tp
        +int index
    }

    class ConfigKeeper {
        -sync.Mutex mu
        +map[string]*CardConfig Cards
        +SetMuteAll(mute bool) void
        +SetDefaultPort(cardName string, portName string, direction int) void
        +GetDefaultPort(cardName string, direction int) string
        +Save() void
    }

    class CardConfig {
        +string Name
        +map[string]*PortConfig Ports
        +string DefaultOutputPort
        +string DefaultInputPort
        +UpdatePortConfig(portConfig PortConfig) void
    }

    class Card {
        +pulse.Card core
        +pulse.CardPortInfos Ports
        +update(card pulse.Card) void
    }

    class pulse.Card
    class pulse.CardPortInfo {
        +string Name
        +int Direction
        +uint32 Priority
        +int Available
    }

    class GetConfigKeeper

    PriorityManager --> PriorityPolicy : Output
    PriorityManager --> PriorityPolicy : Input
    PriorityManager --> Card : uses in refreshPorts
    PriorityManager --> ConfigKeeper : uses via GetConfigKeeper
    PriorityPolicy --> PriorityPort : manages
    PriorityPolicy --> Position : returns
    PriorityPolicy --> pulse.Card : uses
    PriorityPolicy --> pulse.CardPortInfo : uses
    ConfigKeeper --> CardConfig : owns
    Card --> pulse.Card : wraps
    Card --> pulse.CardPortInfo : Ports
    GetConfigKeeper --> ConfigKeeper : returns
Loading

File-Level Changes

Change Details Files
Refactor PriorityManager to manage ports without an external availability map and to keep policies in sync with current PulseAudio cards.
  • Remove portList and availablePort tracking from PriorityManager and the associated helper methods.
  • Change refreshPorts to pre-arrange each card’s ports, collect new sink/source ports, insert only new ports (in reverse order to preserve priority), and then prune non-existent ports from policies.
  • Add helper methods removeNonExistentPorts, arrangeCardPorts, and moveCardPortToFirst to sort ports by priority, move default ports to the front, and drop ports/cards that no longer exist.
  • Update GetTheFirstPort, SetTheFirstPort, and LoopAvaiablePort to work without an availability map and to log/print only via PriorityPolicy and ConfigKeeper.
audio1/priority_manager.go
Change PriorityPolicy semantics so that insertion and selection no longer depend on availability, and ensure newly added ports and user-selected ports are moved to the front of their type and type list.
  • Modify InsertPort to always prepend new ports to their type queue instead of inserting by numerical priority.
  • Simplify GetTheFirstPort and GetNextPort to iterate purely over the type and port lists without checking availability.
  • Refactor SetTheFirstPort to locate the target port, move its type to the front of pp.Types, and move the port to the head of its type list, returning false only when the port does not exist.
audio1/priority_policy.go
Persist and use per-card default output/input ports so manual selection affects future priority ordering.
  • Extend CardConfig with DefaultOutputPort and DefaultInputPort fields and initialize them in NewCardConfig.
  • Add SetDefaultPort and GetDefaultPort methods to ConfigKeeper, keyed by card and direction (sink/source).
  • Update PriorityManager.SetTheFirstPort to call ConfigKeeper.SetDefaultPort so user selections are saved.
  • Adapt existing config_keeper tests to the new CardConfig JSON schema, including default port fields.
audio1/config_keeper.go
audio1/config_keeper_test.go
audio1/priority_manager.go
Update tests to reflect the new SetTheFirstPort and GetTheFirstPort behavior and removal of availability-based logic.
  • Remove use of portList and availability filtering in priority tests and adjust expectations so that operations succeed even if the chosen port is already first.
  • Rename and reword test cases to match the new semantics of always moving the specified port to the first position in its type and type order, and keeping ordering stable otherwise.
audio1/priority_test.go
Remove now-unneeded port sorting and debug printing from Card and Audio initialization.
  • Delete Card.sortPortsByPriority invocation in Card.update, since port ordering is now handled by PriorityManager.arrangeCardPorts.
  • Remove an extra PriorityManager.Print call from audio initialization to reduce noise.
audio1/card.go
audio1/audio.go

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 found 1 issue, and left some high level feedback:

  • ConfigKeeper.SetDefaultPort returns early if the card config does not already exist, which means the first manual selection for a new card will be silently ignored; consider creating a CardConfig on demand (mirroring NewCardConfig) instead of just logging a warning.
  • GetTheFirstPort/GetNextPort in PriorityPolicy no longer take availability into account and instead rely on refreshPorts filtering; if port availability can change without a full refresh, you may want to keep a lightweight availability check here or ensure refreshPorts is always triggered on availability events.
  • InsertPort now always prepends the port and no longer uses insertByPriority, but insertByPriority is still present; consider either removing insertByPriority or documenting clearly why it remains unused to avoid confusion about which priority mechanism is authoritative.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- ConfigKeeper.SetDefaultPort returns early if the card config does not already exist, which means the first manual selection for a new card will be silently ignored; consider creating a CardConfig on demand (mirroring NewCardConfig) instead of just logging a warning.
- GetTheFirstPort/GetNextPort in PriorityPolicy no longer take availability into account and instead rely on refreshPorts filtering; if port availability can change without a full refresh, you may want to keep a lightweight availability check here or ensure refreshPorts is always triggered on availability events.
- InsertPort now always prepends the port and no longer uses insertByPriority, but insertByPriority is still present; consider either removing insertByPriority or documenting clearly why it remains unused to avoid confusion about which priority mechanism is authoritative.

## Individual Comments

### Comment 1
<location path="audio1/priority_policy.go" line_range="242-246" />
<code_context>
 }

-func (pp *PriorityPolicy) GetNextPort(available portList, pos *Position) (*PriorityPort, *Position) {
+func (pp *PriorityPolicy) GetNextPort(pos *Position) (*PriorityPort, *Position) {
 	if pos.tp == PortTypeInvalid {
 		return nil, &Position{tp: PortTypeInvalid, index: -1}
 	}
 	for tp := pos.tp; tp < PortTypeCount; tp++ {
-		for i := pos.index + 1; i < len(pp.Ports[tp]); i++ {
+		startIndex := 0
</code_context>
<issue_to_address>
**issue (bug_risk):** GetNextPort ignores pp.Types ordering, which makes SetTheFirstPort’s type reordering ineffective for iteration

Since SetTheFirstPort mutates pp.Types to express type priority, GetNextPort should honor that same ordering. Right now it iterates numerically from `pos.tp` to `PortTypeCount`, ignoring pp.Types, so the cycle order no longer matches the priority you just established and may confuse users. Please update GetNextPort to iterate over pp.Types (starting from the current type and wrapping) or otherwise align its traversal with the ordering defined by SetTheFirstPort.
</issue_to_address>

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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 调整音频端口优先级策略,使“用户手动选择的端口”能被提升为最高优先级,并将每张声卡的默认输入/输出端口持久化,以便在端口列表刷新时复用这些偏好。

Changes:

  • 移除“availablePort”显式可用端口跟踪,改为在刷新时清理不存在/不可用/禁用端口,并让优先级遍历直接基于清理后的列表。
  • 新增按声卡维度持久化默认输入/输出端口(写入配置),并在刷新端口时按 PulseAudio priority + 默认端口对声卡端口做排序/调整。
  • 调整 SetTheFirstPort 行为:将指定端口(以及其类型)提升到优先级队首;同步更新相关单测与初始化调试输出。

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
audio1/priority_policy.go 简化首选/遍历接口参数;调整插入与“置顶端口”逻辑
audio1/priority_manager.go 重写刷新逻辑:排序声卡端口、插入新端口、清理不存在端口;设置默认端口持久化
audio1/config_keeper.go CardConfig 增加默认输入/输出端口字段,并提供读写接口
audio1/config_keeper_test.go 更新 Save 序列化断言以包含新增字段
audio1/priority_test.go 更新 SetTheFirstPort/GetTheFirstPort 测试以适配新签名与语义
audio1/card.go 移除 update() 内的端口优先级排序调用
audio1/audio.go 初始化时移除一次额外的优先级打印

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fly602 fly602 force-pushed the fix-audio branch 2 times, most recently from cfff0cc to 649a40a Compare March 7, 2026 09:00
之前做音频端口优先级优化时, 忽略了产品需要, 要求将设置的端口放到端口类型优先级队首.

Log: 音频端口优先级策略调整
PMS: BUG-351629
Influence: audio priority
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见

总体评价

这段代码主要对音频端口优先级管理进行了重构,改进了端口排序、默认端口处理和可用性检查机制。整体代码结构清晰,但在一些细节上还有优化空间。

详细审查意见

1. 语法和逻辑问题

1.1 priority_manager.go 中的端口排序逻辑

// 将默认端口移到最前面(倒序插入时会最后插入,获得最高优先级)
if defaultOutputPort != "" {
    pm.moveCardPortToFirst(card.Ports, defaultOutputPort)
}

问题:注释说明"倒序插入时会最后插入,获得最高优先级",但实际代码是将端口移到最前面,逻辑与注释不一致。

建议:修改注释以匹配实际逻辑,或者调整代码实现以匹配注释意图。

1.2 config_keeper.go 中的默认端口初始化

func NewCardConfig(name string) *CardConfig {
    return &CardConfig{
        Name:              name,
        Ports:             make(map[string]*PortConfig),
        DefaultOutputPort: "",
        DefaultInputPort:  "",
    }
}

问题:Go语言中字符串的零值就是空字符串"",显式初始化为""是多余的。

建议:可以简化为:

func NewCardConfig(name string) *CardConfig {
    return &CardConfig{
        Name:  name,
        Ports: make(map[string]*PortConfig),
    }
}

2. 代码质量问题

2.1 priority_manager.go 中的重复代码

// 检查是否为新端口
var policy *PriorityPolicy
if port.Direction == pulse.DirectionSink {
    policy = pm.Output
} else {
    policy = pm.Input
}

问题:这段代码在循环中出现多次,且类似的逻辑在多处重复。

建议:可以提取为辅助方法:

func (pm *PriorityManager) getPolicyByDirection(direction int) *PriorityPolicy {
    if direction == pulse.DirectionSink {
        return pm.Output
    }
    return pm.Input
}

2.2 priority_policy.go 中的类型转换

newTypes := make(PriorityTypeList, 0, len(pp.Types))

问题:这里直接使用了类型别名PriorityTypeList,但在其他地方使用的是[]int

建议:保持一致性,统一使用PriorityTypeList[]int

3. 性能问题

3.1 priority_manager.go 中的端口移动操作

func (pm *PriorityManager) moveCardPortToFirst(ports pulse.CardPortInfos, portName string) {
    // ...
    // 将前面的端口向后移动
    copy(ports[1:portIndex+1], ports[0:portIndex])
    // 插入到最前面
    ports[0] = port
}

问题:每次移动端口都需要进行大量内存拷贝操作,在端口数量较多时可能影响性能。

建议:考虑使用链表结构或索引映射来优化频繁的移动操作。

3.2 priority_manager.go 中的嵌套循环

for _, card := range cards {
    // ...
    for _, port := range card.Ports {
        // ...
        for tp := range policy.Ports {
            // ...
        }
    }
}

问题:三层嵌套循环可能导致性能问题,特别是在处理大量音频设备时。

建议:考虑优化数据结构,减少循环嵌套层数。

4. 安全问题

4.1 config_keeper.go 中的并发访问

func (ck *ConfigKeeper) SetDefaultPort(cardName string, portName string, direction int) {
    ck.mu.Lock()
    defer ck.mu.Unlock()
    // ...
}

问题:虽然使用了互斥锁保护,但在GetDefaultPort中也使用了相同的锁,可能导致死锁风险。

建议:考虑使用读写锁(sync.RWMutex)来优化并发访问,特别是对于读多写少的场景。

4.2 priority_manager.go 中的指针操作

func (pm *PriorityManager) arrangeCardPorts(card *Card) {
    if card == nil || len(card.Ports) == 0 {
        return
    }
    // ...
}

问题:虽然检查了card是否为nil,但没有检查card.Ports是否为nil。

建议:添加更严格的空指针检查:

func (pm *PriorityManager) arrangeCardPorts(card *Card) {
    if card == nil || card.Ports == nil || len(card.Ports) == 0 {
        return
    }
    // ...
}

5. 其他建议

  1. 日志记录:在关键操作处添加更详细的日志记录,便于问题排查。

  2. 错误处理:部分函数返回bool表示成功/失败,建议使用error类型提供更详细的错误信息。

  3. 单元测试:虽然已有测试代码,但建议增加边界条件和异常情况的测试用例。

  4. 文档注释:为公共函数添加更详细的文档注释,说明函数的用途、参数和返回值。

  5. 常量定义:将魔法数字(如pulse.DirectionSink)定义为常量,提高代码可读性。

总结

这段代码整体结构良好,但在性能优化、并发安全和错误处理方面还有改进空间。建议重点关注:

  1. 优化端口移动操作的性能
  2. 加强并发访问的安全性
  3. 完善错误处理和日志记录
  4. 统一代码风格和命名规范

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.

3 participants