Merge "Camera: fix bug in useHalBufMgr mode"
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 58471b9..4b938ed 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -181,7 +181,14 @@
});
}
- mInterface = new HalInterface(session, queue);
+ camera_metadata_entry bufMgrMode =
+ mDeviceInfo.find(ANDROID_INFO_SUPPORTED_BUFFER_MANAGEMENT_VERSION);
+ if (bufMgrMode.count > 0) {
+ mUseHalBufManager = (bufMgrMode.data.u8[0] ==
+ ANDROID_INFO_SUPPORTED_BUFFER_MANAGEMENT_VERSION_HIDL_DEVICE_3_5);
+ }
+
+ mInterface = new HalInterface(session, queue, mUseHalBufManager);
std::string providerType;
mVendorTagId = manager->getProviderTagIdLocked(mId.string());
mTagMonitor.initialize(mVendorTagId);
@@ -230,23 +237,6 @@
/** Register in-flight map to the status tracker */
mInFlightStatusId = mStatusTracker->addComponent();
- /** Create buffer manager */
- mBufferManager = new Camera3BufferManager();
-
- Vector<int32_t> sessionParamKeys;
- camera_metadata_entry_t sessionKeysEntry = mDeviceInfo.find(
- ANDROID_REQUEST_AVAILABLE_SESSION_KEYS);
- if (sessionKeysEntry.count > 0) {
- sessionParamKeys.insertArrayAt(sessionKeysEntry.data.i32, 0, sessionKeysEntry.count);
- }
-
- camera_metadata_entry bufMgrMode =
- mDeviceInfo.find(ANDROID_INFO_SUPPORTED_BUFFER_MANAGEMENT_VERSION);
- if (bufMgrMode.count > 0) {
- mUseHalBufManager = (bufMgrMode.data.u8[0] ==
- ANDROID_INFO_SUPPORTED_BUFFER_MANAGEMENT_VERSION_HIDL_DEVICE_3_5);
- }
-
if (mUseHalBufManager) {
res = mRequestBufferSM.initialize(mStatusTracker);
if (res != OK) {
@@ -258,6 +248,16 @@
}
}
+ /** Create buffer manager */
+ mBufferManager = new Camera3BufferManager();
+
+ Vector<int32_t> sessionParamKeys;
+ camera_metadata_entry_t sessionKeysEntry = mDeviceInfo.find(
+ ANDROID_REQUEST_AVAILABLE_SESSION_KEYS);
+ if (sessionKeysEntry.count > 0) {
+ sessionParamKeys.insertArrayAt(sessionKeysEntry.data.i32, 0, sessionKeysEntry.count);
+ }
+
/** Start up request queue thread */
mRequestThread = new RequestThread(
this, mStatusTracker, mInterface, sessionParamKeys, mUseHalBufManager);
@@ -1330,14 +1330,24 @@
}
bDst.stream = stream->asHalStream();
- buffer_handle_t *buffer;
+ bool noBufferReturned = false;
+ buffer_handle_t *buffer = nullptr;
if (mUseHalBufManager) {
+ // This is suspicious most of the time but can be correct during flush where HAL
+ // has to return capture result before a buffer is requested
if (bSrc.bufferId == HalInterface::BUFFER_ID_NO_BUFFER) {
- ALOGE("%s: Frame %d: Buffer %zu: No bufferId for stream %d",
- __FUNCTION__, result.frameNumber, i, bSrc.streamId);
- return;
+ if (bSrc.status == BufferStatus::OK) {
+ ALOGE("%s: Frame %d: Buffer %zu: No bufferId for stream %d",
+ __FUNCTION__, result.frameNumber, i, bSrc.streamId);
+ // Still proceeds so other buffers can be returned
+ }
+ noBufferReturned = true;
}
- res = mInterface->popInflightRequestBuffer(bSrc.bufferId, &buffer);
+ if (noBufferReturned) {
+ res = OK;
+ } else {
+ res = mInterface->popInflightRequestBuffer(bSrc.bufferId, &buffer);
+ }
} else {
res = mInterface->popInflightBuffer(result.frameNumber, bSrc.streamId, &buffer);
}
@@ -1354,6 +1364,9 @@
if (bSrc.releaseFence == nullptr) {
bDst.release_fence = -1;
} else if (bSrc.releaseFence->numFds == 1) {
+ if (noBufferReturned) {
+ ALOGE("%s: got releaseFence without output buffer!", __FUNCTION__);
+ }
bDst.release_fence = dup(bSrc.releaseFence->data[0]);
} else {
ALOGE("%s: Frame %d: Invalid release fence for buffer %zu, fd count is %d, not 1",
@@ -3090,6 +3103,16 @@
for (size_t i = 0; i < numBuffers; i++)
{
+ if (outputBuffers[i].buffer == nullptr) {
+ if (!mUseHalBufManager) {
+ // With HAL buffer management API, HAL sometimes will have to return buffers that
+ // has not got a output buffer handle filled yet. This is though illegal if HAL
+ // buffer management API is not being used.
+ ALOGE("%s: cannot return a null buffer!", __FUNCTION__);
+ }
+ continue;
+ }
+
Camera3StreamInterface *stream = Camera3Stream::cast(outputBuffers[i].stream);
int streamId = stream->getId();
const auto& it = outputSurfaces.find(streamId);
@@ -3847,9 +3870,11 @@
Camera3Device::HalInterface::HalInterface(
sp<ICameraDeviceSession> &session,
- std::shared_ptr<RequestMetadataQueue> queue) :
+ std::shared_ptr<RequestMetadataQueue> queue,
+ bool useHalBufManager) :
mHidlSession(session),
- mRequestMetadataQueue(queue) {
+ mRequestMetadataQueue(queue),
+ mUseHalBufManager(useHalBufManager) {
// Check with hardware service manager if we can downcast these interfaces
// Somewhat expensive, so cache the results at startup
auto castResult_3_5 = device::V3_5::ICameraDeviceSession::castFrom(mHidlSession);
@@ -3866,11 +3891,12 @@
}
}
-Camera3Device::HalInterface::HalInterface() {}
+Camera3Device::HalInterface::HalInterface() : mUseHalBufManager(false) {}
Camera3Device::HalInterface::HalInterface(const HalInterface& other) :
mHidlSession(other.mHidlSession),
- mRequestMetadataQueue(other.mRequestMetadataQueue) {}
+ mRequestMetadataQueue(other.mRequestMetadataQueue),
+ mUseHalBufManager(other.mUseHalBufManager) {}
bool Camera3Device::HalInterface::valid() {
return (mHidlSession != nullptr);
@@ -4186,14 +4212,14 @@
return res;
}
-void Camera3Device::HalInterface::wrapAsHidlRequest(camera3_capture_request_t* request,
+status_t Camera3Device::HalInterface::wrapAsHidlRequest(camera3_capture_request_t* request,
/*out*/device::V3_2::CaptureRequest* captureRequest,
/*out*/std::vector<native_handle_t*>* handlesCreated) {
ATRACE_CALL();
if (captureRequest == nullptr || handlesCreated == nullptr) {
ALOGE("%s: captureRequest (%p) and handlesCreated (%p) must not be null",
__FUNCTION__, captureRequest, handlesCreated);
- return;
+ return BAD_VALUE;
}
captureRequest->frameNumber = request->frame_number;
@@ -4234,26 +4260,37 @@
const camera3_stream_buffer_t *src = request->output_buffers + i;
StreamBuffer &dst = captureRequest->outputBuffers[i];
int32_t streamId = Camera3Stream::cast(src->stream)->getId();
- buffer_handle_t buf = *(src->buffer);
- auto pair = getBufferId(buf, streamId);
- bool isNewBuffer = pair.first;
- dst.streamId = streamId;
- dst.bufferId = pair.second;
- dst.buffer = isNewBuffer ? buf : nullptr;
- dst.status = BufferStatus::OK;
- native_handle_t *acquireFence = nullptr;
- if (src->acquire_fence != -1) {
- acquireFence = native_handle_create(1,0);
- acquireFence->data[0] = src->acquire_fence;
- handlesCreated->push_back(acquireFence);
+ if (src->buffer != nullptr) {
+ buffer_handle_t buf = *(src->buffer);
+ auto pair = getBufferId(buf, streamId);
+ bool isNewBuffer = pair.first;
+ dst.bufferId = pair.second;
+ dst.buffer = isNewBuffer ? buf : nullptr;
+ native_handle_t *acquireFence = nullptr;
+ if (src->acquire_fence != -1) {
+ acquireFence = native_handle_create(1,0);
+ acquireFence->data[0] = src->acquire_fence;
+ handlesCreated->push_back(acquireFence);
+ }
+ dst.acquireFence = acquireFence;
+ } else if (mUseHalBufManager) {
+ // HAL buffer management path
+ dst.bufferId = BUFFER_ID_NO_BUFFER;
+ dst.buffer = nullptr;
+ dst.acquireFence = nullptr;
+ } else {
+ ALOGE("%s: cannot send a null buffer in capture request!", __FUNCTION__);
+ return BAD_VALUE;
}
- dst.acquireFence = acquireFence;
+ dst.streamId = streamId;
+ dst.status = BufferStatus::OK;
dst.releaseFence = nullptr;
pushInflightBufferLocked(captureRequest->frameNumber, streamId,
src->buffer, src->acquire_fence);
}
}
+ return OK;
}
status_t Camera3Device::HalInterface::processBatchCaptureRequests(
@@ -4277,12 +4314,17 @@
}
std::vector<native_handle_t*> handlesCreated;
+ status_t res = OK;
for (size_t i = 0; i < batchSize; i++) {
if (hidlSession_3_4 != nullptr) {
- wrapAsHidlRequest(requests[i], /*out*/&captureRequests_3_4[i].v3_2,
+ res = wrapAsHidlRequest(requests[i], /*out*/&captureRequests_3_4[i].v3_2,
/*out*/&handlesCreated);
} else {
- wrapAsHidlRequest(requests[i], /*out*/&captureRequests[i], /*out*/&handlesCreated);
+ res = wrapAsHidlRequest(requests[i],
+ /*out*/&captureRequests[i], /*out*/&handlesCreated);
+ }
+ if (res != OK) {
+ return res;
}
}
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index 35b9d6d..419ac42 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -256,7 +256,8 @@
class HalInterface : public camera3::Camera3StreamBufferFreedListener {
public:
HalInterface(sp<hardware::camera::device::V3_2::ICameraDeviceSession> &session,
- std::shared_ptr<RequestMetadataQueue> queue);
+ std::shared_ptr<RequestMetadataQueue> queue,
+ bool useHalBufManager);
HalInterface(const HalInterface &other);
HalInterface();
@@ -322,7 +323,7 @@
// The output HIDL request still depends on input camera3_capture_request_t
// Do not free input camera3_capture_request_t before output HIDL request
- void wrapAsHidlRequest(camera3_capture_request_t* in,
+ status_t wrapAsHidlRequest(camera3_capture_request_t* in,
/*out*/hardware::camera::device::V3_2::CaptureRequest* out,
/*out*/std::vector<native_handle_t*>* handlesCreated);
@@ -376,6 +377,8 @@
std::unordered_map<uint64_t, buffer_handle_t*> mRequestedBuffers;
uint32_t mNextStreamConfigCounter = 1;
+
+ const bool mUseHalBufManager;
};
sp<HalInterface> mInterface;