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