libauiohal: Implement StreamOut callbacks

Implement StreamOutHalInterface::set{|Event|LatencyMode}Callback
methods. Due to differences in callback registration between
HIDL and AIDL, some extra management logic is required.

Bug: 205884982
Test: start cuttlefish with AIDL HAL enabled
      (there are no unit tests for these callbacks,
       and cuttlefish does not support offload)
Change-Id: Ic8e2ab5a0b75d1501289474b3064e5ca74f0e024
diff --git a/media/libaudiohal/impl/DeviceHalAidl.cpp b/media/libaudiohal/impl/DeviceHalAidl.cpp
index 62b9ec7..2951752a 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalAidl.cpp
@@ -20,6 +20,8 @@
 #include <algorithm>
 #include <forward_list>
 
+#include <aidl/android/hardware/audio/core/BnStreamCallback.h>
+#include <aidl/android/hardware/audio/core/BnStreamOutEventCallback.h>
 #include <aidl/android/hardware/audio/core/StreamDescriptor.h>
 #include <error/expected_utils.h>
 #include <media/AidlConversionCppNdk.h>
@@ -37,6 +39,7 @@
 using aidl::android::media::audio::common::AudioDeviceType;
 using aidl::android::media::audio::common::AudioInputFlags;
 using aidl::android::media::audio::common::AudioIoFlags;
+using aidl::android::media::audio::common::AudioLatencyMode;
 using aidl::android::media::audio::common::AudioMode;
 using aidl::android::media::audio::common::AudioOutputFlags;
 using aidl::android::media::audio::common::AudioPort;
@@ -299,6 +302,123 @@
     return OK;
 }
 
