-
-
Notifications
You must be signed in to change notification settings - Fork 352
feat: Per program audio sliders #859
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
added an optional per program volume slider in the modules/bar/popouts/audio
onMoved function of program sliders were not pointing to parent and would result in a ReferenceError
removed trailing spaces and lines with only indentation
soramanew
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice PR!
modules/bar/popouts/Audio.qml
Outdated
| PwNodeLinkTracker { | ||
| id: linkTracker | ||
| node: Pipewire.defaultAudioSink | ||
| } | ||
| PwObjectTracker { | ||
| objects: [linkTracker.linkGroups] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this, add a streams property to the Audio service instead, then use that.
So in services/Audio.qml:
diff --git a/services/Audio.qml b/services/Audio.qml
index 71ccb86..a36d79c 100644
--- a/services/Audio.qml
+++ b/services/Audio.qml
@@ -14,7 +14,9 @@ Singleton {
property string previousSourceName: ""
readonly property var nodes: Pipewire.nodes.values.reduce((acc, node) => {
- if (!node.isStream) {
+ if (node.isStream) {
+ acc.streams.push(node);
+ } else {
if (node.isSink)
acc.sinks.push(node);
else if (node.audio)
@@ -22,10 +24,12 @@ Singleton {
}
return acc;
}, {
+ streams: [],
sources: [],
sinks: []
})
+ readonly property list<PwNode> streams: nodes.streams
readonly property list<PwNode> sinks: nodes.sinks
readonly property list<PwNode> sources: nodes.sources
modules/bar/popouts/Audio.qml
Outdated
| font.weight: 500 | ||
| } | ||
| Repeater { | ||
| model: linkTracker.linkGroups.filter(e => e.source.isStream) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Audio.streams
modules/bar/popouts/Audio.qml
Outdated
| if (modelData.source.audio.muted) | ||
| return "no_sound"; | ||
| if (modelData.source.audio.volume >= 0.5) | ||
| return "volume_up"; | ||
| if (modelData.source.audio.volume > 0) | ||
| return "volume_down"; | ||
| return "volume_mute"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Icons.getVolumeIcon
|
Oh nice PR, I was working on a volume mixer widget myself for the dashboard, but got sidetracked haha |
|
I am currently working on the Audio.streams. However when I try to use PwNodeLinkTracker on these nodes under the repeater with: linkTracker.linkGroups returns an empty array. I am assuming this has got something to do with whether the node is a sink or not. Which then means I can not filter by link group's state.
I didn't really consider any other forms of filtering tbh. I might try to think of another method that doesn't require the linkGroup |
|
Can you not just use the streams directly? Why do you need the link tracker? Also, I was thinking, for a full audio mixer, we might want to have it in a separate page in the control centre instead of having it in the popout |
…to control centre (faulty) currently has problems with heights of the components added an extra property to recording's prop value to track mixer accordion state added streams list to Audio service removed the audioPopup config values
Since the sliders were on the audio popup, I thought having too may of them on that popout wouldn't look good so I tried to filter them by their link states
I first tried to place these in the popout since that is where I launch pavucontrol from recording_20251030_22-08-03.mp4But I couldn't figure out how to handle the height of the accordion properly so it looks really bad right now Aside from what is visible on the video, to stop the accordion from taking too much space I have copied the same scroll bar from the recordingList. But this isn't ideal because this space now has a bunch of interactable sliders inside a scrollbar which fight clash for the control of the mouse |
I think soramanew meant as a separate page, as in switching context off from the various cards normally in that drawer, to one showing only the volume mixer, maybe with kind of like pagination or tabs like the dashboard, it's intuitively similar to other desktop environments and even Windows. I personally think vertical column on the left in the control centre for tabs could work, opens up room for organization and sections of different quick controls and so even if there's barely much in terms of new pages quite yet? |
|
That isn't actually the control centre, the control centre is the window that contains the Bluetooth settings |
Moved audio mixer to dashboard's media tab and it replaces the bongocat gif if enabled in the config Also added an optional error type to StyledSliders
Oh, is the control centre the window that opens after clicking the settings button in the quick toggles menu? After my last failed attempt, I gave up on messing with dynamic heights and accordions. And thought about @PixelKhaos's comment on placing it in the dashboard
I thought the space bongocat gif is taking could be used for a mixer and put it there if enabled by a new config option recording_20251102_00-50-03.mp4let me know what you guys think |
I have made a mistake in the if conditions, instead of checking if a node is a stream, I was checking if node is a stream and is a sink, resulting in a situation where a non-stream node would enter the else block
|
Maybe the waveform and cover section on the left would need to be a bit smaller to allow for more padding for the mixer, so spacing and all gets consistent. Atm it looks a bit crammed in there. But I do think the dashboard is a good place for a mixer to be fast to access, and does help with not worrying about dynamic height wrapping a scrollable area etc. In the control centre the mixer could be more granular, show more details and maybe grouping etc, but as a simple mixer widget version I think what ya got is pretty much it! |
Also added a Styled Select, however it needs further work done to it
I think I saw some config options around the dashboard spacing/sizes so I left that part untocuhed, at least for now
I also worked on a prototype for an audio tab for the control centre, which currently looks like this: recording_20251108_20-14-12.mp4I will try to move 'programs' part to be in it's own Card, and I want to do further work on the 'StyledSelect' component I have added. however, besides adding a main volume slider and adding a device selector (which are already present in the bar's audio popup) this page doesn't add any extra utility I looked at the functionalities pavucontrol has besides what I already have and it seems to be a couple things:
I am thinking of implementing the first point by having the slider 'blink', as in, change the opacity of the slider by reading the volumes property of PwNodeAudio object. So that currently audio producing sliders have a subtle animation to them.
What kind of grouping are you thinking about? Grouping sliders by which application they originate from? |
I have added a per program volume slider that can be set to only show active sources
This is my first time handling QML so please let me know about any problems, as I am sure I made many here. Especially on my use of PwObjectTracker,
Side note: This is far-fetched, but on my machine I am getting an error from LogindManager so for my local copy I disabled those lines out of laziness, however I kept them as-is in this PR. I do not think this would result in any breaking behavior but I guessed I should still mention that