From b02341aa909b25ec9596f2d585e35594469a1c01 Mon Sep 17 00:00:00 2001
From: Maxime Fournier <mfournier@baylibre.com>
Date: Wed, 27 Aug 2025 14:07:33 +0200
Subject: [PATCH] android: hal: implement signalStreamFlush with race condition
 handling

Add proper implementation of signalStreamFlush() to handle stream buffer
flushing before reconfiguration. Features:
- Stream configuration counter to prevent race conditions
- Validation of stream IDs against configured streams
- Inflight buffer tracking and reporting for affected streams
- Warning when buffers are detected to prevent fatal errors

Increments mStreamConfigCounter during configureStreams() and uses atomic
operations to detect stale signalStreamFlush() calls, ensuring proper
synchronization during stream reconfiguration.

Further details about the race condition scenario can be found at [1].

[1] https://android.googlesource.com/platform/hardware/interfaces/+/master/camera/device/aidl/android/hardware/camera/device/ICameraDeviceSession.aidl#489

Signed-off-by: Maxime Fournier <mfournier@baylibre.com>
---
 .../hal/device/LibcameraDeviceSession.cpp     | 68 ++++++++++++++++++-
 .../hal/device/LibcameraDeviceSession.h       |  4 ++
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/src/android/hal/device/LibcameraDeviceSession.cpp b/src/android/hal/device/LibcameraDeviceSession.cpp
index b1397900..a2e7b731 100644
--- a/src/android/hal/device/LibcameraDeviceSession.cpp
+++ b/src/android/hal/device/LibcameraDeviceSession.cpp
@@ -1092,6 +1092,8 @@ ScopedAStatus LibcameraDeviceSession::configureStreams(
             convertToAidl(static_cast<Camera3Stream*>(streams[i]), &out[i]);
         }
         mFirstRequest = true;
+        // Increment stream configuration counter for signalStreamFlush race condition handling
+        mStreamConfigCounter++;
     }
     return fromStatus(status);
 }
@@ -1710,8 +1712,70 @@ ScopedAStatus LibcameraDeviceSession::isReconfigurationRequired(
 }
 ScopedAStatus LibcameraDeviceSession::signalStreamFlush(const std::vector<int32_t>& in_streamIds,
                                                         int32_t in_streamConfigCounter) {
-    ALOGI("%s()", __func__);
-    return fromStatus(Status::INTERNAL_ERROR);
+    ALOGV("%s: streamIds size %zu, configCounter %d", __FUNCTION__,
+          in_streamIds.size(), in_streamConfigCounter);
+
+    Status status = initStatus();
+    if (status != Status::OK) {
+        ALOGE("%s: camera init failed or disconnected", __FUNCTION__);
+        return fromStatus(status);
+    }
+
+    // Check for race condition - if the counter is less than our last configureStreams,
+    // this call is stale and should be ignored
+    int32_t currentCounter = mStreamConfigCounter.load();
+    if (in_streamConfigCounter < currentCounter) {
+        ALOGD("%s: Stale signalStreamFlush call (counter %d < current %d), ignoring",
+              __FUNCTION__, in_streamConfigCounter, currentCounter);
+        return fromStatus(Status::OK);
+    }
+
+    // Validate stream IDs
+    for (int32_t streamId : in_streamIds) {
+        if (mStreamMap.find(streamId) == mStreamMap.end()) {
+            ALOGE("%s: Invalid stream ID %d", __FUNCTION__, streamId);
+            return fromStatus(Status::ILLEGAL_ARGUMENT);
+        }
+    }
+
+    ALOGD("%s: Flushing buffers for %zu streams before reconfiguration",
+          __FUNCTION__, in_streamIds.size());
+
+    // Check current inflight buffers for the specified streams
+    {
+        Mutex::Autolock _l(mInflightLock);
+
+        size_t totalInflightBuffers = 0;
+        for (int32_t streamId : in_streamIds) {
+            size_t streamInflightCount = 0;
+            for (const auto& pair : mInflightBuffers) {
+                if (pair.first.first == streamId) {
+                    streamInflightCount++;
+                }
+            }
+
+            if (streamInflightCount > 0) {
+                ALOGD("%s: Stream %d has %zu inflight buffers",
+                      __FUNCTION__, streamId, streamInflightCount);
+                totalInflightBuffers += streamInflightCount;
+            }
+        }
+
+        if (totalInflightBuffers > 0) {
+            ALOGW("%s: Total %zu inflight buffers detected for streams being flushed. "
+                  "HAL should return these buffers promptly to avoid fatal errors.",
+                  __FUNCTION__, totalInflightBuffers);
+        } else {
+            ALOGV("%s: No inflight buffers found for specified streams", __FUNCTION__);
+        }
+    }
+
+    // This is mainly a hint/notification from camera service
+    // The actual buffer return will happen through normal capture request completion
+    // or error callbacks. We just log the request and let the system know we received it.
+
+    ALOGV("%s: Stream flush signal processed successfully", __FUNCTION__);
+    return fromStatus(Status::OK);
 }
 ScopedAStatus LibcameraDeviceSession::switchToOffline(
         const std::vector<int32_t>& in_streamsToKeep,
diff --git a/src/android/hal/device/LibcameraDeviceSession.h b/src/android/hal/device/LibcameraDeviceSession.h
index 8d64ca33..f2af9fdf 100644
--- a/src/android/hal/device/LibcameraDeviceSession.h
+++ b/src/android/hal/device/LibcameraDeviceSession.h
@@ -25,6 +25,7 @@
 
 #include "convert.h"
 
+#include <atomic>
 #include <deque>
 #include <map>
 #include <unordered_map>
@@ -158,6 +159,9 @@ protected:
     // Stream ID -> Camera3Stream cache
     std::map<int, Camera3Stream> mStreamMap;
 
+    // Stream configuration counter to handle race conditions in signalStreamFlush
+    std::atomic<int32_t> mStreamConfigCounter{0};
+
     mutable Mutex mInflightLock; // protecting mInflightBuffers and mCirculatingBuffers
     // (streamID, frameNumber) -> inflight buffer cache
     std::map<std::pair<int, uint32_t>, camera3_stream_buffer_t>  mInflightBuffers;