From 32f456e93756bece1d05f5933ad627696ca14b98 Mon Sep 17 00:00:00 2001 From: dianjixz <18637716021@163.com> Date: Fri, 13 Nov 2026 17:15:49 +0800 Subject: [PATCH] fix(launcher): cache system volume so OSD and settings agree The volume shortcut re-read the mixer on every press while Settings -> Speaker rendered whatever its own asynchronous read returned, so the two could show different values for the same mixer state. Read the mixer once at launcher startup on a worker thread. The reader waits for two consecutive equal readings before publishing, because WirePlumber applies the card's stored volume while it comes up, and falls back to the last valid reading if the value never settles. Shortcuts now step from the cached value instead of issuing another read, and every volume write updates the cache. Settings -> Speaker seeds its first frame from the same cache and pushes mixer results (read, preview and commit) back into it, so leaving the page cannot expose a stale value. The persisted config key remains only a fallback for a page opened before the startup read finished. Launcher teardown joins the startup reader, and the test build now links pthread for the worker thread. --- .../main/ui/launcher_media_controls.cpp | 160 +++++++++++++++++- .../main/ui/launcher_media_controls.h | 15 ++ .../main/ui/settings/settings_volume_page.cpp | 32 ++-- projects/APPLaunch/main/ui/ui.cpp | 5 + projects/APPLaunch/tests/run_tests.sh | 2 +- .../tests/test_launcher_media_controls.cpp | 81 +++++++-- .../test_settings_volume_initial_selection.py | 73 ++++++-- 7 files changed, 318 insertions(+), 50 deletions(-) diff --git a/projects/APPLaunch/main/ui/launcher_media_controls.cpp b/projects/APPLaunch/main/ui/launcher_media_controls.cpp index b8adf171..46e1b998 100644 --- a/projects/APPLaunch/main/ui/launcher_media_controls.cpp +++ b/projects/APPLaunch/main/ui/launcher_media_controls.cpp @@ -12,12 +12,131 @@ #include "model/setup_value_policy.hpp" #include +#include +#include +#include +#include #include +#include namespace { LauncherMediaControlsModel model; +/* Volume cached at launcher startup. Every successful write (and every + * authoritative mixer read routed through update_cached_volume()) refreshes + * it, so shortcuts and Settings render one value instead of racing the mixer. */ +std::atomic g_cached_volume{-1}; + +constexpr int kStartupVolumeReadAttempts = 5; +constexpr std::chrono::milliseconds kStartupVolumeReadInterval{400}; + +int read_mixer_volume() +{ + int volume = -1; + try { + cp0_signal_audio_api({"VolumeRead"}, [&](int code, std::string data) { + int parsed = 0; + if (code == 0 && LauncherMediaControlsModel::parse_percent(data, parsed)) volume = parsed; + }); + } catch (...) { + return -1; + } + return volume; +} + +/* Reads the mixer on a worker thread because a PulseAudio session can take up + * to two seconds to answer, and the startup path must not block the first + * frame. WirePlumber applies the speaker port's stored/default volume while + * the card comes up, which can land after the first successful read, so the + * value is published only once two consecutive reads agree. If that never + * happens, the last valid reading is used rather than leaving the cache empty. + * The worker is joinable: launcher teardown joins it before the audio signal + * is released, and the static destructor joins as a last resort. */ +class StartupVolumeCache +{ +public: + ~StartupVolumeCache() { stop(); } + + void start() + { + stop(); + { + std::lock_guard lock(mutex_); + stop_ = false; + } + worker_ = std::thread([this] { + try { + run(); + } catch (...) { + } + }); + } + + void stop() + { + { + std::lock_guard lock(mutex_); + stop_ = true; + } + wake_.notify_all(); + if (worker_.joinable()) + worker_.join(); + } + +private: + bool stop_requested() + { + std::lock_guard lock(mutex_); + return stop_; + } + + bool wait_for_next_attempt() + { + std::unique_lock lock(mutex_); + wake_.wait_for(lock, kStartupVolumeReadInterval, [this] { return stop_; }); + return stop_; + } + + void publish(int volume) + { + g_cached_volume.store(LauncherMediaControlsModel::clamp_percent(volume)); + } + + void run() + { + int previous = -1; + int last_valid = -1; + for (int attempt = 0; attempt < kStartupVolumeReadAttempts; ++attempt) { + if (attempt > 0 && wait_for_next_attempt()) + return; + if (stop_requested()) + return; + + const int volume = read_mixer_volume(); + if (volume < 0) { + previous = -1; + continue; + } + last_valid = volume; + if (volume == previous) { + publish(volume); + return; + } + previous = volume; + } + if (last_valid >= 0 && !stop_requested()) + publish(last_valid); + } + + std::mutex mutex_; + std::condition_variable wake_; + bool stop_ = false; + std::thread worker_; +}; + +StartupVolumeCache g_startup_volume_cache; + int read_config_int(const char *key, int fallback) { int value = fallback; @@ -49,14 +168,17 @@ bool write_config_int(const char *key, int value, int fallback) int read_volume() { - int volume = -1; - cp0_signal_audio_api({"VolumeRead"}, [&](int code, std::string data) { - int parsed = 0; - if (code == 0 && LauncherMediaControlsModel::parse_percent(data, parsed)) volume = parsed; - }); + const int cached = g_cached_volume.load(); + if (cached >= 0) return LauncherMediaControlsModel::clamp_percent(cached); + + int volume = read_mixer_volume(); if (volume < 0) volume = read_config_int("volume", model.volume_or(50)); - return LauncherMediaControlsModel::clamp_percent(volume); + volume = LauncherMediaControlsModel::clamp_percent(volume); + /* The first authoritative value becomes the cache, so a shortcut pressed + * before the startup read completed cannot disagree with Settings. */ + g_cached_volume.store(volume); + return volume; } bool read_mute() @@ -87,6 +209,7 @@ int write_volume(int previous, int percent) return previous; } model.set_volume(written); + g_cached_volume.store(written); return written; } @@ -161,6 +284,31 @@ int write_brightness(int previous_percent, int percent) namespace launcher_media_controls { +bool has_cached_volume() +{ + return g_cached_volume.load() >= 0; +} + +int cached_volume() +{ + return g_cached_volume.load(); +} + +void start_volume_cache() +{ + g_startup_volume_cache.start(); +} + +void stop_volume_cache() +{ + g_startup_volume_cache.stop(); +} + +void update_cached_volume(int percent) +{ + g_cached_volume.store(LauncherMediaControlsModel::clamp_percent(percent)); +} + int adjust_volume(int delta_percent) { if (read_mute()) diff --git a/projects/APPLaunch/main/ui/launcher_media_controls.h b/projects/APPLaunch/main/ui/launcher_media_controls.h index a4a83a88..ae2e0d12 100644 --- a/projects/APPLaunch/main/ui/launcher_media_controls.h +++ b/projects/APPLaunch/main/ui/launcher_media_controls.h @@ -13,6 +13,21 @@ namespace launcher_media_controls { constexpr int VOLUME_STEP_PERCENT = setup_values::volume_metric(setup_values::VolumeMetric::StepPercent); +/* Volume cached for the lifetime of the launcher process. The launcher reads + * the system mixer once at startup and keeps this value in sync with every + * volume write it performs, so the shortcut OSD and Settings -> Speaker render + * the same number instead of querying the mixer at different times. + * cached_volume() returns -1 until the startup read produced a value. */ +bool has_cached_volume(); +int cached_volume(); +/* Read the mixer once, off the LVGL thread, and publish the settled value. + * Safe to call once at launcher startup; a later call restarts the read. */ +void start_volume_cache(); +/* Join the startup reader so teardown cannot race a signal call. */ +void stop_volume_cache(); +/* Record a value that was just written to the mixer, or read back from it. */ +void update_cached_volume(int percent); + int adjust_volume(int delta_percent); int adjust_brightness(int delta_percent); bool toggle_mute(); diff --git a/projects/APPLaunch/main/ui/settings/settings_volume_page.cpp b/projects/APPLaunch/main/ui/settings/settings_volume_page.cpp index 58934cc0..608b485b 100644 --- a/projects/APPLaunch/main/ui/settings/settings_volume_page.cpp +++ b/projects/APPLaunch/main/ui/settings/settings_volume_page.cpp @@ -7,6 +7,7 @@ #include "settings_volume_page.hpp" #include "hal_lvgl_bsp.h" +#include "../launcher_media_controls.h" #include #include @@ -158,9 +159,10 @@ class settings_audio_com { return saved.code == 0 ? saved : backend; } - // The persisted key is what the startup restore writes to the mixer, so it - // is the best value the page can show before the asynchronous mixer read - // returns. A missing or invalid entry keeps the supplied fallback. + // The persisted key is only a fallback: the launcher caches the mixer + // volume at startup, and that cache is what the shortcut OSD and this page + // must both render. A page opened before the startup read completed keeps + // the persisted value and the asynchronous read below refreshes both. static int read_saved_volume(int fallback) { const auto saved = decode_volume(invoke_config_default( @@ -310,11 +312,13 @@ void LvSettingVolumePage3::initialize_page(lv_obj_t *parent, std::function Speaker and the shortcut OSD agree. The persisted key is + // only used when the startup read has not produced a value yet; the + // asynchronous mixer read below refreshes both when it returns. + const int saved = launcher_media_controls::has_cached_volume() + ? launcher_media_controls::cached_volume() + : settings_audio_com::read_saved_volume(metric(LayoutMetric::MaxVolume)); original_volume_ = saved; backend_volume_ = saved; last_preview_requested_ = saved; @@ -532,6 +536,9 @@ void LvSettingVolumePage3::handle_volume_result(const ActiveRequest &request, const bool valid = result.code == 0 && settings_audio_com::volume_value_valid(result.value); if (request.kind == RequestKind::Read) { if (valid) { + /* The mixer is authoritative; publish what it reports so the + * shortcut OSD and every later render share this value. */ + launcher_media_controls::update_cached_volume(result.value); original_volume_ = result.value; backend_volume_ = result.value; last_preview_requested_ = result.value; @@ -554,10 +561,12 @@ void LvSettingVolumePage3::handle_volume_result(const ActiveRequest &request, rollback_pending_ = false; commit_requested_ = false; commit_leave_requested_ = false; - if (valid) + if (valid) { backend_volume_ = result.value; - else + launcher_media_controls::update_cached_volume(result.value); + } else { last_error_ = result.data.empty() ? "unable to restore volume" : result.data; + } select_volume(selection_for_volume(original_volume_)); preview_touched_ = false; last_preview_requested_ = backend_volume_; @@ -574,6 +583,9 @@ void LvSettingVolumePage3::handle_volume_result(const ActiveRequest &request, } backend_volume_ = result.value; + /* Preview and commit writes already changed the mixer; keep the shared + * cache in step so leaving this page cannot expose a stale value. */ + launcher_media_controls::update_cached_volume(result.value); last_error_.clear(); if (request.kind == RequestKind::PreviewWrite) { handle_preview_success(request); diff --git a/projects/APPLaunch/main/ui/ui.cpp b/projects/APPLaunch/main/ui/ui.cpp index e9020adc..670fa004 100644 --- a/projects/APPLaunch/main/ui/ui.cpp +++ b/projects/APPLaunch/main/ui/ui.cpp @@ -10,6 +10,7 @@ // Project name: zero #include "ui.h" +#include "launcher_media_controls.h" #include "launcher_ui_app_page.hpp" #include "ui_low_battery.h" #include "ui_global_hint.h" @@ -37,6 +38,9 @@ void init() { if (home) return; + /* Cache the system volume once here so the shortcut OSD and + * Settings -> Speaker share one value for the whole launcher session. */ + launcher_media_controls::start_volume_cache(); cp0_keyboard_set_global_key_handler(ui_global_hint::on_key); home = std::make_unique(); home->start(); @@ -46,6 +50,7 @@ void init() void deinit() { cp0_keyboard_set_global_key_handler(nullptr); + launcher_media_controls::stop_volume_cache(); ui_global_hint::shutdown(); launcher_battery_ui::shutdown_warning(); ui_loading::shutdown(); diff --git a/projects/APPLaunch/tests/run_tests.sh b/projects/APPLaunch/tests/run_tests.sh index b98b75fc..3ef0fbd0 100755 --- a/projects/APPLaunch/tests/run_tests.sh +++ b/projects/APPLaunch/tests/run_tests.sh @@ -240,7 +240,7 @@ ${CXX:-g++} -std=c++17 -Wall -Wextra -Werror \ -o "$build_dir/test_launcher_media_model" "$build_dir/test_launcher_media_model" -${CXX:-g++} -std=c++17 -Wall -Wextra -Werror \ +${CXX:-g++} -std=c++17 -Wall -Wextra -Werror -pthread \ -I"$(dirname "$0")/../../../ext_components/cp0_lvgl/include" \ -I"$(dirname "$0")/../../../SDK/github_source/eventpp/include" \ "$(dirname "$0")/test_launcher_media_controls.cpp" \ diff --git a/projects/APPLaunch/tests/test_launcher_media_controls.cpp b/projects/APPLaunch/tests/test_launcher_media_controls.cpp index 7aaf1daa..63c6071f 100644 --- a/projects/APPLaunch/tests/test_launcher_media_controls.cpp +++ b/projects/APPLaunch/tests/test_launcher_media_controls.cpp @@ -8,11 +8,15 @@ #include "hal_lvgl_bsp.h" +#include #include +#include +#include #include #include #include #include +#include #include eventpp::CallbackList, std::function)> @@ -36,6 +40,23 @@ bool sink_muted = true; int mute_toggles = 0; std::vector audio_commands; +std::size_t count_commands(const std::string &command) +{ + return static_cast( + std::count(audio_commands.begin(), audio_commands.end(), command)); +} + +bool wait_for_cached_volume(int expected, int timeout_ms = 3000) +{ + const auto deadline = + std::chrono::steady_clock::now() + std::chrono::milliseconds(timeout_ms); + while (launcher_media_controls::cached_volume() != expected) { + if (std::chrono::steady_clock::now() >= deadline) return false; + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + return true; +} + void reply(const std::function &callback, int code, const std::string &data) { @@ -124,37 +145,61 @@ int main() }); assert(launcher_media_controls::VOLUME_STEP_PERCENT == 10); + + // The launcher reads the mixer once at startup and caches it. The worker + // waits for two equal readings so a pre-settle level cannot be captured. + sink_volume = 63; + launcher_media_controls::start_volume_cache(); + assert(wait_for_cached_volume(63)); + launcher_media_controls::stop_volume_cache(); + assert(launcher_media_controls::has_cached_volume()); + const std::size_t startup_reads = count_commands("VolumeRead"); + assert(startup_reads >= 2); + + // A shortcut steps from the cached value; it must not issue a fresh read. assert(launcher_media_controls::adjust_volume(-10) == 50); assert(!sink_muted); assert(sink_volume == 50); assert(configured_volume == 50); + assert(launcher_media_controls::cached_volume() == 50); assert(mute_toggles == 1); - assert(audio_commands.size() >= 4); - assert(audio_commands[0] == "MuteRead"); - assert(audio_commands[1] == "MuteToggle"); - assert(audio_commands[2] == "VolumeRead"); - assert(audio_commands[3] == "VolumeWrite"); + assert(count_commands("VolumeRead") == startup_reads); + assert(audio_commands[0] == "VolumeRead"); + assert(audio_commands[1] == "VolumeRead"); + assert(audio_commands[2] == "MuteRead"); + assert(audio_commands[3] == "MuteToggle"); + assert(audio_commands[4] == "VolumeWrite"); + // An external mixer change does not reach the shortcut until something + // publishes it (the Settings read or a write does). + sink_volume = 90; sink_muted = true; - sink_volume = 63; - assert(launcher_media_controls::adjust_volume(10) == 70); - assert(!sink_muted); - assert(sink_volume == 70); - assert(configured_volume == 70); - assert(mute_toggles == 2); - - sink_volume = 71; - assert(launcher_media_controls::adjust_volume(10) == 80); + assert(launcher_media_controls::adjust_volume(10) == 60); assert(!sink_muted); - assert(sink_volume == 80); - assert(configured_volume == 80); + assert(sink_volume == 60); + assert(configured_volume == 60); + assert(launcher_media_controls::cached_volume() == 60); assert(mute_toggles == 2); + assert(count_commands("VolumeRead") == startup_reads); - sink_volume = 82; + launcher_media_controls::update_cached_volume(82); + assert(launcher_media_controls::cached_volume() == 82); assert(launcher_media_controls::adjust_volume(-10) == 70); assert(sink_volume == 70); assert(configured_volume == 70); - assert(mute_toggles == 2); + assert(launcher_media_controls::cached_volume() == 70); + + launcher_media_controls::update_cached_volume(150); + assert(launcher_media_controls::cached_volume() == 100); + launcher_media_controls::update_cached_volume(-10); + assert(launcher_media_controls::cached_volume() == 0); + + // A launcher restart re-reads the mixer and replaces the cache. + sink_volume = 63; + launcher_media_controls::start_volume_cache(); + assert(wait_for_cached_volume(63)); + launcher_media_controls::stop_volume_cache(); + assert(launcher_media_controls::cached_volume() == 63); assert(launcher_media_controls::adjust_brightness(-10) == 30); assert(backlight_value == 76); diff --git a/projects/APPLaunch/tests/test_settings_volume_initial_selection.py b/projects/APPLaunch/tests/test_settings_volume_initial_selection.py index e1bcc728..8b9a6f86 100644 --- a/projects/APPLaunch/tests/test_settings_volume_initial_selection.py +++ b/projects/APPLaunch/tests/test_settings_volume_initial_selection.py @@ -9,19 +9,20 @@ SOURCE = (ROOT / "main/ui/settings/settings_volume_page.cpp").read_text( encoding="utf-8" ) +UI_SOURCE = (ROOT / "main/ui/ui.cpp").read_text(encoding="utf-8") -def function_body(signature: str) -> str: - start = SOURCE.index(signature) - opening_brace = SOURCE.index("{", start) +def function_body(source: str, signature: str) -> str: + start = source.index(signature) + opening_brace = source.index("{", start) depth = 0 - for index in range(opening_brace, len(SOURCE)): - if SOURCE[index] == "{": + for index in range(opening_brace, len(source)): + if source[index] == "{": depth += 1 - elif SOURCE[index] == "}": + elif source[index] == "}": depth -= 1 if depth == 0: - return SOURCE[opening_brace + 1 : index] + return source[opening_brace + 1 : index] raise AssertionError(f"unterminated function: {signature}") @@ -29,12 +30,14 @@ def normalized(source: str) -> str: return re.sub(r"\s+", " ", source).strip() -def test_page_seeds_the_persisted_volume_before_the_list_is_built(): - body = normalized(function_body("void LvSettingVolumePage3::initialize_page")) - seed = body.index("read_saved_volume") +def test_page_seeds_the_cached_volume_before_the_list_is_built(): + body = normalized( + function_body(SOURCE, "void LvSettingVolumePage3::initialize_page") + ) + seed = body.index("launcher_media_controls::cached_volume") built = body.index("initialize(parent)") - assert seed < built, "the saved volume must be read before the value list is built" + assert seed < built, "the cached volume must be read before the value list is built" for member in ( "original_volume_", "backend_volume_", @@ -45,22 +48,38 @@ def test_page_seeds_the_persisted_volume_before_the_list_is_built(): assert member in body[seed:built], f"{member} is not seeded before create_ui" -def test_seed_uses_the_persisted_config_not_the_mixer(): - body = normalized(function_body("static int read_saved_volume(int fallback)")) +def test_seed_falls_back_to_the_persisted_config_only_without_a_cache(): + body = normalized( + function_body(SOURCE, "void LvSettingVolumePage3::initialize_page") + ) + + assert "launcher_media_controls::has_cached_volume" in body + assert "read_saved_volume" in body + assert body.index("cached_volume") < body.index("read_saved_volume"), ( + "the launcher cache is the primary seed; the persisted key is a fallback" + ) + + +def test_fallback_seed_uses_the_persisted_config_not_the_mixer(): + body = normalized( + function_body(SOURCE, "static int read_saved_volume(int fallback)") + ) assert '{"GetInt", "volume"' in body assert "VolumeRead" not in body def test_initial_selection_uses_the_seeded_volume(): - body = normalized(function_body("int LvSettingVolumePage3::initial_selection() const")) + body = normalized( + function_body(SOURCE, "int LvSettingVolumePage3::initial_selection() const") + ) assert "return selection_for_volume(backend_volume_);" in body assert "return 0;" not in body def test_failed_read_does_not_restore_the_100_percent_default(): - body = function_body("void LvSettingVolumePage3::handle_volume_result(") + body = function_body(SOURCE, "void LvSettingVolumePage3::handle_volume_result(") read_branch = body[ body.index("if (request.kind == RequestKind::Read)") : body.index( "if (request.kind == RequestKind::RestoreWrite)" @@ -71,3 +90,27 @@ def test_failed_read_does_not_restore_the_100_percent_default(): assert "metric(LayoutMetric::MaxVolume)" not in read_branch assert "select_volume(0);" not in read_branch assert "select_volume(selection_for_volume(backend_volume_));" in read_branch + + +def test_mixer_results_refresh_the_shared_cache(): + body = normalized( + function_body(SOURCE, "void LvSettingVolumePage3::handle_volume_result(") + ) + cache = "launcher_media_controls::update_cached_volume" + + assert cache in body, "the page must publish mixer results to the shared cache" + read_branch = body[ + body.index("RequestKind::Read") : body.index("RequestKind::RestoreWrite") + ] + assert cache in read_branch, "the mixer read must refresh the shared cache" + + write_branch = body[body.index("if (!valid)") :] + assert cache in write_branch, "preview/commit writes must refresh the shared cache" + + +def test_launcher_startup_starts_and_stops_the_cache(): + init_body = normalized(function_body(UI_SOURCE, "void init()")) + deinit_body = normalized(function_body(UI_SOURCE, "void deinit()")) + + assert "launcher_media_controls::start_volume_cache()" in init_body + assert "launcher_media_controls::stop_volume_cache()" in deinit_body