Fix application volume sliders becoming non-functional after extended runtime#21
Fix application volume sliders becoming non-functional after extended runtime#21
Conversation
|
@cwage 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: cwage <190973+cwage@users.noreply.github.com>
Co-authored-by: cwage <190973+cwage@users.noreply.github.com>
|
@copilot-swe-agent The analysis looks perfect! Please go ahead and implement Option B: PulseAudio Event Subscription as you outlined. This is exactly the right approach:
Please proceed with the implementation. Thanks! |
Option B: PulseAudio Event Subscription has been successfully implemented! ✅ The implementation includes:
The fix ensures volume sliders remain functional indefinitely by dynamically updating their stored PulseAudio sink input indexes as applications come and go. Project builds successfully with no compilation errors. Ready for testing and review. Commit: e9ec9eb Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where application volume sliders become non-functional after extended runtime due to stale PulseAudio sink input indexes. The fix implements a PulseAudio event subscription system to dynamically update slider references when applications start/stop audio streams.
- Added PulseAudio event subscription system to detect sink input changes
- Implemented dynamic slider index updating mechanism with recursive widget traversal
- Added change tracking flag to efficiently handle updates only when needed
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/volmix.c | Added UI update mechanism with recursive widget traversal and timer callback enhancements |
| src/pulse_client.h | Added sink_inputs_changed flag and API function declaration |
| src/pulse_client.c | Implemented event subscription system and callback handlers |
| src/Makefile.in | Updated build configuration with DATADIR definition |
| src/Makefile | Updated generated makefile with build environment changes |
| // Refresh the sink input list | ||
| pulse_client_refresh_apps(&app->pulse_client); | ||
|
|
||
| // Process the refresh operation |
There was a problem hiding this comment.
The magic numbers MAX_REFRESH_TIMEOUT and DELAY_5_MS_USEC are used but not defined in this file. These constants should be defined at the top of the file or in a header to improve code maintainability.
| if ((t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) == PA_SUBSCRIPTION_EVENT_SINK_INPUT) { | ||
| // Mark that sink inputs have changed - this will trigger UI update | ||
| client->sink_inputs_changed = TRUE; | ||
| printf("Sink input event detected (index=%u, type=%s)\n", index, |
There was a problem hiding this comment.
Debug printf statements should be replaced with proper logging or removed from production code. Consider using a logging framework or conditional compilation for debug output.
| printf("Sink input event detected (index=%u, type=%s)\n", index, | |
| g_debug("Sink input event detected (index=%u, type=%s)", index, |
| // Check if sink inputs have changed and window is visible | ||
| if (pulse_client_sink_inputs_changed(&app->pulse_client) && | ||
| app->volmix_window && gtk_widget_get_visible(app->volmix_window)) { | ||
| printf("Sink inputs changed, updating slider indexes...\n"); |
There was a problem hiding this comment.
Debug printf statements should be replaced with proper logging or removed from production code. Consider using a logging framework or conditional compilation for debug output.
| printf("Sink inputs changed, updating slider indexes...\n"); | |
| g_debug("Sink inputs changed, updating slider indexes..."); |
| if (*stored_index != app->index) { | ||
| printf("Updating slider for '%s': index %u -> %u\n", | ||
| stored_app_name, *stored_index, app->index); |
There was a problem hiding this comment.
Debug printf statements should be replaced with proper logging or removed from production code. Consider using a logging framework or conditional compilation for debug output.
| if (*stored_index != app->index) { | |
| printf("Updating slider for '%s': index %u -> %u\n", | |
| stored_app_name, *stored_index, app->index); | |
| if (*stored_index != app->index) { | |
| #ifdef DEBUG | |
| printf("Updating slider for '%s': index %u -> %u\n", | |
| stored_app_name, *stored_index, app->index); | |
| #endif |
| PA_SUBSCRIPTION_MASK_SINK_INPUT, | ||
| NULL, NULL); | ||
| if (!client->operation) { | ||
| printf("Failed to subscribe to PulseAudio events\n"); |
There was a problem hiding this comment.
The error message is unclear about the consequences of the subscription failure. Consider providing more context about what this means for the application's functionality.
| printf("Failed to subscribe to PulseAudio events\n"); | |
| printf("Failed to subscribe to PulseAudio events. Audio application monitoring will be disabled and the client will not detect when applications start or stop audio playback.\n"); |
This PR fixes a bug where application volume sliders would stop working after volmix runs for an extended period. The sliders would still move visually, but wouldn't actually control application volumes.
Root Cause
The issue was caused by stale PulseAudio sink input indexes stored in slider widgets:
Solution
Implemented a PulseAudio event subscription system that dynamically updates slider references:
PA_SUBSCRIPTION_EVENT_SINK_INPUTevents to detect when applications start/stop audio streamssink_inputs_changedflag to efficiently track when updates are neededKey Changes
pulse_client.c: Added subscription setup and callback to handle PulseAudio eventspulse_client.h: Added change tracking flag and API functionvolmix.c: Added UI update mechanism with recursive widget traversalTesting
The fix ensures volume sliders remain functional indefinitely, even as PulseAudio recycles sink input indexes during normal operation.
Fixes #20.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
esm.ubuntu.com/usr/lib/apt/methods/https(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.