From 822f8d20f0b92537a580d6f25d6c7056dd51212f Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Tue, 3 Sep 2024 15:39:23 -0700
Subject: [PATCH] audio: Fix observable/hardware position check in VTS

Unify logic for checking the observable and hardware
position returned by the HAL. Fix the problem that
the reported hardware position was not checked for
being `UNKNOWN`.

Bug: 363139283
Test: atest VtsHalAudioCoreTargetTest
Change-Id: Id3beab1d1401a9608ad7e604a800ae19873270bf
---
 .../vts/VtsHalAudioCoreModuleTargetTest.cpp   | 54 +++++++++----------
 1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
index bbc4caf79e..8d430de53c 100644
--- a/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
+++ b/audio/aidl/vts/VtsHalAudioCoreModuleTargetTest.cpp
@@ -2975,15 +2975,15 @@ class StreamLogicDefaultDriver : public StreamLogicDriver {
 
     // The five methods below is intended to be called after the worker
     // thread has joined, thus no extra synchronization is needed.
-    bool hasObservablePositionIncrease() const { return mObservablePositionIncrease; }
-    bool hasObservableRetrogradePosition() const { return mRetrogradeObservablePosition; }
+    bool hasObservablePositionIncrease() const { return mObservable.hasPositionIncrease; }
+    bool hasObservableRetrogradePosition() const { return mObservable.hasRetrogradePosition; }
     bool hasHardwarePositionIncrease() const {
         // For non-MMap, always return true to pass the validation.
-        return mIsMmap ? mHardwarePositionIncrease : true;
+        return mIsMmap ? mHardware.hasPositionIncrease : true;
     }
     bool hasHardwareRetrogradePosition() const {
         // For non-MMap, always return false to pass the validation.
-        return mIsMmap ? mRetrogradeHardwarePosition : false;
+        return mIsMmap ? mHardware.hasRetrogradePosition : false;
     }
     std::string getUnexpectedStateTransition() const { return mUnexpectedTransition; }
 
@@ -3011,25 +3011,9 @@ class StreamLogicDefaultDriver : public StreamLogicDriver {
     }
     bool interceptRawReply(const StreamDescriptor::Reply&) override { return false; }
     bool processValidReply(const StreamDescriptor::Reply& reply) override {
-        if (reply.observable.frames != StreamDescriptor::Position::UNKNOWN) {
-            if (mPreviousObservableFrames.has_value()) {
-                if (reply.observable.frames > mPreviousObservableFrames.value()) {
-                    mObservablePositionIncrease = true;
-                } else if (reply.observable.frames < mPreviousObservableFrames.value()) {
-                    mRetrogradeObservablePosition = true;
-                }
-            }
-            mPreviousObservableFrames = reply.observable.frames;
-        }
+        mObservable.update(reply.observable.frames);
         if (mIsMmap) {
-            if (mPreviousHardwareFrames.has_value()) {
-                if (reply.hardware.frames > mPreviousHardwareFrames.value()) {
-                    mHardwarePositionIncrease = true;
-                } else if (reply.hardware.frames < mPreviousHardwareFrames.value()) {
-                    mRetrogradeHardwarePosition = true;
-                }
-            }
-            mPreviousHardwareFrames = reply.hardware.frames;
+            mHardware.update(reply.hardware.frames);
         }
 
         auto expected = mCommands->getExpectedStates();
@@ -3054,16 +3038,30 @@ class StreamLogicDefaultDriver : public StreamLogicDriver {
     }
 
   protected:
+    struct FramesCounter {
+        std::optional<int64_t> previous;
+        bool hasPositionIncrease = false;
+        bool hasRetrogradePosition = false;
+
+        void update(int64_t position) {
+            if (position == StreamDescriptor::Position::UNKNOWN) return;
+            if (previous.has_value()) {
+                if (position > previous.value()) {
+                    hasPositionIncrease = true;
+                } else if (position < previous.value()) {
+                    hasRetrogradePosition = true;
+                }
+            }
+            previous = position;
+        }
+    };
+
     std::shared_ptr<StateSequence> mCommands;
     const size_t mFrameSizeBytes;
     const bool mIsMmap;
     std::optional<StreamDescriptor::State> mPreviousState;
-    std::optional<int64_t> mPreviousObservableFrames;
-    bool mObservablePositionIncrease = false;
-    bool mRetrogradeObservablePosition = false;
-    std::optional<int64_t> mPreviousHardwareFrames;
-    bool mHardwarePositionIncrease = false;
-    bool mRetrogradeHardwarePosition = false;
+    FramesCounter mObservable;
+    FramesCounter mHardware;
     std::string mUnexpectedTransition;
 };