CRAS: Return CrasEffectUIAppearance

This changes makes AudioEffectUIAppearance return CrasEffectUIAppearance instead of audio_effects_ready. BUG=b:368043481 TEST=bazel test //... Change-Id: I1094d46968c328825a5472c852731ae6dad1e232 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/5934484 Reviewed-by: Hung-Hsien Chen <[email protected]> Tested-by: [email protected] <[email protected]> Reviewed-by: Li-Yu Yu <[email protected]> Commit-Queue: Cranel W <[email protected]>
chromeos · Oct 25, 2024 · 0e7e891 · 0e7e891
1 parent d218fe0
commit 0e7e891
Show file tree

Hide file tree

Showing 13 changed files with 145 additions and 45 deletions.
diff --git a/cras/common/BUILD.bazel b/cras/common/BUILD.bazel
@@ -56,6 +56,15 @@ cras_cbindgen(
     lib = ":rust_common",
 )
 
+filegroup(
+    name = "rust_common_h_generated",
+    srcs = ["rust_common.h"],
+    visibility = [
+        "//cras/src/common:__pkg__",
+        "//cras/src/tests:__pkg__",
+    ],
+)
+
 cc_library(
     name = "rust_common_cc",
     hdrs = ["rust_common.h"],

diff --git a/cras/dbus_bindings/org.chromium.cras.Control.xml b/cras/dbus_bindings/org.chromium.cras.Control.xml
@@ -723,6 +723,15 @@
       </tp:docstring>
       <arg name="supported" type="b" direction="out"/>
     </method>
+
+    <signal name="AudioEffectUIAppearanceChanged">
+      <tp:docstring>
+        Signal that indicates the audio effect ui appearance needs update. See also GetVoiceIsolationUIAppearance.
+      </tp:docstring>
+      <arg name="toggle_type" type="u"/>
+      <arg name="effect_mode_options" type="u"/>
+      <arg name="show_effect_fallback_message" type="b"/>
+    </signal>
   </interface>
 
   <interface name="org.freedesktop.DBus.Introspectable">

diff --git a/cras/server/s2/s2.h b/cras/server/s2/s2.h
@@ -19,7 +19,7 @@ extern "C" {
 #include <stdlib.h>
 #include "cras/common/rust_common.h"
 
-typedef void (*NotifyAudioEffectUIAppearanceChanged)(bool);
+typedef void (*NotifyAudioEffectUIAppearanceChanged)(struct CrasEffectUIAppearance);
 
 void cras_s2_set_ap_nc_featured_allowed(bool allowed);
 

diff --git a/cras/server/s2/src/global.rs b/cras/server/s2/src/global.rs
@@ -48,7 +48,7 @@ pub extern "C" fn cras_s2_are_audio_effects_ready() -> bool {
 
 // Called when audio_effects_ready is changed, with the following arguments:
 // - bool: indicates whether the audio effects is ready.
-pub type NotifyAudioEffectUIAppearanceChanged = extern "C" fn(bool);
+pub type NotifyAudioEffectUIAppearanceChanged = extern "C" fn(CrasEffectUIAppearance);
 
 #[no_mangle]
 pub extern "C" fn cras_s2_set_notify_audio_effect_ui_appearance_changed(

diff --git a/cras/server/s2/src/lib.rs b/cras/server/s2/src/lib.rs
@@ -444,7 +444,7 @@ impl S2 {
 
         if let Some(callback) = self.callbacks.notify_audio_effect_ui_appearance_changed {
             if prev_audio_effects_ready != self.output.audio_effects_ready {
-                callback(self.output.audio_effects_ready)
+                callback(self.output.audio_effect_ui_appearance)
             }
         }
     }
@@ -459,9 +459,11 @@ mod tests {
     use std::collections::HashMap;
     use std::collections::HashSet;
     use std::sync::atomic::AtomicBool;
+    use std::sync::atomic::AtomicU32;
     use std::sync::atomic::Ordering;
 
     use cras_common::types_internal::CrasDlcId;
+    use cras_common::types_internal::CrasEffectUIAppearance;
     use cras_common::types_internal::CRAS_NC_PROVIDER;
     use cras_common::types_internal::EFFECT_TYPE;
 
@@ -622,11 +624,19 @@ mod tests {
         let mut s = S2::new();
         assert!(!s.output.audio_effects_ready);
 
+        // Simply verifies the callback is called.
         static CALLED: AtomicBool = AtomicBool::new(false);
-        static IS_READY: AtomicBool = AtomicBool::new(false);
-        extern "C" fn fake_notify_audio_effect_ui_appearance_changed(ready: bool) {
+        static TOGGLE_TYPE: AtomicU32 = AtomicU32::new(0);
+        static EFFECT_MODE_OPTIONS: AtomicU32 = AtomicU32::new(0);
+        static SHOW_EFFECT_FALLBACK_MESSAGE: AtomicBool = AtomicBool::new(false);
+        extern "C" fn fake_notify_audio_effect_ui_appearance_changed(
+            appearance: CrasEffectUIAppearance,
+        ) {
             CALLED.store(true, Ordering::SeqCst);
-            IS_READY.store(ready, Ordering::SeqCst);
+            TOGGLE_TYPE.store(appearance.toggle_type.bits(), Ordering::SeqCst);
+            EFFECT_MODE_OPTIONS.store(appearance.effect_mode_options.bits(), Ordering::SeqCst);
+            SHOW_EFFECT_FALLBACK_MESSAGE
+                .store(appearance.show_effect_fallback_message, Ordering::SeqCst);
         }
 
         s.set_notify_audio_effect_ui_appearance_changed(
@@ -642,7 +652,6 @@ mod tests {
         ]));
         assert!(!s.output.audio_effects_ready);
         assert!(!CALLED.load(Ordering::SeqCst));
-        assert!(!IS_READY.load(Ordering::SeqCst));
 
         s.set_dlc_installed(CrasDlcId::CrasDlcNcAp, true);
         assert!(!s.output.audio_effects_ready);
@@ -651,13 +660,45 @@ mod tests {
         s.set_dlc_installed(CrasDlcId::CrasDlcIntelligoBeamforming, true);
         assert!(s.output.audio_effects_ready);
         assert!(CALLED.load(Ordering::SeqCst));
-        assert!(IS_READY.load(Ordering::SeqCst));
+        assert_eq!(
+            TOGGLE_TYPE.load(Ordering::SeqCst),
+            s.output.audio_effect_ui_appearance.toggle_type.bits()
+        );
+        assert_eq!(
+            EFFECT_MODE_OPTIONS.load(Ordering::SeqCst),
+            s.output
+                .audio_effect_ui_appearance
+                .effect_mode_options
+                .bits()
+        );
+        assert_eq!(
+            SHOW_EFFECT_FALLBACK_MESSAGE.load(Ordering::SeqCst),
+            s.output
+                .audio_effect_ui_appearance
+                .show_effect_fallback_message
+        );
         CALLED.store(false, Ordering::SeqCst);
 
         s.set_dlc_installed(CrasDlcId::CrasDlcNcAp, false);
         assert!(!s.output.audio_effects_ready);
         assert!(CALLED.load(Ordering::SeqCst));
-        assert!(!IS_READY.load(Ordering::SeqCst));
+        assert_eq!(
+            TOGGLE_TYPE.load(Ordering::SeqCst),
+            s.output.audio_effect_ui_appearance.toggle_type.bits()
+        );
+        assert_eq!(
+            EFFECT_MODE_OPTIONS.load(Ordering::SeqCst),
+            s.output
+                .audio_effect_ui_appearance
+                .effect_mode_options
+                .bits()
+        );
+        assert_eq!(
+            SHOW_EFFECT_FALLBACK_MESSAGE.load(Ordering::SeqCst),
+            s.output
+                .audio_effect_ui_appearance
+                .show_effect_fallback_message
+        );
     }
 
     #[test]

diff --git a/cras/src/common/BUILD.bazel b/cras/src/common/BUILD.bazel
@@ -61,6 +61,7 @@ cc_library(
     name = "cras_observer_ops",
     hdrs = [
         "cras_observer_ops.h",
+        "//cras/common:rust_common_h_generated",
     ],
     visibility = [
         "//cras/src/libcras:__pkg__",

diff --git a/cras/src/common/cras_observer_ops.h b/cras/src/common/cras_observer_ops.h
@@ -6,6 +6,7 @@
 #ifndef CRAS_SRC_COMMON_CRAS_OBSERVER_OPS_H_
 #define CRAS_SRC_COMMON_CRAS_OBSERVER_OPS_H_
 
+#include "cras/common/rust_common.h"
 #include "cras_types.h"
 
 #ifdef __cplusplus
@@ -100,8 +101,9 @@ struct cras_observer_ops {
   void (*sidetone_supported_changed)(void* context, bool supported);
 
   // State regarding whether the audio effects are ready.
-  void (*audio_effect_ui_appearance_changed)(void* context,
-                                             bool audio_effects_ready);
+  void (*audio_effect_ui_appearance_changed)(
+      void* context,
+      struct CrasEffectUIAppearance ui_appearance);
 };
 
 #ifdef __cplusplus

diff --git a/cras/src/server/cras_dbus_control.c b/cras/src/server/cras_dbus_control.c
@@ -1096,6 +1096,23 @@ static DBusHandlerResult handle_get_audio_effect_dlcs(DBusConnection* conn,
   return ret;
 }
 
+// Appends args for audio effect ui appearance.
+// Returns 0 if success. Otherwise, DBusHandlerResult is returned.
+static int append_args_audio_effect_ui_appearance(
+    DBusMessage* msg,
+    struct CrasEffectUIAppearance appearance) {
+  // boolean size must be fixed at 4 bytes due to wire protocol!
+  dbus_bool_t show_effect_fallback_message =
+      appearance.show_effect_fallback_message;
+  if (!dbus_message_append_args(
+          msg, DBUS_TYPE_UINT32, &appearance.toggle_type, DBUS_TYPE_UINT32,
+          &appearance.effect_mode_options, DBUS_TYPE_BOOLEAN,
+          &show_effect_fallback_message, DBUS_TYPE_INVALID)) {
+    return DBUS_HANDLER_RESULT_NEED_MEMORY;
+  }
+  return 0;
+}
+
 static DBusHandlerResult handle_get_voice_isolation_ui_appearance(
     DBusConnection* conn,
     DBusMessage* message,
@@ -1109,18 +1126,8 @@ static DBusHandlerResult handle_get_voice_isolation_ui_appearance(
     return DBUS_HANDLER_RESULT_NEED_MEMORY;
   }
 
-  struct CrasEffectUIAppearance appearance =
-      cras_s2_get_audio_effect_ui_appearance();
-  dbus_uint32_t toggle_type = appearance.toggle_type;
-  dbus_uint32_t effect_mode_options = appearance.effect_mode_options;
-  // boolean size must be fixed at 4 bytes due to wire protocol!
-  dbus_bool_t show_effect_fallback_message =
-      appearance.show_effect_fallback_message;
-  if (!dbus_message_append_args(
-          reply, DBUS_TYPE_UINT32, &toggle_type, DBUS_TYPE_UINT32,
-          &effect_mode_options, DBUS_TYPE_BOOLEAN,
-          &show_effect_fallback_message, DBUS_TYPE_INVALID)) {
-    ret = DBUS_HANDLER_RESULT_NEED_MEMORY;
+  if (append_args_audio_effect_ui_appearance(
+          reply, cras_s2_get_audio_effect_ui_appearance()) != 0) {
     goto unref_reply;
   }
   if (!dbus_connection_send(conn, reply, &serial)) {
@@ -2389,17 +2396,17 @@ static void signal_sidetone_supported_changed(void* context, bool supported) {
 
 static void signal_audio_effect_ui_appearance_changed(
     void* context,
-    bool audio_effects_ready) {
+    struct CrasEffectUIAppearance ui_appearance) {
   struct cras_dbus_control* control = (struct cras_dbus_control*)context;
   dbus_uint32_t serial = 0;
-  dbus_bool_t ready = audio_effects_ready;
 
   DBusMessage* msg = create_dbus_message("AudioEffectUIAppearanceChanged");
   if (!msg) {
     return;
   }
-
-  dbus_message_append_args(msg, DBUS_TYPE_BOOLEAN, &ready, DBUS_TYPE_INVALID);
+  if (append_args_audio_effect_ui_appearance(msg, ui_appearance) != 0) {
+    return;
+  }
   dbus_connection_send(control->conn, msg, &serial);
   dbus_message_unref(msg);
 }

diff --git a/cras/src/server/cras_observer.c b/cras/src/server/cras_observer.c
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "cras/common/rust_common.h"
 #include "cras/src/common/cras_observer_ops.h"
 #include "cras/src/server/cras_alert.h"
 #include "cras/src/server/cras_iodev_list.h"
@@ -148,7 +149,7 @@ struct cras_observer_alert_data_sidetone_supported_changed {
 };
 
 struct cras_observer_alert_data_audio_effect_ui_appearance_changed {
-  bool audio_effects_ready;
+  struct CrasEffectUIAppearance ui_appearance;
 };
 
 // Global observer instance.
@@ -496,8 +497,8 @@ static void audio_effect_ui_appearance_changed_alert(void* arg, void* data) {
 
   DL_FOREACH (g_observer->clients, client) {
     if (client->ops.audio_effect_ui_appearance_changed) {
-      client->ops.audio_effect_ui_appearance_changed(
-          client->context, report->audio_effects_ready);
+      client->ops.audio_effect_ui_appearance_changed(client->context,
+                                                     report->ui_appearance);
     }
   }
 }
@@ -903,9 +904,9 @@ void cras_observer_notify_sidetone_supported_changed(bool supported) {
 }
 
 void cras_observer_notify_audio_effect_ui_appearance_changed(
-    bool audio_effects_ready) {
+    struct CrasEffectUIAppearance ui_appearance) {
   struct cras_observer_alert_data_audio_effect_ui_appearance_changed data = {
-      .audio_effects_ready = audio_effects_ready};
+      .ui_appearance = ui_appearance};
   cras_alert_pending_data(g_observer->alerts.audio_effect_ui_appearance_changed,
                           &data, sizeof(data));
 }
diff --git a/cras/src/server/cras_observer.h b/cras/src/server/cras_observer.h
@@ -6,6 +6,7 @@
 #ifndef CRAS_SRC_SERVER_CRAS_OBSERVER_H_
 #define CRAS_SRC_SERVER_CRAS_OBSERVER_H_
 
+#include "cras/common/rust_common.h"
 #include "cras/src/common/cras_observer_ops.h"
 
 #ifdef __cplusplus
@@ -150,7 +151,7 @@ void cras_observer_notify_sidetone_supported_changed(bool supported);
 
 // Notify observers whether the audio effects are ready.
 void cras_observer_notify_audio_effect_ui_appearance_changed(
-    bool audio_effects_ready);
+    struct CrasEffectUIAppearance ui_appearance);
 
 #ifdef __cplusplus
 }  // extern "C"

diff --git a/cras/src/tests/BUILD.bazel b/cras/src/tests/BUILD.bazel
@@ -678,6 +678,7 @@ cc_test(
     name = "cras_abi_unittest",
     srcs = [
         ":cras_abi_unittest.cc",
+        "//cras/common:rust_common_h_generated",
         "//cras/src/common:cras_audio_format.c",
         "//cras/src/common:cras_config.c",
         "//cras/src/common:cras_file_wait.c",
@@ -700,6 +701,7 @@ cc_test(
     name = "cras_client_unittest",
     srcs = [
         ":cras_client_unittest.cc",
+        "//cras/common:rust_common_h_generated",
         "//cras/src/common:cras_config.c",
         "//cras/src/common:cras_file_wait.c",
         "//cras/src/common:cras_shm.c",