feat(audio): implement volume change audio notifications - #558
romanstingler wants to merge 1 commit into
Conversation
85ef1f8 to
f3fe0aa
Compare
f3fe0aa to
f9448df
Compare
|
I was thinking that now that we were able to reduce the number of dependencies we could evaluate to use rodio. We should probably need to add some dependency in the package and nix config to be able to compile. So we could proceed by step. If this solution is acceptable let's merge and see, then if it's used and adding rodio is not a pain in the ass we can proceed adding a "real" audio support |
f9448df to
87a6d23
Compare
|
agree, we can add this one and if there is more demand we can just rework it. |
|
Does this still make sense? |
|
@MalpenZibo I think people still want this |
|
@romanstingler ok! let's try |
7bf32f0 to
072605f
Compare
|
@MalpenZibo ready to test |
072605f to
e5d2110
Compare
| if let Some(service) = self.service.as_mut() { | ||
| if let Some(value) = message.value() { | ||
| let _ = service.command(AudioCommand::SourceVolume(value)); | ||
| self.audio_feedback.play(value); |
There was a problem hiding this comment.
Feedback on source volume change seems unusual.
MalpenZibo
left a comment
There was a problem hiding this comment.
We still have some issues to address.
I think that with the current implementation, we keep the audio device out of suspend for the whole ashell session, and we don't have any recovery if PulseAudio restarts or is not available yet.
Anyway, could we pause this feature for now? I still need to decide how to handle this
Based on the discussion in #525 and the follow-up conversation.
After we agreed to move away from spawning external processes (
pw-cat), I first looked intorodiobut that would pull inlibasound2-devas an additional system dependency, which I wanted to avoid to keep the dependency footprint small.Instead I switched to
libpulse-simple-bindingwhich reuses the already requiredlibpulse.so, so no new system library is needed while still giving us "real" audio playback.What changed from the original approach:
assets/bell.pcm) played directly via PulseAudio's simple API.(I tried to synthesize this one but it is too complex and sounds really off otherwise).
audio_feedbackis configurable in[settings](default:true), and hot-reloadable.@MalpenZibo your call on the default value
Docs updated accordingly.
closes #525