+namespace {
+
+class StreamCallbackBase {
+  protected:
+    explicit StreamCallbackBase(const sp<CallbackBroker>& broker) : mBroker(broker) {}
+  public:
+    void* getCookie() const { return mCookie; }
+    void setCookie(void* cookie) { mCookie = cookie; }
+    sp<CallbackBroker> getBroker() const {
+        if (void* cookie = mCookie; cookie != nullptr) return mBroker.promote();
+        return nullptr;
+    }
+  private:
+    const wp<CallbackBroker> mBroker;
+    std::atomic<void*> mCookie;
+};
+
+template<class C>
+class StreamCallbackBaseHelper {
+  protected:
+    explicit StreamCallbackBaseHelper(const StreamCallbackBase& base) : mBase(base) {}
+    sp<C> getCb(const sp<CallbackBroker>& broker, void* cookie);
+    using CbRef = const sp<C>&;
+    ndk::ScopedAStatus runCb(const std::function<void(CbRef cb)>& f) {
+        if (auto cb = getCb(mBase.getBroker(), mBase.getCookie()); cb != nullptr) f(cb);
+        return ndk::ScopedAStatus::ok();
+    }
+  private:
+    const StreamCallbackBase& mBase;
+};
+
+template<>
+sp<StreamOutHalInterfaceCallback> StreamCallbackBaseHelper<StreamOutHalInterfaceCallback>::getCb(
+        const sp<CallbackBroker>& broker, void* cookie) {
+    if (broker != nullptr) return broker->getStreamOutCallback(cookie);
+    return nullptr;
+}
+
+template<>
+sp<StreamOutHalInterfaceEventCallback>
+StreamCallbackBaseHelper<StreamOutHalInterfaceEventCallback>::getCb(
+        const sp<CallbackBroker>& broker, void* cookie) {
+    if (broker != nullptr) return broker->getStreamOutEventCallback(cookie);
+    return nullptr;
+}
+
+template<>
+sp<StreamOutHalInterfaceLatencyModeCallback>
+StreamCallbackBaseHelper<StreamOutHalInterfaceLatencyModeCallback>::getCb(
+        const sp<CallbackBroker>& broker, void* cookie) {
+    if (broker != nullptr) return broker->getStreamOutLatencyModeCallback(cookie);
+    return nullptr;
+}
+
+/*
+Note on the callback ownership.
+
+In the Binder ownership model, the server implementation is kept alive
+as long as there is any client (proxy object) alive. This is done by
+incrementing the refcount of the server-side object by the Binder framework.
+When it detects that the last client is gone, it decrements the refcount back.
+
+Thus, it is not needed to keep any references to StreamCallback on our
+side (after we have sent an instance to the client), because we are
+the server-side. The callback object will be kept alive as long as the HAL server
+holds a strong ref to IStreamCallback proxy.
+*/
+
+class OutputStreamCallbackAidl : public StreamCallbackBase,
+                             public StreamCallbackBaseHelper<StreamOutHalInterfaceCallback>,
+                             public ::aidl::android::hardware::audio::core::BnStreamCallback {
+  public:
+    explicit OutputStreamCallbackAidl(const sp<CallbackBroker>& broker)
+            : StreamCallbackBase(broker),
+              StreamCallbackBaseHelper<StreamOutHalInterfaceCallback>(
+                      *static_cast<StreamCallbackBase*>(this)) {}
+    ndk::ScopedAStatus onTransferReady() override {
+        return runCb([](CbRef cb) { cb->onWriteReady(); });
+    }
+    ndk::ScopedAStatus onError() override {
+        return runCb([](CbRef cb) { cb->onError(); });
+    }
+    ndk::ScopedAStatus onDrainReady() override {
+        return runCb([](CbRef cb) { cb->onDrainReady(); });
+    }
+};
+
+class OutputStreamEventCallbackAidl :
+            public StreamCallbackBase,
+            public StreamCallbackBaseHelper<StreamOutHalInterfaceEventCallback>,
+            public StreamCallbackBaseHelper<StreamOutHalInterfaceLatencyModeCallback>,
+            public ::aidl::android::hardware::audio::core::BnStreamOutEventCallback {
+  public:
+    explicit OutputStreamEventCallbackAidl(const sp<CallbackBroker>& broker)
+            : StreamCallbackBase(broker),
+              StreamCallbackBaseHelper<StreamOutHalInterfaceEventCallback>(
+                      *static_cast<StreamCallbackBase*>(this)),
+              StreamCallbackBaseHelper<StreamOutHalInterfaceLatencyModeCallback>(
+                      *static_cast<StreamCallbackBase*>(this)) {}
+    ndk::ScopedAStatus onCodecFormatChanged(const std::vector<uint8_t>& in_audioMetadata) override {
+        std::basic_string<uint8_t> halMetadata(in_audioMetadata.begin(), in_audioMetadata.end());
+        return StreamCallbackBaseHelper<StreamOutHalInterfaceEventCallback>::runCb(
+                [&halMetadata](auto cb) { cb->onCodecFormatChanged(halMetadata); });
+    }
+    ndk::ScopedAStatus onRecommendedLatencyModeChanged(
+            const std::vector<AudioLatencyMode>& in_modes) override {
+        auto halModes = VALUE_OR_FATAL(
+                ::aidl::android::convertContainer<std::vector<audio_latency_mode_t>>(
+                        in_modes,
+                        ::aidl::android::aidl2legacy_AudioLatencyMode_audio_latency_mode_t));
+        return StreamCallbackBaseHelper<StreamOutHalInterfaceLatencyModeCallback>::runCb(
+                [&halModes](auto cb) { cb->onRecommendedLatencyModeChanged(halModes); });
+    }
+};
+
+}  // namespace
+
 status_t DeviceHalAidl::openOutputStream(
         audio_io_handle_t handle, audio_devices_t devices,
         audio_output_flags_t flags, struct audio_config* config,
@@ -326,8 +446,19 @@
                     &cleanups, &aidlConfig, &mixPortConfig, &nominalLatency));
     ::aidl::android::hardware::audio::core::IModule::OpenOutputStreamArguments args;
     args.portConfigId = mixPortConfig.id;
-    args.offloadInfo = aidlConfig.offloadInfo;
+    const bool isOffload = isBitPositionFlagSet(
+            aidlOutputFlags, AudioOutputFlags::COMPRESS_OFFLOAD);
+    std::shared_ptr<OutputStreamCallbackAidl> streamCb;
+    if (isOffload) {
+        streamCb = ndk::SharedRefBase::make<OutputStreamCallbackAidl>(this);
+    }
+    auto eventCb = ndk::SharedRefBase::make<OutputStreamEventCallbackAidl>(this);
+    if (isOffload) {
+        args.offloadInfo = aidlConfig.offloadInfo;
+        args.callback = streamCb;
+    }
     args.bufferSizeFrames = aidlConfig.frameCount;
