SyncEvent: modernize C++

And add unit test mediasyncevent_tests.

Test: atest mediasyncevent_tests
Bug: 283021652
Merged-In: I37711f4271c68042f178db9370549c0c3260ace8
Change-Id: I37711f4271c68042f178db9370549c0c3260ace8
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 2550672..d055dd2 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1236,18 +1236,19 @@
         }
 
         // Look for sync events awaiting for a session to be used.
-        for (size_t i = 0; i < mPendingSyncEvents.size(); i++) {
-            if (mPendingSyncEvents[i]->triggerSession() == sessionId) {
-                if (thread->isValidSyncEvent(mPendingSyncEvents[i])) {
+        for (auto it = mPendingSyncEvents.begin(); it != mPendingSyncEvents.end();) {
+            if ((*it)->triggerSession() == sessionId) {
+                if (thread->isValidSyncEvent(*it)) {
                     if (lStatus == NO_ERROR) {
-                        (void) track->setSyncEvent(mPendingSyncEvents[i]);
+                        (void) track->setSyncEvent(*it);
                     } else {
-                        mPendingSyncEvents[i]->cancel();
+                        (*it)->cancel();
                     }
-                    mPendingSyncEvents.removeAt(i);
-                    i--;
+                    it = mPendingSyncEvents.erase(it);
+                    continue;
                 }
             }
+            ++it;
         }
         if ((output.flags & AUDIO_OUTPUT_FLAG_HW_AV_SYNC) == AUDIO_OUTPUT_FLAG_HW_AV_SYNC) {
             setAudioHwSyncForSession_l(thread, sessionId);
@@ -3919,15 +3920,16 @@
     track->setTeePatches(std::move(teePatches));
 }
 
-sp<SyncEvent> AudioFlinger::createSyncEvent(AudioSystem::sync_event_t type,
+sp<audioflinger::SyncEvent> AudioFlinger::createSyncEvent(AudioSystem::sync_event_t type,
                                     audio_session_t triggerSession,
                                     audio_session_t listenerSession,
-                                    sync_event_callback_t callBack,
+                                    audioflinger::SyncEventCallback callBack,
                                     const wp<RefBase>& cookie)
 {
     Mutex::Autolock _l(mLock);
 
-    sp<SyncEvent> event = new SyncEvent(type, triggerSession, listenerSession, callBack, cookie);
+    auto event = sp<audioflinger::SyncEvent>::make(
+            type, triggerSession, listenerSession, callBack, cookie);
     status_t playStatus = NAME_NOT_FOUND;
     status_t recStatus = NAME_NOT_FOUND;
     for (size_t i = 0; i < mPlaybackThreads.size(); i++) {
@@ -3943,7 +3945,7 @@
         }
     }
     if (playStatus == NAME_NOT_FOUND || recStatus == NAME_NOT_FOUND) {
-        mPendingSyncEvents.add(event);
+        mPendingSyncEvents.emplace_back(event);
     } else {
         ALOGV("createSyncEvent() invalid event %d", event->type());
         event.clear();
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index b99b859..69fd18d 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -375,10 +375,10 @@
 
     static inline std::atomic<AudioFlinger *> gAudioFlinger = nullptr;
 
-    sp<SyncEvent> createSyncEvent(AudioSystem::sync_event_t type,
+    sp<audioflinger::SyncEvent> createSyncEvent(AudioSystem::sync_event_t type,
                                         audio_session_t triggerSession,
                                         audio_session_t listenerSession,
-                                        sync_event_callback_t callBack,
+                                        audioflinger::SyncEventCallback callBack,
                                         const wp<RefBase>& cookie);
 
     bool        btNrecIsOff() const { return mBtNrecIsOff.load(); }
@@ -939,8 +939,8 @@
                 bool        masterMute_l() const;
                 AudioHwDevice* loadHwModule_l(const char *name);
 
-                Vector < sp<SyncEvent> > mPendingSyncEvents; // sync events awaiting for a session
-                                                             // to be created
+                // sync events awaiting for a session to be created.
+                std::list<sp<audioflinger::SyncEvent>> mPendingSyncEvents;
 
                 // Effect chains without a valid thread
                 DefaultKeyedVector< audio_session_t , sp<EffectChain> > mOrphanEffectChains;
diff --git a/services/audioflinger/PlaybackTracks.h b/services/audioflinger/PlaybackTracks.h
index 33983d7..efcd832 100644
--- a/services/audioflinger/PlaybackTracks.h
+++ b/services/audioflinger/PlaybackTracks.h
@@ -131,7 +131,7 @@
 // implement FastMixerState::VolumeProvider interface
     virtual gain_minifloat_packed_t getVolumeLR();
 
-    virtual status_t    setSyncEvent(const sp<SyncEvent>& event);
+            status_t    setSyncEvent(const sp<audioflinger::SyncEvent>& event) override;
 
     virtual bool        isFastTrack() const { return (mFlags & AUDIO_OUTPUT_FLAG_FAST) != 0; }
 
diff --git a/services/audioflinger/RecordTracks.h b/services/audioflinger/RecordTracks.h
index f0a5f76..868acfc 100644
--- a/services/audioflinger/RecordTracks.h
+++ b/services/audioflinger/RecordTracks.h
@@ -58,7 +58,7 @@
             void        appendDumpHeader(String8& result);
             void        appendDump(String8& result, bool active);
 
-            void        handleSyncStartEvent(const sp<SyncEvent>& event);
+            void        handleSyncStartEvent(const sp<audioflinger::SyncEvent>& event);
             void        clearSyncStartEvent();
 
             void        updateTrackFrameInfo(int64_t trackFramesReleased,
@@ -107,7 +107,7 @@
 
             // sync event triggering actual audio capture. Frames read before this event will
             // be dropped and therefore not read by the application.
-            sp<SyncEvent>                       mSyncStartEvent;
+            sp<audioflinger::SyncEvent>        mSyncStartEvent;
 
             // number of captured frames to drop after the start sync event has been received.
             // when < 0, maximum frames to drop before starting capture even if sync event is
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 398155e..3a3d829 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -3278,7 +3278,7 @@
     return (uint32_t)((uint32_t)((mNormalFrameCount * 1000) / mSampleRate) * 1000);
 }
 
-status_t AudioFlinger::PlaybackThread::setSyncEvent(const sp<SyncEvent>& event)
+status_t AudioFlinger::PlaybackThread::setSyncEvent(const sp<audioflinger::SyncEvent>& event)
 {
     if (!isValidSyncEvent(event)) {
         return BAD_VALUE;
@@ -3297,7 +3297,8 @@
     return NAME_NOT_FOUND;
 }
 
-bool AudioFlinger::PlaybackThread::isValidSyncEvent(const sp<SyncEvent>& event) const
+bool AudioFlinger::PlaybackThread::isValidSyncEvent(
+        const sp<audioflinger::SyncEvent>& event) const
 {
     return event->type() == AudioSystem::SYNC_EVENT_PRESENTATION_COMPLETE;
 }
@@ -8680,9 +8681,9 @@
     }
 }
 
-void AudioFlinger::RecordThread::syncStartEventCallback(const wp<SyncEvent>& event)
+void AudioFlinger::RecordThread::syncStartEventCallback(const wp<audioflinger::SyncEvent>& event)
 {
-    sp<SyncEvent> strongEvent = event.promote();
+    sp<audioflinger::SyncEvent> strongEvent = event.promote();
 
     if (strongEvent != 0) {
         sp<RefBase> ptr = strongEvent->cookie().promote();
@@ -8721,12 +8722,14 @@
     return false;
 }
 
-bool AudioFlinger::RecordThread::isValidSyncEvent(const sp<SyncEvent>& event __unused) const
+bool AudioFlinger::RecordThread::isValidSyncEvent(
+        const sp<audioflinger::SyncEvent>& /* event */) const
 {
     return false;
 }
 
-status_t AudioFlinger::RecordThread::setSyncEvent(const sp<SyncEvent>& event __unused)
+status_t AudioFlinger::RecordThread::setSyncEvent(
+        const sp<audioflinger::SyncEvent>& event __unused)
 {
 #if 0   // This branch is currently dead code, but is preserved in case it will be needed in future
     if (!isValidSyncEvent(event)) {
@@ -10259,12 +10262,13 @@
     // and because it can cause a recursive mutex lock on stop().
 }
 
-status_t AudioFlinger::MmapThread::setSyncEvent(const sp<SyncEvent>& event __unused)
+status_t AudioFlinger::MmapThread::setSyncEvent(const sp<audioflinger::SyncEvent>& /* event */)
 {
     return BAD_VALUE;
 }
 
-bool AudioFlinger::MmapThread::isValidSyncEvent(const sp<SyncEvent>& event __unused) const
+bool AudioFlinger::MmapThread::isValidSyncEvent(
+        const sp<audioflinger::SyncEvent>& /* event */) const
 {
     return false;
 }
diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h
index e4fd3fc..0094a9f 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -516,8 +516,8 @@
                                                  audio_session_t sessionId,
                                                  bool threadLocked);
 
-                virtual status_t    setSyncEvent(const sp<SyncEvent>& event) = 0;
-                virtual bool        isValidSyncEvent(const sp<SyncEvent>& event) const = 0;
+                virtual status_t setSyncEvent(const sp<audioflinger::SyncEvent>& event) = 0;
+                virtual bool isValidSyncEvent(const sp<audioflinger::SyncEvent>& event) const = 0;
 
                 // Return a reference to a per-thread heap which can be used to allocate IMemory
                 // objects that will be read-only to client processes, read/write to mediaserver,
@@ -1019,8 +1019,8 @@
                 virtual product_strategy_t getStrategyForSession_l(audio_session_t sessionId);
 
 
-                virtual status_t setSyncEvent(const sp<SyncEvent>& event);
-                virtual bool     isValidSyncEvent(const sp<SyncEvent>& event) const;
+                status_t setSyncEvent(const sp<audioflinger::SyncEvent>& event) override;
+                bool     isValidSyncEvent(const sp<audioflinger::SyncEvent>& event) const override;
 
                 // called with AudioFlinger lock held
                         bool     invalidateTracks_l(audio_stream_type_t streamType);
@@ -1921,10 +1921,10 @@
             // FIXME replace by Set [and implement Bag/Multiset for other uses].
             KeyedVector<audio_session_t, bool> sessionIds() const;
 
-    virtual status_t setSyncEvent(const sp<SyncEvent>& event);
-    virtual bool     isValidSyncEvent(const sp<SyncEvent>& event) const;
+            status_t setSyncEvent(const sp<audioflinger::SyncEvent>& event) override;
+            bool     isValidSyncEvent(const sp<audioflinger::SyncEvent>& event) const override;
 
-    static void syncStartEventCallback(const wp<SyncEvent>& event);
+    static void syncStartEventCallback(const wp<audioflinger::SyncEvent>& event);
 
     virtual size_t      frameCount() const { return mFrameCount; }
             bool        hasFastCapture() const { return mFastCapture != 0; }
@@ -2127,8 +2127,8 @@
                                 // Note: using mActiveTracks as no mTracks here.
                                 return ThreadBase::hasAudioSession_l(sessionId, mActiveTracks);
                             }
-    virtual     status_t    setSyncEvent(const sp<SyncEvent>& event);
-    virtual     bool        isValidSyncEvent(const sp<SyncEvent>& event) const;
+    virtual     status_t    setSyncEvent(const sp<audioflinger::SyncEvent>& event);
+    virtual     bool        isValidSyncEvent(const sp<audioflinger::SyncEvent>& event) const;
 
     virtual     void        checkSilentMode_l() {}
     virtual     void        processVolume_l() {}
diff --git a/services/audioflinger/TrackBase.h b/services/audioflinger/TrackBase.h
index db056d2..6c42dc8 100644
--- a/services/audioflinger/TrackBase.h
+++ b/services/audioflinger/TrackBase.h
@@ -84,7 +84,7 @@
             pid_t       creatorPid() const { return mCreatorPid; }
 
             audio_port_handle_t portId() const { return mPortId; }
-    virtual status_t    setSyncEvent(const sp<SyncEvent>& event);
+    virtual status_t    setSyncEvent(const sp<audioflinger::SyncEvent>& event);
 
             sp<IMemory> getBuffers() const { return mBufferMemory; }
             void*       buffer() const { return mBuffer; }
@@ -374,7 +374,7 @@
 
     const audio_session_t mSessionId;
     uid_t               mUid;
-    Vector < sp<SyncEvent> >mSyncEvents;
+    std::list<sp<audioflinger::SyncEvent>> mSyncEvents;
     const bool          mIsOut;
     sp<ServerProxy>     mServerProxy;
     const int           mId;
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index a4f7ca4..76afcb0 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -302,9 +302,10 @@
     mServerProxy->releaseBuffer(&buf);
 }
 
-status_t AudioFlinger::ThreadBase::TrackBase::setSyncEvent(const sp<SyncEvent>& event)
+status_t AudioFlinger::ThreadBase::TrackBase::setSyncEvent(
+        const sp<audioflinger::SyncEvent>& event)
 {
-    mSyncEvents.add(event);
+    mSyncEvents.emplace_back(event);
     return NO_ERROR;
 }
 
@@ -1612,12 +1613,12 @@
 
 void AudioFlinger::PlaybackThread::Track::triggerEvents(AudioSystem::sync_event_t type)
 {
-    for (size_t i = 0; i < mSyncEvents.size();) {
-        if (mSyncEvents[i]->type() == type) {
-            mSyncEvents[i]->trigger();
-            mSyncEvents.removeAt(i);
+    for (auto it = mSyncEvents.begin(); it != mSyncEvents.end();) {
+        if ((*it)->type() == type) {
+            (*it)->trigger();
+            it = mSyncEvents.erase(it);
         } else {
-            ++i;
+            ++it;
         }
     }
 }
@@ -1649,7 +1650,8 @@
     return vlr;
 }
 
-status_t AudioFlinger::PlaybackThread::Track::setSyncEvent(const sp<SyncEvent>& event)
+status_t AudioFlinger::PlaybackThread::Track::setSyncEvent(
+        const sp<audioflinger::SyncEvent>& event)
 {
     if (isTerminated() || mState == PAUSED ||
             ((framesReady() == 0) && ((mSharedBuffer != 0) ||
@@ -2599,7 +2601,8 @@
     result.append("\n");
 }
 
-void AudioFlinger::RecordThread::RecordTrack::handleSyncStartEvent(const sp<SyncEvent>& event)
+void AudioFlinger::RecordThread::RecordTrack::handleSyncStartEvent(
+        const sp<audioflinger::SyncEvent>& event)
 {
     if (event == mSyncStartEvent) {
         ssize_t framesToDrop = 0;
diff --git a/services/audioflinger/timing/Android.bp b/services/audioflinger/timing/Android.bp
index 17ce8bd..269f796 100644
--- a/services/audioflinger/timing/Android.bp
+++ b/services/audioflinger/timing/Android.bp
@@ -10,6 +10,10 @@
 cc_library {
     name: "libaudioflinger_timing",
 
+    defaults: [
+        "audioflinger_flags_defaults",
+    ],
+
     host_supported: true,
 
     srcs: [
diff --git a/services/audioflinger/timing/SyncEvent.h b/services/audioflinger/timing/SyncEvent.h
index 9912580..b5a3b40 100644
--- a/services/audioflinger/timing/SyncEvent.h
+++ b/services/audioflinger/timing/SyncEvent.h
@@ -16,43 +16,55 @@
 
 #pragma once
 
-namespace android {
+#include <functional>
+#include <mutex>
+
+#include <media/AudioSystem.h>
+#include <utils/RefBase.h>
+
+namespace android::audioflinger {
 
 class SyncEvent;
-
-typedef void (*sync_event_callback_t)(const wp<SyncEvent>& event) ;
+using SyncEventCallback = std::function<void(const wp<SyncEvent>& event)>;
 
 class SyncEvent : public RefBase {
 public:
     SyncEvent(AudioSystem::sync_event_t type,
               audio_session_t triggerSession,
               audio_session_t listenerSession,
-              sync_event_callback_t callBack,
+              const SyncEventCallback& callBack,
               const wp<RefBase>& cookie)
     : mType(type), mTriggerSession(triggerSession), mListenerSession(listenerSession),
-      mCallback(callBack), mCookie(cookie)
+      mCookie(cookie), mCallback(callBack)
     {}
 
-    virtual ~SyncEvent() {}
-
     void trigger() {
-        Mutex::Autolock _l(mLock);
-        if (mCallback) mCallback(wp<SyncEvent>(this));
+        std::lock_guard l(mLock);
+        if (mCallback) mCallback(wp<SyncEvent>::fromExisting(this));
     }
-    bool isCancelled() const { Mutex::Autolock _l(mLock); return (mCallback == NULL); }
-    void cancel() { Mutex::Autolock _l(mLock); mCallback = NULL; }
+
+    bool isCancelled() const {
+        std::lock_guard l(mLock);
+        return mCallback == nullptr;
+    }
+
+    void cancel() {
+        std::lock_guard l(mLock);
+        mCallback = nullptr;
+    }
+
     AudioSystem::sync_event_t type() const { return mType; }
     audio_session_t triggerSession() const { return mTriggerSession; }
     audio_session_t listenerSession() const { return mListenerSession; }
-    wp<RefBase> cookie() const { return mCookie; }
+    const wp<RefBase>& cookie() const { return mCookie; }
 
 private:
       const AudioSystem::sync_event_t mType;
       const audio_session_t mTriggerSession;
       const audio_session_t mListenerSession;
-      sync_event_callback_t mCallback;
       const wp<RefBase> mCookie;
-      mutable Mutex mLock;
+      mutable std::mutex mLock;
+      SyncEventCallback mCallback GUARDED_BY(mLock);
 };
 
-} // namespace android
+} // namespace android::audioflinger
diff --git a/services/audioflinger/timing/tests/Android.bp b/services/audioflinger/timing/tests/Android.bp
index 29267a6..c360799 100644
--- a/services/audioflinger/timing/tests/Android.bp
+++ b/services/audioflinger/timing/tests/Android.bp
@@ -8,6 +8,31 @@
 }
 
 cc_test {
+    name: "mediasyncevent_tests",
+
+    host_supported: true,
+
+    srcs: [
+        "mediasyncevent_tests.cpp"
+    ],
+
+    header_libs: [
+        "libaudioclient_headers",
+    ],
+
+    static_libs: [
+        "liblog",
+        "libutils", // RefBase
+    ],
+
+    cflags: [
+        "-Wall",
+        "-Werror",
+        "-Wextra",
+    ],
+}
+
+cc_test {
     name: "monotonicframecounter_tests",
 
     host_supported: true,
diff --git a/services/audioflinger/timing/tests/mediasyncevent_tests.cpp b/services/audioflinger/timing/tests/mediasyncevent_tests.cpp
new file mode 100644
index 0000000..2922d90
--- /dev/null
+++ b/services/audioflinger/timing/tests/mediasyncevent_tests.cpp
@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// #define LOG_NDEBUG 0
+#define LOG_TAG "mediasyncevent_tests"
+
+#include "../SyncEvent.h"
+
+#include <gtest/gtest.h>
+
+using namespace android;
+using namespace android::audioflinger;
+
+namespace {
+
+TEST(MediaSyncEventTests, Basic) {
+    struct Cookie : public RefBase {};
+
+    // These variables are set by trigger().
+    bool triggered = false;
+    wp<SyncEvent> param;
+
+    constexpr auto type = AudioSystem::SYNC_EVENT_PRESENTATION_COMPLETE;
+    constexpr auto triggerSession = audio_session_t(10);
+    constexpr auto listenerSession = audio_session_t(11);
+    const SyncEventCallback callback =
+            [&](const wp<SyncEvent>& event) {
+                triggered = true;
+                param = event;
+            };
+    const auto cookie = sp<Cookie>::make();
+
+    // Since the callback uses a weak pointer to this,
+    // don't allocate on the stack.
+    auto syncEvent = sp<SyncEvent>::make(
+            type,
+            triggerSession,
+            listenerSession,
+            callback,
+            cookie);
+
+    ASSERT_EQ(type, syncEvent->type());
+    ASSERT_EQ(triggerSession, syncEvent->triggerSession());
+    ASSERT_EQ(listenerSession, syncEvent->listenerSession());
+    ASSERT_EQ(cookie, syncEvent->cookie());
+    ASSERT_FALSE(triggered);
+
+    syncEvent->trigger();
+    ASSERT_TRUE(triggered);
+    ASSERT_EQ(param, syncEvent);
+
+    ASSERT_FALSE(syncEvent->isCancelled());
+    syncEvent->cancel();
+    ASSERT_TRUE(syncEvent->isCancelled());
+}
+
+} // namespace