diff --git a/camera/device/default/ExternalCameraDeviceSession.cpp b/camera/device/default/ExternalCameraDeviceSession.cpp index a16dd7fd83..91196d4228 100644 --- a/camera/device/default/ExternalCameraDeviceSession.cpp +++ b/camera/device/default/ExternalCameraDeviceSession.cpp @@ -538,6 +538,19 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques return Status::INTERNAL_ERROR; } + if (request.outputBuffers.empty()) { + ALOGE("%s: No output buffers provided.", __FUNCTION__); + return Status::ILLEGAL_ARGUMENT; + } + + for (auto& outputBuf : request.outputBuffers) { + if (outputBuf.streamId == -1 || mStreamMap.find(outputBuf.streamId) == mStreamMap.end()) { + ALOGE("%s: Invalid streamId in CaptureRequest.outputBuffers: %d", __FUNCTION__, + outputBuf.streamId); + return Status::ILLEGAL_ARGUMENT; + } + } + const camera_metadata_t* rawSettings = nullptr; bool converted; CameraMetadata settingsFmq; // settings from FMQ @@ -572,8 +585,6 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques return Status::ILLEGAL_ARGUMENT; } - std::vector allBufPtrs; - std::vector allFences; size_t numOutputBufs = request.outputBuffers.size(); if (numOutputBufs == 0) { @@ -629,11 +640,6 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques } } - status = importRequestLocked(request, allBufPtrs, allFences); - if (status != Status::OK) { - return status; - } - nsecs_t shutterTs = 0; std::unique_ptr frameIn = dequeueV4l2FrameLocked(&shutterTs); if (frameIn == nullptr) { @@ -656,8 +662,8 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques halBuf.height = stream.height; halBuf.format = stream.format; halBuf.usage = stream.usage; - halBuf.bufPtr = allBufPtrs[i]; - halBuf.acquireFence = allFences[i]; + halBuf.bufPtr = nullptr; // threadloop will request buffer from cameraservice + halBuf.acquireFence = 0; // threadloop will request fence from cameraservice halBuf.fenceTimeout = false; } { @@ -1351,58 +1357,6 @@ bool ExternalCameraDeviceSession::isSupported( return false; } -Status ExternalCameraDeviceSession::importRequestLocked(const CaptureRequest& request, - std::vector& allBufPtrs, - std::vector& allFences) { - return importRequestLockedImpl(request, allBufPtrs, allFences); -} - -Status ExternalCameraDeviceSession::importRequestLockedImpl( - const CaptureRequest& request, std::vector& allBufPtrs, - std::vector& allFences) { - size_t numOutputBufs = request.outputBuffers.size(); - size_t numBufs = numOutputBufs; - // Validate all I/O buffers - std::vector allBufs; - std::vector allBufIds; - allBufs.resize(numBufs); - allBufIds.resize(numBufs); - allBufPtrs.resize(numBufs); - allFences.resize(numBufs); - std::vector streamIds(numBufs); - - for (size_t i = 0; i < numOutputBufs; i++) { - allBufs[i] = ::android::makeFromAidl(request.outputBuffers[i].buffer); - allBufIds[i] = request.outputBuffers[i].bufferId; - allBufPtrs[i] = &allBufs[i]; - streamIds[i] = request.outputBuffers[i].streamId; - } - - { - Mutex::Autolock _l(mCbsLock); - for (size_t i = 0; i < numBufs; i++) { - Status st = importBufferLocked(streamIds[i], allBufIds[i], allBufs[i], &allBufPtrs[i]); - if (st != Status::OK) { - // Detailed error logs printed in importBuffer - return st; - } - } - } - - // All buffers are imported. Now validate output buffer acquire fences - for (size_t i = 0; i < numOutputBufs; i++) { - native_handle_t* h = ::android::makeFromAidl(request.outputBuffers[i].acquireFence); - if (!sHandleImporter.importFence(h, allFences[i])) { - ALOGE("%s: output buffer %zu acquire fence is invalid", __FUNCTION__, i); - cleanupInflightFences(allFences, i); - native_handle_delete(h); - return Status::INTERNAL_ERROR; - } - native_handle_delete(h); - } - return Status::OK; -} - Status ExternalCameraDeviceSession::importBuffer(int32_t streamId, uint64_t bufId, buffer_handle_t buf, /*out*/ buffer_handle_t** outBufPtr) { @@ -1772,8 +1726,8 @@ Status ExternalCameraDeviceSession::processCaptureRequestError( result.outputBuffers[i].bufferId = req->buffers[i].bufferId; result.outputBuffers[i].status = BufferStatus::ERROR; if (req->buffers[i].acquireFence >= 0) { - native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0); - handle->data[0] = req->buffers[i].acquireFence; + // numFds = 0 for error + native_handle_t* handle = native_handle_create(/*numFds*/ 0, /*numInts*/ 0); result.outputBuffers[i].releaseFence = android::dupToAidl(handle); native_handle_delete(handle); } @@ -2007,9 +1961,16 @@ int ExternalCameraDeviceSession::BufferRequestThread::waitForBufferRequestDone( std::chrono::milliseconds timeout = std::chrono::milliseconds(kReqProcTimeoutMs); auto st = mRequestDoneCond.wait_for(lk, timeout); if (st == std::cv_status::timeout) { + mRequestingBuffer = false; ALOGE("%s: wait for buffer request finish timeout!", __FUNCTION__); return -1; } + + if (mPendingReturnBufferReqs.empty()) { + mRequestingBuffer = false; + ALOGE("%s: cameraservice did not return any buffers!", __FUNCTION__); + return -1; + } } mRequestingBuffer = false; *outBufReqs = std::move(mPendingReturnBufferReqs); @@ -2059,6 +2020,8 @@ bool ExternalCameraDeviceSession::BufferRequestThread::threadLoop() { if (!ret.isOk()) { ALOGE("%s: Transaction error: %d:%d", __FUNCTION__, ret.getExceptionCode(), ret.getServiceSpecificError()); + mBufferReqs.clear(); + mRequestDoneCond.notify_one(); return false; } @@ -2067,17 +2030,24 @@ bool ExternalCameraDeviceSession::BufferRequestThread::threadLoop() { if (bufRets.size() != mHalBufferReqs.size()) { ALOGE("%s: expect %zu buffer requests returned, only got %zu", __FUNCTION__, mHalBufferReqs.size(), bufRets.size()); + mBufferReqs.clear(); + lk.unlock(); + mRequestDoneCond.notify_one(); return false; } auto parent = mParent.lock(); if (parent == nullptr) { ALOGE("%s: session has been disconnected!", __FUNCTION__); + mBufferReqs.clear(); + lk.unlock(); + mRequestDoneCond.notify_one(); return false; } std::vector importedFences; importedFences.resize(bufRets.size()); + bool hasError = false; for (size_t i = 0; i < bufRets.size(); i++) { int streamId = bufRets[i].streamId; switch (bufRets[i].val.getTag()) { @@ -2088,7 +2058,8 @@ bool ExternalCameraDeviceSession::BufferRequestThread::threadLoop() { bufRets[i].val.get(); if (hBufs.size() != 1) { ALOGE("%s: expect 1 buffer returned, got %zu!", __FUNCTION__, hBufs.size()); - return false; + hasError = true; + break; } const StreamBuffer& hBuf = hBufs[0]; @@ -2105,25 +2076,38 @@ bool ExternalCameraDeviceSession::BufferRequestThread::threadLoop() { if (s != Status::OK) { ALOGE("%s: stream %d import buffer failed!", __FUNCTION__, streamId); cleanupInflightFences(importedFences, i - 1); - return false; + hasError = true; + break; } h = makeFromAidl(hBuf.acquireFence); if (!sHandleImporter.importFence(h, mBufferReqs[i].acquireFence)) { ALOGE("%s: stream %d import fence failed!", __FUNCTION__, streamId); cleanupInflightFences(importedFences, i - 1); native_handle_delete(h); - return false; + hasError = true; + break; } native_handle_delete(h); importedFences[i] = mBufferReqs[i].acquireFence; } break; default: ALOGE("%s: Unknown StreamBuffersVal!", __FUNCTION__); - return false; + hasError = true; + break; + } + if (hasError) { + mBufferReqs.clear(); + lk.unlock(); + mRequestDoneCond.notify_one(); + return true; } } } else { ALOGE("%s: requestStreamBuffers call failed!", __FUNCTION__); + mBufferReqs.clear(); + lk.unlock(); + mRequestDoneCond.notify_one(); + return true; } mPendingReturnBufferReqs = std::move(mBufferReqs); @@ -2828,6 +2812,11 @@ bool ExternalCameraDeviceSession::OutputThread::threadLoop() { if (res != 0) { // For some webcam, the first few V4L2 frames might be malformed... ALOGE("%s: Convert V4L2 frame to YU12 failed! res %d", __FUNCTION__, res); + + ATRACE_BEGIN("Wait for BufferRequest done"); + res = waitForBufferRequestDone(&req->buffers); + ATRACE_END(); + lk.unlock(); Status st = parent->processCaptureRequestError(req); if (st != Status::OK) { @@ -2843,9 +2832,15 @@ bool ExternalCameraDeviceSession::OutputThread::threadLoop() { ATRACE_END(); if (res != 0) { + // HAL buffer management buffer request can fail ALOGE("%s: wait for BufferRequest done failed! res %d", __FUNCTION__, res); lk.unlock(); - return onDeviceError("%s: failed to process buffer request error!", __FUNCTION__); + Status st = parent->processCaptureRequestError(req); + if (st != Status::OK) { + return onDeviceError("%s: failed to process capture request error!", __FUNCTION__); + } + signalRequestDone(); + return true; } ALOGV("%s processing new request", __FUNCTION__); diff --git a/camera/device/default/ExternalCameraDeviceSession.h b/camera/device/default/ExternalCameraDeviceSession.h index 736bfd1528..795b5899b6 100644 --- a/camera/device/default/ExternalCameraDeviceSession.h +++ b/camera/device/default/ExternalCameraDeviceSession.h @@ -266,15 +266,6 @@ class ExternalCameraDeviceSession : public BnCameraDeviceSession, public OutputT const std::vector& supportedFormats, const ExternalCameraConfig& cfg); - // Validate and import request's output buffers and acquire fence - Status importRequestLocked(const CaptureRequest& request, - std::vector& allBufPtrs, - std::vector& allFences); - - Status importRequestLockedImpl(const CaptureRequest& request, - std::vector& allBufPtrs, - std::vector& allFences); - Status importBufferLocked(int32_t streamId, uint64_t bufId, buffer_handle_t buf, /*out*/ buffer_handle_t** outBufPtr); static void cleanupInflightFences(std::vector& allFences, size_t numFences);