+    args.eventCallback = eventCb;
     ::aidl::android::hardware::audio::core::IModule::OpenOutputStreamReturn ret;
     RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->openOutputStream(args, &ret)));
     StreamContextAidl context(ret.desc);
@@ -337,7 +468,14 @@
         return NO_INIT;
     }
     *outStream = sp<StreamOutHalAidl>::make(*config, std::move(context), nominalLatency,
-            std::move(ret.stream));
+            std::move(ret.stream), this /*callbackBroker*/);
+    void* cbCookie = (*outStream).get();
+    {
+        std::lock_guard l(mLock);
+        mCallbacks.emplace(cbCookie, Callbacks{});
+    }
+    if (streamCb) streamCb->setCookie(cbCookie);
+    eventCb->setCookie(cbCookie);
     cleanups.disarmAll();
     return OK;
 }
@@ -883,4 +1021,55 @@
     ALOGE("%s: port config id %d not found", __func__, portConfigId);
 }
 
+void DeviceHalAidl::clearCallbacks(void* cookie) {
+    std::lock_guard l(mLock);
+    mCallbacks.erase(cookie);
+}
+
+sp<StreamOutHalInterfaceCallback> DeviceHalAidl::getStreamOutCallback(void* cookie) {
+    return getCallbackImpl(cookie, &Callbacks::out);
+}
+
+void DeviceHalAidl::setStreamOutCallback(
+        void* cookie, const sp<StreamOutHalInterfaceCallback>& cb) {
+    setCallbackImpl(cookie, &Callbacks::out, cb);
+}
+
+sp<StreamOutHalInterfaceEventCallback> DeviceHalAidl::getStreamOutEventCallback(
+        void* cookie) {
+    return getCallbackImpl(cookie, &Callbacks::event);
+}
+
+void DeviceHalAidl::setStreamOutEventCallback(
+        void* cookie, const sp<StreamOutHalInterfaceEventCallback>& cb) {
+    setCallbackImpl(cookie, &Callbacks::event, cb);
+}
+
+sp<StreamOutHalInterfaceLatencyModeCallback> DeviceHalAidl::getStreamOutLatencyModeCallback(
+        void* cookie) {
+    return getCallbackImpl(cookie, &Callbacks::latency);
+}
+
+void DeviceHalAidl::setStreamOutLatencyModeCallback(
+        void* cookie, const sp<StreamOutHalInterfaceLatencyModeCallback>& cb) {
+    setCallbackImpl(cookie, &Callbacks::latency, cb);
+}
+
+template<class C>
+sp<C> DeviceHalAidl::getCallbackImpl(void* cookie, wp<C> DeviceHalAidl::Callbacks::* field) {
+    std::lock_guard l(mLock);
+    if (auto it = mCallbacks.find(cookie); it != mCallbacks.end()) {
+        return ((it->second).*field).promote();
+    }
+    return nullptr;
+}
+template<class C>
+void DeviceHalAidl::setCallbackImpl(
+        void* cookie, wp<C> DeviceHalAidl::Callbacks::* field, const sp<C>& cb) {
+    std::lock_guard l(mLock);
+    if (auto it = mCallbacks.find(cookie); it != mCallbacks.end()) {
+        (it->second).*field = cb;
+    }
+}
+
 } // namespace android
diff --git a/media/libaudiohal/impl/DeviceHalAidl.h b/media/libaudiohal/impl/DeviceHalAidl.h
index f5dd476..ea0cd4d 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.h
+++ b/media/libaudiohal/impl/DeviceHalAidl.h
@@ -22,6 +22,7 @@
 
 #include <aidl/android/hardware/audio/core/BpModule.h>
 #include <aidl/android/hardware/audio/core/sounddose/BpSoundDose.h>
+#include <android-base/thread_annotations.h>
 #include <media/audiohal/DeviceHalInterface.h>
 #include <media/audiohal/EffectHalInterface.h>
 
