Camera: improve error handling when HAL crash
Return buffers managed by HAL buffer manager in disconnect.
Test: kill HAL process and check for buffer leak in cameraserver
Bug: 126889012
Change-Id: I83173c5eaae13ee11eb3f185e7204a2dd8855b4e
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 923d17a..bc76ada 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -58,6 +58,8 @@
#include "CameraService.h"
#include "utils/CameraThreadState.h"
+#include <tuple>
+
using namespace android::camera3;
using namespace android::hardware::camera;
using namespace android::hardware::camera::device::V3_2;
@@ -1094,7 +1096,7 @@
hBuf.acquireFence.setTo(acquireFence, /*shouldOwn*/true);
hBuf.releaseFence = nullptr;
- res = mInterface->pushInflightRequestBuffer(bufferId, buffer);
+ res = mInterface->pushInflightRequestBuffer(bufferId, buffer, streamId);
if (res != OK) {
ALOGE("%s: Can't get register request buffers for stream %d: %s (%d)",
__FUNCTION__, streamId, strerror(-res), res);
@@ -3270,7 +3272,15 @@
std::vector<std::pair<int32_t, int32_t>> inflightKeys;
mInterface->getInflightBufferKeys(&inflightKeys);
- int32_t inputStreamId = (mInputStream != nullptr) ? mInputStream->getId() : -1;
+ // Inflight buffers for HAL buffer manager
+ std::vector<uint64_t> inflightRequestBufferKeys;
+ mInterface->getInflightRequestBufferKeys(&inflightRequestBufferKeys);
+
+ // (streamId, frameNumber, buffer_handle_t*) tuple for all inflight buffers.
+ // frameNumber will be -1 for buffers from HAL buffer manager
+ std::vector<std::tuple<int32_t, int32_t, buffer_handle_t*>> inflightBuffers;
+ inflightBuffers.reserve(inflightKeys.size() + inflightRequestBufferKeys.size());
+
for (auto& pair : inflightKeys) {
int32_t frameNumber = pair.first;
int32_t streamId = pair.second;
@@ -3281,6 +3291,26 @@
__FUNCTION__, frameNumber, streamId);
continue;
}
+ inflightBuffers.push_back(std::make_tuple(streamId, frameNumber, buffer));
+ }
+
+ for (auto& bufferId : inflightRequestBufferKeys) {
+ int32_t streamId = -1;
+ buffer_handle_t* buffer = nullptr;
+ status_t res = mInterface->popInflightRequestBuffer(bufferId, &buffer, &streamId);
+ if (res != OK) {
+ ALOGE("%s: cannot find in-flight buffer %" PRIu64, __FUNCTION__, bufferId);
+ continue;
+ }
+ inflightBuffers.push_back(std::make_tuple(streamId, /*frameNumber*/-1, buffer));
+ }
+
+ int32_t inputStreamId = (mInputStream != nullptr) ? mInputStream->getId() : -1;
+ for (auto& tuple : inflightBuffers) {
+ status_t res = OK;
+ int32_t streamId = std::get<0>(tuple);
+ int32_t frameNumber = std::get<1>(tuple);
+ buffer_handle_t* buffer = std::get<2>(tuple);
camera3_stream_buffer_t streamBuffer;
streamBuffer.buffer = buffer;
@@ -4583,6 +4613,17 @@
return;
}
+void Camera3Device::HalInterface::getInflightRequestBufferKeys(
+ std::vector<uint64_t>* out) {
+ std::lock_guard<std::mutex> lock(mRequestedBuffersLock);
+ out->clear();
+ out->reserve(mRequestedBuffers.size());
+ for (auto& pair : mRequestedBuffers) {
+ out->push_back(pair.first);
+ }
+ return;
+}
+
status_t Camera3Device::HalInterface::pushInflightBufferLocked(
int32_t frameNumber, int32_t streamId, buffer_handle_t *buffer, int acquireFence) {
uint64_t key = static_cast<uint64_t>(frameNumber) << 32 | static_cast<uint64_t>(streamId);
@@ -4610,9 +4651,9 @@
}
status_t Camera3Device::HalInterface::pushInflightRequestBuffer(
- uint64_t bufferId, buffer_handle_t* buf) {
+ uint64_t bufferId, buffer_handle_t* buf, int32_t streamId) {
std::lock_guard<std::mutex> lock(mRequestedBuffersLock);
- auto pair = mRequestedBuffers.insert({bufferId, buf});
+ auto pair = mRequestedBuffers.insert({bufferId, {streamId, buf}});
if (!pair.second) {
ALOGE("%s: bufId %" PRIu64 " is already inflight!",
__FUNCTION__, bufferId);
@@ -4623,7 +4664,13 @@
// Find and pop a buffer_handle_t based on bufferId
status_t Camera3Device::HalInterface::popInflightRequestBuffer(
- uint64_t bufferId, /*out*/ buffer_handle_t **buffer) {
+ uint64_t bufferId,
+ /*out*/ buffer_handle_t** buffer,
+ /*optional out*/ int32_t* streamId) {
+ if (buffer == nullptr) {
+ ALOGE("%s: buffer (%p) must not be null", __FUNCTION__, buffer);
+ return BAD_VALUE;
+ }
std::lock_guard<std::mutex> lock(mRequestedBuffersLock);
auto it = mRequestedBuffers.find(bufferId);
if (it == mRequestedBuffers.end()) {
@@ -4631,7 +4678,10 @@
__FUNCTION__, bufferId);
return BAD_VALUE;
}
- *buffer = it->second;
+ *buffer = it->second.second;
+ if (streamId != nullptr) {
+ *streamId = it->second.first;
+ }
mRequestedBuffers.erase(it);
return OK;
}
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index b25d89d..d3bb212 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -320,16 +320,22 @@
status_t popInflightBuffer(int32_t frameNumber, int32_t streamId,
/*out*/ buffer_handle_t **buffer);
- // Register a bufId/buffer_handle_t to inflight request buffer
- status_t pushInflightRequestBuffer(uint64_t bufferId, buffer_handle_t* buf);
+ // Register a bufId (streamId, buffer_handle_t) to inflight request buffer
+ status_t pushInflightRequestBuffer(
+ uint64_t bufferId, buffer_handle_t* buf, int32_t streamId);
// Find a buffer_handle_t based on bufferId
- status_t popInflightRequestBuffer(uint64_t bufferId, /*out*/ buffer_handle_t **buffer);
+ status_t popInflightRequestBuffer(uint64_t bufferId,
+ /*out*/ buffer_handle_t** buffer,
+ /*optional out*/ int32_t* streamId = nullptr);
// Get a vector of (frameNumber, streamId) pair of currently inflight
// buffers
void getInflightBufferKeys(std::vector<std::pair<int32_t, int32_t>>* out);
+ // Get a vector of bufferId of currently inflight buffers
+ void getInflightRequestBufferKeys(std::vector<uint64_t>* out);
+
static const uint64_t BUFFER_ID_NO_BUFFER = 0;
private:
// Always valid
@@ -398,7 +404,7 @@
// Buffers given to HAL through requestStreamBuffer API
std::mutex mRequestedBuffersLock;
- std::unordered_map<uint64_t, buffer_handle_t*> mRequestedBuffers;
+ std::unordered_map<uint64_t, std::pair<int32_t, buffer_handle_t*>> mRequestedBuffers;
uint32_t mNextStreamConfigCounter = 1;