fix: 解决使用音量快捷键设置时静音状态异常的问题

[fly602]

SetMute中判断了音量是否为0，但是Volume是根据事件刷新的，此时还不是设置后的音量，因此会影响判断

Log:
PMS: BUG-351645
Influence: audio mute settings

Summary by Sourcery

Fix incorrect mute state handling when adjusting volume via shortcuts by aligning mute logic with updated volume and configuration state.

Bug Fixes:

• Ensure unmuting on volume increase no longer relies on stale volume state, preventing incorrect persistent mute status.

Enhancements:

• Update mute handling to synchronize configuration mute flags when unmuting and add logging on mute update failures.

[deepin-ci-robot]

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

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sourcery-ai]

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts Sink.SetVolume’s unmute path to call the internal mute setter, handle errors, and synchronize the global mute output config when unmuting, fixing incorrect mute state when using volume hotkeys.

Sequence diagram for updated Sink.SetVolume unmute behavior

sequenceDiagram
    participant App as App
    participant Sink as Sink
    participant Mute as setMute
    participant Config as ConfigKeeper
    participant Log as logger

    App->>Sink: SetVolume(value > 0, isPlay)
    alt value <= 0.001
        Sink->>Sink: value = 0.001
        Sink->>Mute: setMute(true)
    else value > 0.001
        Sink->>Mute: setMute(false)
        alt setMute succeeds
            Sink->>Config: GetConfigKeeper()
            Config-->>Sink: keeper
            Sink->>Config: read keeper.Mute.MuteOutput
            alt keeper.Mute.MuteOutput is true
                Sink->>Config: SetMuteOutput(false)
            end
        else setMute returns error
            Sink->>Log: Warning(err)
        end
    end
    Sink->>Sink: update s.cVolume and properties
    Sink-->>App: *dbus.Error (nil on success)

Loading

Class diagram for Sink and ConfigKeeper changes

classDiagram
    class Sink {
        -PropsMu
        -cVolume
        SetVolume(value float64, isPlay bool) *dbus.Error
        -setMute(mute bool) error
    }

    class ConfigKeeper {
        -MuteConfig MuteConfig
        +Mute MuteConfig
        +SetMuteOutput(muted bool) error
    }

    class MuteConfig {
        +MuteOutput bool
    }

    Sink ..> ConfigKeeper : uses
    ConfigKeeper o-- MuteConfig : has

Loading

File-Level Changes

Change	Details	Files
Update the non-zero volume path in SetVolume to use setMute with error handling and to reset global mute output config when unmuting succeeds.	Replace the previous direct unmute call with a call to the internal setMute(false) function so that its volume-based logic is respected. Add a check on the result of setMute(false); only if it succeeds, clear the MuteOutput flag in the configuration when it is set. Log a warning if setMute(false) fails instead of silently ignoring errors.	audio1/sink.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

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[deepin-ci-robot]

deepin pr auto review

这段代码修改主要是为了解决在设置音量时，静音状态更新与音量属性更新不同步的问题。以下是对这段代码的详细审查和改进建议：

1. 代码逻辑审查

优点：

• 正确识别了原代码中 SetMute 和 Volume 更新之间的竞态条件问题
• 添加了配置同步逻辑，确保 MuteOutput 配置与实际静音状态一致
• 增加了错误处理，避免静音设置失败时程序崩溃

问题点：

• 错误处理不够完善，只记录了日志但没有进一步处理
• 没有考虑到 setMute 失败时是否应该继续执行后续操作
• 日志信息不够详细，不利于问题排查

2. 代码质量改进建议

func (s *Sink) SetVolume(value float64, isPlay bool) *dbus.Error {
    if value < 0.001 {
        value = 0.001
        if err := s.setMute(true); err != nil {
            logger.Warningf("Failed to set mute to true: %v", err)
            // 根据业务需求决定是否返回错误
            // return dbus.NewError("org.example.Error", []string{err.Error()})
        }
    } else {
        // 明确说明取消静音的操作
        if err := s.setMute(false); err != nil {
            logger.Warningf("Failed to set mute to false: %v", err)
            // 考虑是否需要继续执行
        } else {
            // 只有成功取消静音时才更新配置
            if GetConfigKeeper().Mute.MuteOutput {
                if err := GetConfigKeeper().SetMuteOutput(false); err != nil {
                    logger.Warningf("Failed to update MuteOutput config: %v", err)
                }
            }
        }
    }
    
    s.PropsMu.Lock()
    cv := s.cVolume.SetAvg(value)
    // ... 其余代码
}

3. 代码性能建议

1. 减少配置检查频率：如果 GetConfigKeeper().Mute.MuteOutput 是频繁访问的配置，可以考虑缓存其值
2. 锁的粒度：PropsMu 锁的范围可能需要调整，确保在调用 setMute 时不会造成死锁
3. 异步处理：如果配置更新不是关键路径，可以考虑异步处理

4. 代码安全建议

1. 输入验证：

   if value < 0 || value > 1.0 {
       return dbus.NewError("org.example.InvalidVolume", []string{"volume must be between 0 and 1.0"})
   }

2. 并发安全：

   • 确保 setMute 和配置更新操作是线程安全的
   • 考虑使用 sync.RWMutex 替代 sync.Mutex 如果读多写少

3. 错误处理：

   • 对于关键操作失败，应该返回错误给调用者
   • 考虑实现重试机制或降级处理

4. 配置一致性：

   • 添加配置校验逻辑，确保内存中的状态与持久化配置一致
   • 考虑实现配置回滚机制

5. 其他建议

1. 添加单元测试：

   func TestSinkSetVolume(t *testing.T) {
       tests := []struct {
           name     string
           volume   float64
           isPlay   bool
           wantErr  bool
       }{
           {"normal volume", 0.5, true, false},
           {"zero volume", 0.0, true, false},
           {"negative volume", -0.1, true, true},
       }
       // 测试实现...
   }

2. 添加文档注释：

   // SetVolume 设置音频输出设备的音量
   // 参数:
   //   - value: 音量值，范围 [0.0, 1.0]
   //   - isPlay: 是否正在播放
   // 返回:
   //   - *dbus.Error: 操作失败时返回错误，成功时返回 nil
   func (s *Sink) SetVolume(value float64, isPlay bool) *dbus.Error {
       // 实现...
   }

3. 日志改进：

   • 使用结构化日志
   • 添加上下文信息，如设备ID、当前音量等

4. 监控指标：

   • 添加音量设置成功的监控指标
   • 记录静音状态变更的频率

这些改进可以使代码更加健壮、可维护和可调试。

[sourcery-ai]

Hey - I've left some high level feedback:

• In the new else branch, logger.Warning(err) references err outside the short if err := s.setMute(false); scope, which will not compile; declare err separately before the if or log it inside the if block.
• You call GetConfigKeeper() twice in the same success path; consider storing the result in a local variable to avoid repeated lookups and make the intent clearer when updating MuteOutput.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In the new `else` branch, `logger.Warning(err)` references `err` outside the short `if err := s.setMute(false);` scope, which will not compile; declare `err` separately before the `if` or log it inside the `if` block.
- You call `GetConfigKeeper()` twice in the same success path; consider storing the result in a local variable to avoid repeated lookups and make the intent clearer when updating `MuteOutput`.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-bot]

TAG Bot

New tag: 6.1.77
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1048