@@ -29,7 +30,35 @@
 
 namespace android {
 
-class DeviceHalAidl : public DeviceHalInterface, public ConversionHelperAidl {
+class StreamOutHalInterfaceCallback;
+class StreamOutHalInterfaceEventCallback;
+class StreamOutHalInterfaceLatencyModeCallback;
+
+// The role of the broker is to connect AIDL callback interface implementations
+// with StreamOut callback implementations. Since AIDL requires all callbacks
+// to be provided upfront, while libaudiohal interfaces allow late registration,
+// there is a need to coordinate the matching process.
+class CallbackBroker : public virtual RefBase {
+  public:
+    virtual ~CallbackBroker() = default;
+    // The cookie is always the stream instance pointer. We don't use weak pointers to avoid extra
+    // costs on reference counting. The stream cleans up related entries on destruction. Since
+    // access to the callbacks map is synchronized, the possibility for pointer aliasing due to
+    // allocation of a new stream at the address of previously deleted stream is avoided.
+    virtual void clearCallbacks(void* cookie) = 0;
+    virtual sp<StreamOutHalInterfaceCallback> getStreamOutCallback(void* cookie) = 0;
+    virtual void setStreamOutCallback(void* cookie, const sp<StreamOutHalInterfaceCallback>&) = 0;
+    virtual sp<StreamOutHalInterfaceEventCallback> getStreamOutEventCallback(void* cookie) = 0;
+    virtual void setStreamOutEventCallback(void* cookie,
+            const sp<StreamOutHalInterfaceEventCallback>&) = 0;
+    virtual sp<StreamOutHalInterfaceLatencyModeCallback> getStreamOutLatencyModeCallback(
+            void* cookie) = 0;
+    virtual void setStreamOutLatencyModeCallback(
+            void* cookie, const sp<StreamOutHalInterfaceLatencyModeCallback>&) = 0;
+};
+
+class DeviceHalAidl : public DeviceHalInterface, public ConversionHelperAidl,
+                      public CallbackBroker {
   public:
     // Sets the value of 'devices' to a bitmask of 1 or more values of audio_devices_t.
     status_t getSupportedDevices(uint32_t *devices) override;
@@ -127,6 +156,12 @@
 
   private:
     friend class sp<DeviceHalAidl>;
+
+    struct Callbacks {  // No need to use `atomic_wp` because access is serialized.
+        wp<StreamOutHalInterfaceCallback> out;
+        wp<StreamOutHalInterfaceEventCallback> event;
+        wp<StreamOutHalInterfaceLatencyModeCallback> latency;
+    };
     using Patches = std::map<int32_t /*patch ID*/,
             ::aidl::android::hardware::audio::core::AudioPatch>;
     using PortConfigs = std::map<int32_t /*port config ID*/,
@@ -195,6 +230,21 @@
     void resetPatch(int32_t patchId);
     void resetPortConfig(int32_t portConfigId);
 
+    // CallbackBroker implementation
+    void clearCallbacks(void* cookie) override;
+    sp<StreamOutHalInterfaceCallback> getStreamOutCallback(void* cookie) override;
+    void setStreamOutCallback(void* cookie, const sp<StreamOutHalInterfaceCallback>& cb) override;
+    sp<StreamOutHalInterfaceEventCallback> getStreamOutEventCallback(void* cookie) override;
+    void setStreamOutEventCallback(void* cookie,
+            const sp<StreamOutHalInterfaceEventCallback>& cb) override;
+    sp<StreamOutHalInterfaceLatencyModeCallback> getStreamOutLatencyModeCallback(
+            void* cookie) override;
+    void setStreamOutLatencyModeCallback(
+            void* cookie, const sp<StreamOutHalInterfaceLatencyModeCallback>& cb) override;
+    // Implementation helpers.
+    template<class C> sp<C> getCallbackImpl(void* cookie, wp<C> Callbacks::* field);
+    template<class C> void setCallbackImpl(void* cookie, wp<C> Callbacks::* field, const sp<C>& cb);
+
     const std::string mInstance;
     const std::shared_ptr<::aidl::android::hardware::audio::core::IModule> mModule;
     std::shared_ptr<::aidl::android::hardware::audio::core::sounddose::ISoundDose>
@@ -204,7 +254,9 @@
     int32_t mDefaultOutputPortId;
     PortConfigs mPortConfigs;
     Patches mPatches;
-    std::map<audio_patch_handle_t, int32_t /* patch ID */> mFwkHandles;
+    std::map<audio_patch_handle_t, int32_t /*patch ID*/> mFwkHandles;
+    std::mutex mLock;
+    std::map<void*, Callbacks> mCallbacks GUARDED_BY(mLock);
 };
 
 } // namespace android
diff --git a/media/libaudiohal/impl/StreamHalAidl.cpp b/media/libaudiohal/impl/StreamHalAidl.cpp
index 2b85f97..a97bd02 100644
--- a/media/libaudiohal/impl/StreamHalAidl.cpp
+++ b/media/libaudiohal/impl/StreamHalAidl.cpp
@@ -20,7 +20,6 @@
 #include <algorithm>
 #include <cstdint>
 
-#include <aidl/android/hardware/audio/core/BnStreamCallback.h>
 #include <audio_utils/clock.h>
 #include <mediautils/TimeCheck.h>
 #include <utils/Log.h>
@@ -448,43 +447,18 @@
     return OK;
 }
 
