gugelfrei
/
android_frameworks_av
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Camera: fix acquire fence FD double close

Browse Source 
This patch fixes several issues:
* Change the timing the acquire fence FD is closed: we used to
  close the FD when the buffer is returned from HAL. This patch
  changes that to after the request is sent to HAL (or failed
  to send to HAL)
* Cleanup inflight buffer map if the request fails to be sent to
  HAL
* With the CL, the acquire fence FDs are now closed by
    - HalInterface::processBatchCaptureRequests if the HIDL
      processCaptureRequests call succeeds and HAL is running
      in binderized mode (for passthrough mode the FD is owned
      by HAL if the HIDL call succeeds)
    - Camera3Device::cleanupFailedRequests otherwise

Test: Camera CTS tests
Bug: 132594861
Change-Id: I5f67ae9e7b8008738bd9a24246d754a6a3669b0c
gugelfrei
Yin-Chia Yeh 5 years ago
parent 11648852d7
commit f8e28fb940
2 changed files with 73 additions and 27 deletions
Show Stats Download Patch File Download Diff File

84
services/camera/libcameraservice/device3/Camera3Device.cpp
Unescape View File

@ -4381,11 +4381,12 @@ status_t Camera3Device::HalInterface::configureStreams(const camera_metadata_t *
status_t Camera3Device::HalInterface::wrapAsHidlRequest(camera3_capture_request_t* request,
/*out*/device::V3_2::CaptureRequest* captureRequest,
/*out*/std::vector<native_handle_t*>* handlesCreated) {
/*out*/std::vector<native_handle_t*>* handlesCreated,
/*out*/std::vector<std::pair<int32_t, int32_t>>* inflightBuffers) {
ATRACE_CALL();
if (captureRequest == nullptr || handlesCreated == nullptr) {
ALOGE("%s: captureRequest (%p) and handlesCreated (%p) must not be null",
__FUNCTION__, captureRequest, handlesCreated);
if (captureRequest == nullptr || handlesCreated == nullptr || inflightBuffers == nullptr) {
ALOGE("%s: captureRequest (%p), handlesCreated (%p), and inflightBuffers(%p) "
"must not be null", __FUNCTION__, captureRequest, handlesCreated, inflightBuffers);
return BAD_VALUE;
}
@ -4415,8 +4416,8 @@ status_t Camera3Device::HalInterface::wrapAsHidlRequest(camera3_capture_request_
captureRequest->inputBuffer.releaseFence = nullptr;
pushInflightBufferLocked(captureRequest->frameNumber, streamId,
request->input_buffer->buffer,
request->input_buffer->acquire_fence);
request->input_buffer->buffer);
inflightBuffers->push_back(std::make_pair(captureRequest->frameNumber, streamId));
} else {
captureRequest->inputBuffer.streamId = -1;
captureRequest->inputBuffer.bufferId = BUFFER_ID_NO_BUFFER;
@ -4455,14 +4456,31 @@ status_t Camera3Device::HalInterface::wrapAsHidlRequest(camera3_capture_request_
// Output buffers are empty when using HAL buffer manager
if (!mUseHalBufManager) {
pushInflightBufferLocked(captureRequest->frameNumber, streamId,
src->buffer, src->acquire_fence);
pushInflightBufferLocked(captureRequest->frameNumber, streamId, src->buffer);
inflightBuffers->push_back(std::make_pair(captureRequest->frameNumber, streamId));
}
}
}
return OK;
}
void Camera3Device::HalInterface::cleanupNativeHandles(
std::vector<native_handle_t*> *handles, bool closeFd) {
if (handles == nullptr) {
return;
}
if (closeFd) {
for (auto& handle : *handles) {
native_handle_close(handle);
}
}
for (auto& handle : *handles) {
native_handle_delete(handle);
}
handles->clear();
return;
}
status_t Camera3Device::HalInterface::processBatchCaptureRequests(
std::vector<camera3_capture_request_t*>& requests,/*out*/uint32_t* numRequestProcessed) {
ATRACE_NAME("CameraHal::processBatchCaptureRequests");
@ -4483,17 +4501,20 @@ status_t Camera3Device::HalInterface::processBatchCaptureRequests(
captureRequests.resize(batchSize);
}
std::vector<native_handle_t*> handlesCreated;
std::vector<std::pair<int32_t, int32_t>> inflightBuffers;
status_t res = OK;
for (size_t i = 0; i < batchSize; i++) {
if (hidlSession_3_4 != nullptr) {
res = wrapAsHidlRequest(requests[i], /*out*/&captureRequests_3_4[i].v3_2,
/*out*/&handlesCreated);
/*out*/&handlesCreated, /*out*/&inflightBuffers);
} else {
res = wrapAsHidlRequest(requests[i],
/*out*/&captureRequests[i], /*out*/&handlesCreated);
res = wrapAsHidlRequest(requests[i], /*out*/&captureRequests[i],
/*out*/&handlesCreated, /*out*/&inflightBuffers);
}
if (res != OK) {
popInflightBuffers(inflightBuffers);
cleanupNativeHandles(&handlesCreated);
return res;
}
}
@ -4590,18 +4611,30 @@ status_t Camera3Device::HalInterface::processBatchCaptureRequests(
}
if (!err.isOk()) {
ALOGE("%s: Transaction error: %s", __FUNCTION__, err.description().c_str());
return DEAD_OBJECT;
status = common::V1_0::Status::CAMERA_DISCONNECTED;
}
if (status == common::V1_0::Status::OK && *numRequestProcessed != batchSize) {
ALOGE("%s: processCaptureRequest returns OK but processed %d/%zu requests",
__FUNCTION__, *numRequestProcessed, batchSize);
status = common::V1_0::Status::INTERNAL_ERROR;
}
for (auto& handle : handlesCreated) {
native_handle_delete(handle);
res = CameraProviderManager::mapToStatusT(status);
if (res == OK) {
if (mHidlSession->isRemote()) {
// Only close acquire fence FDs when the HIDL transaction succeeds (so the FDs have been
// sent to camera HAL processes)
cleanupNativeHandles(&handlesCreated, /*closeFd*/true);
} else {
// In passthrough mode the FDs are now owned by HAL
cleanupNativeHandles(&handlesCreated);
}
} else {
popInflightBuffers(inflightBuffers);
cleanupNativeHandles(&handlesCreated);
}
return CameraProviderManager::mapToStatusT(status);
return res;
}
status_t Camera3Device::HalInterface::flush() {
@ -4683,10 +4716,9 @@ void Camera3Device::HalInterface::getInflightRequestBufferKeys(
}
status_t Camera3Device::HalInterface::pushInflightBufferLocked(
int32_t frameNumber, int32_t streamId, buffer_handle_t *buffer, int acquireFence) {
int32_t frameNumber, int32_t streamId, buffer_handle_t *buffer) {
uint64_t key = static_cast<uint64_t>(frameNumber) << 32 | static_cast<uint64_t>(streamId);
auto pair = std::make_pair(buffer, acquireFence);
mInflightBufferMap[key] = pair;
mInflightBufferMap[key] = buffer;
return OK;
}
@ -4698,16 +4730,22 @@ status_t Camera3Device::HalInterface::popInflightBuffer(
uint64_t key = static_cast<uint64_t>(frameNumber) << 32 | static_cast<uint64_t>(streamId);
auto it = mInflightBufferMap.find(key);
if (it == mInflightBufferMap.end()) return NAME_NOT_FOUND;
auto pair = it->second;
*buffer = pair.first;
int acquireFence = pair.second;
if (acquireFence > 0) {
::close(acquireFence);
if (buffer != nullptr) {
*buffer = it->second;
}
mInflightBufferMap.erase(it);
return OK;
}
void Camera3Device::HalInterface::popInflightBuffers(
const std::vector<std::pair<int32_t, int32_t>>& buffers) {
for (const auto& pair : buffers) {
int32_t frameNumber = pair.first;
int32_t streamId = pair.second;
popInflightBuffer(frameNumber, streamId, nullptr);
}
}
status_t Camera3Device::HalInterface::pushInflightRequestBuffer(
uint64_t bufferId, buffer_handle_t* buf, int32_t streamId) {
std::lock_guard<std::mutex> lock(mRequestedBuffersLock);

16
services/camera/libcameraservice/device3/Camera3Device.h
Unescape View File

@ -358,13 +358,21 @@ class Camera3Device :
// Do not free input camera3_capture_request_t before output HIDL request
status_t wrapAsHidlRequest(camera3_capture_request_t* in,
/*out*/hardware::camera::device::V3_2::CaptureRequest* out,
/*out*/std::vector<native_handle_t*>* handlesCreated);
/*out*/std::vector<native_handle_t*>* handlesCreated,
/*out*/std::vector<std::pair<int32_t, int32_t>>* inflightBuffers);
status_t pushInflightBufferLocked(int32_t frameNumber, int32_t streamId,
buffer_handle_t *buffer, int acquireFence);
buffer_handle_t *buffer);
// Pop inflight buffers based on pairs of (frameNumber,streamId)
void popInflightBuffers(const std::vector<std::pair<int32_t, int32_t>>& buffers);
// Cache of buffer handles keyed off (frameNumber << 32 | streamId)
// value is a pair of (buffer_handle_t*, acquire_fence FD)
std::unordered_map<uint64_t, std::pair<buffer_handle_t*, int>> mInflightBufferMap;
std::unordered_map<uint64_t, buffer_handle_t*> mInflightBufferMap;
// Delete and optionally close native handles and clear the input vector afterward
static void cleanupNativeHandles(
std::vector<native_handle_t*> *handles, bool closeFd = false);
struct BufferHasher {
size_t operator()(const buffer_handle_t& buf) const {

Write Preview
Loading…
Cancel
Save