-namespace {
-
-/* Notes on callback ownership.
-
-This is how Binder ownership model looks like. The server implementation
-is owned by Binder framework (via sp<>). Proxies are owned by clients.
-When the last proxy disappears, Binder framework releases the server impl.
-
-Thus, it is not needed to keep any references to StreamCallback (this is
-the server impl) -- it will live as long as HAL server holds a strong ref to
-IStreamCallback proxy.
-
-The callback only keeps a weak reference to the stream. The stream is owned
-by AudioFlinger.
-
-*/
-
-class StreamCallback : public ::aidl::android::hardware::audio::core::BnStreamCallback {
-    ndk::ScopedAStatus onTransferReady() override {
-        return ndk::ScopedAStatus::ok();
-    }
-    ndk::ScopedAStatus onError() override {
-        return ndk::ScopedAStatus::ok();
-    }
-    ndk::ScopedAStatus onDrainReady() override {
-        return ndk::ScopedAStatus::ok();
-    }
-};
-
-}  // namespace
-
 StreamOutHalAidl::StreamOutHalAidl(
         const audio_config& config, StreamContextAidl&& context, int32_t nominalLatency,
-        const std::shared_ptr<IStreamOut>& stream)
+        const std::shared_ptr<IStreamOut>& stream, const sp<CallbackBroker>& callbackBroker)
         : StreamHalAidl("StreamOutHalAidl", false /*isInput*/, config, nominalLatency,
                 std::move(context), getStreamCommon(stream)),
-          mStream(stream) {}
+          mStream(stream), mCallbackBroker(callbackBroker) {}
+
+StreamOutHalAidl::~StreamOutHalAidl() {
+    if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
+        broker->clearCallbacks(this);
+    }
+}
 
 status_t StreamOutHalAidl::getLatency(uint32_t *latency) {
     return StreamHalAidl::getLatency(latency);
@@ -529,10 +503,18 @@
     return INVALID_OPERATION;
 }
 
-status_t StreamOutHalAidl::setCallback(wp<StreamOutHalInterfaceCallback> callback __unused) {
+status_t StreamOutHalAidl::setCallback(wp<StreamOutHalInterfaceCallback> callback) {
     TIME_CHECK();
     if (!mStream) return NO_INIT;
-    ALOGE("%s not implemented yet", __func__);
+    if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
+        if (auto cb = callback.promote(); cb != nullptr) {
+            broker->setStreamOutCallback(this, cb);
+        } else {
+            // It is expected that the framework never passes a null pointer.
+            // In the AIDL model callbacks can't be "unregistered".
+            LOG_ALWAYS_FATAL("%s: received an expired or null callback pointer", __func__);
+        }
+    }
     return OK;
 }
 
@@ -639,23 +621,15 @@
 }
 
 status_t StreamOutHalAidl::setEventCallback(
-        const sp<StreamOutHalInterfaceEventCallback>& callback __unused) {
+        const sp<StreamOutHalInterfaceEventCallback>& callback) {
     TIME_CHECK();
     if (!mStream) return NO_INIT;
-    ALOGE("%s not implemented yet", __func__);
+    if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
+        broker->setStreamOutEventCallback(this, callback);
+    }
     return OK;
 }
 
-namespace {
-
-struct StreamOutEventCallback {
-    StreamOutEventCallback(const wp<StreamOutHalAidl>& stream) : mStream(stream) {}
-  private:
-    wp<StreamOutHalAidl> mStream;
-};
-
-}  // namespace
-
 status_t StreamOutHalAidl::setLatencyMode(audio_latency_mode_t mode __unused) {
     TIME_CHECK();
     if (!mStream) return NO_INIT;
@@ -672,48 +646,15 @@
 };
 
 status_t StreamOutHalAidl::setLatencyModeCallback(
-        const sp<StreamOutHalInterfaceLatencyModeCallback>& callback __unused) {
+        const sp<StreamOutHalInterfaceLatencyModeCallback>& callback) {
     TIME_CHECK();
     if (!mStream) return NO_INIT;
-    ALOGE("%s not implemented yet", __func__);
+    if (auto broker = mCallbackBroker.promote(); broker != nullptr) {
+        broker->setStreamOutLatencyModeCallback(this, callback);
+    }
     return OK;
 };
 
-void StreamOutHalAidl::onWriteReady() {
-    sp<StreamOutHalInterfaceCallback> callback = mCallback.load().promote();
-    if (callback == 0) return;
-    ALOGV("asyncCallback onWriteReady");
-    callback->onWriteReady();
-}
-
-void StreamOutHalAidl::onDrainReady() {
-    sp<StreamOutHalInterfaceCallback> callback = mCallback.load().promote();
-    if (callback == 0) return;
-    ALOGV("asyncCallback onDrainReady");
-    callback->onDrainReady();
-}
-
-void StreamOutHalAidl::onError() {
-    sp<StreamOutHalInterfaceCallback> callback = mCallback.load().promote();
-    if (callback == 0) return;
-    ALOGV("asyncCallback onError");
-    callback->onError();
-}
-
-void StreamOutHalAidl::onCodecFormatChanged(const std::basic_string<uint8_t>& metadataBs __unused) {
-    sp<StreamOutHalInterfaceEventCallback> callback = mEventCallback.load().promote();
-    if (callback == nullptr) return;
-    ALOGV("asyncCodecFormatCallback %s", __func__);
-    callback->onCodecFormatChanged(metadataBs);
-}
-
-void StreamOutHalAidl::onRecommendedLatencyModeChanged(
-        const std::vector<audio_latency_mode_t>& modes __unused) {
-    sp<StreamOutHalInterfaceLatencyModeCallback> callback = mLatencyModeCallback.load().promote();
-    if (callback == nullptr) return;
-    callback->onRecommendedLatencyModeChanged(modes);
-}
-
 status_t StreamOutHalAidl::exit() {
     return StreamHalAidl::exit();
 }
diff --git a/media/libaudiohal/impl/StreamHalAidl.h b/media/libaudiohal/impl/StreamHalAidl.h
index ce6c31c..162c7bc 100644
--- a/media/libaudiohal/impl/StreamHalAidl.h
+++ b/media/libaudiohal/impl/StreamHalAidl.h
@@ -27,7 +27,6 @@
 #include <fmq/AidlMessageQueue.h>
 #include <media/audiohal/EffectHalInterface.h>
 #include <media/audiohal/StreamHalInterface.h>
-#include <mediautils/Synchronization.h>
 
 #include "ConversionHelperAidl.h"
 #include "StreamPowerLog.h"
@@ -221,6 +220,8 @@
     std::atomic<pid_t> mWorkerTid = -1;
 };
 
+class CallbackBroker;
+
 class StreamOutHalAidl : public StreamOutHalInterface, public StreamHalAidl {
   public:
     // Return the audio hardware driver estimated latency in milliseconds.
@@ -294,33 +295,21 @@
     status_t setLatencyModeCallback(
             const sp<StreamOutHalInterfaceLatencyModeCallback>& callback) override;
 
-    void onRecommendedLatencyModeChanged(const std::vector<audio_latency_mode_t>& modes);
-
     status_t exit() override;
 
-    void onCodecFormatChanged(const std::basic_string<uint8_t>& metadataBs);
-
-    // Methods used by StreamOutCallback ().
-    // FIXME: Consider the required visibility.
-    void onWriteReady();
-    void onDrainReady();
-    void onError();
-
   private:
     friend class sp<StreamOutHalAidl>;
 
-    mediautils::atomic_wp<StreamOutHalInterfaceCallback> mCallback;
-    mediautils::atomic_wp<StreamOutHalInterfaceEventCallback> mEventCallback;
-    mediautils::atomic_wp<StreamOutHalInterfaceLatencyModeCallback> mLatencyModeCallback;
-
     const std::shared_ptr<::aidl::android::hardware::audio::core::IStreamOut> mStream;
+    const wp<CallbackBroker> mCallbackBroker;
 
     // Can not be constructed directly by clients.
     StreamOutHalAidl(
             const audio_config& config, StreamContextAidl&& context, int32_t nominalLatency,
-            const std::shared_ptr<::aidl::android::hardware::audio::core::IStreamOut>& stream);
+            const std::shared_ptr<::aidl::android::hardware::audio::core::IStreamOut>& stream,
+            const sp<CallbackBroker>& callbackBroker);
 
-    ~StreamOutHalAidl() override = default;
+    ~StreamOutHalAidl() override;
 };
 
 class StreamInHalAidl : public StreamInHalInterface, public StreamHalAidl {
diff --git a/media/libaudiohal/include/media/audiohal/DeviceHalInterface.h b/media/libaudiohal/include/media/audiohal/DeviceHalInterface.h
index d5a1a60..2df2f5d 100644
--- a/media/libaudiohal/include/media/audiohal/DeviceHalInterface.h
+++ b/media/libaudiohal/include/media/audiohal/DeviceHalInterface.h
@@ -35,7 +35,7 @@
 class StreamInHalInterface;
 class StreamOutHalInterface;
 
-class DeviceHalInterface : public RefBase
+class DeviceHalInterface : public virtual RefBase
 {
   public:
     // Sets the value of 'devices' to a bitmask of 1 or more values of audio_devices_t.
diff --git a/media/libaudiohal/include/media/audiohal/StreamHalInterface.h b/media/libaudiohal/include/media/audiohal/StreamHalInterface.h
index 1d52b7d..a651d9b 100644
--- a/media/libaudiohal/include/media/audiohal/StreamHalInterface.h
+++ b/media/libaudiohal/include/media/audiohal/StreamHalInterface.h
@@ -110,8 +110,8 @@
     virtual void onError() {}
 
   protected:
-    StreamOutHalInterfaceCallback() {}
-    virtual ~StreamOutHalInterfaceCallback() {}
+    StreamOutHalInterfaceCallback() = default;
+    virtual ~StreamOutHalInterfaceCallback() = default;
 };
 
 class StreamOutHalInterfaceEventCallback : public virtual RefBase {
@@ -119,8 +119,8 @@
     virtual void onCodecFormatChanged(const std::basic_string<uint8_t>& metadataBs) = 0;
 
 protected:
-    StreamOutHalInterfaceEventCallback() {}
-    virtual ~StreamOutHalInterfaceEventCallback() {}
+    StreamOutHalInterfaceEventCallback() = default;
+    virtual ~StreamOutHalInterfaceEventCallback() = default;
 };
 
 class StreamOutHalInterfaceLatencyModeCallback : public virtual RefBase {
@@ -131,8 +131,8 @@
     virtual void onRecommendedLatencyModeChanged(std::vector<audio_latency_mode_t> modes) = 0;
 
 protected:
-    StreamOutHalInterfaceLatencyModeCallback() {}
-    virtual ~StreamOutHalInterfaceLatencyModeCallback() {}
+    StreamOutHalInterfaceLatencyModeCallback() = default;
+    virtual ~StreamOutHalInterfaceLatencyModeCallback() = default;
 };
 
 class StreamOutHalInterface : public virtual StreamHalInterface {