From bed3a9473e43030d98678e14e4e4cc69dae41a6f Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Mon, 6 Mar 2017 14:14:17 -0800 Subject: [PATCH] Camera: add batching support Currently only batching high speed recording request/results. Test: GCA slow motion recording working Bug: 34899394 Change-Id: Id83b9d1cefe011391c86a5e7e898262a169cc9e7 --- camera/device/3.2/ICameraDeviceCallback.hal | 25 +- camera/device/3.2/ICameraDeviceSession.hal | 26 +- camera/device/3.2/default/CameraDevice.cpp | 13 +- .../3.2/default/CameraDeviceSession.cpp | 436 ++++++++++++++++-- .../device/3.2/default/CameraDeviceSession.h | 116 ++++- .../VtsHalCameraProviderV2_4TargetTest.cpp | 87 ++-- 6 files changed, 624 insertions(+), 79 deletions(-) diff --git a/camera/device/3.2/ICameraDeviceCallback.hal b/camera/device/3.2/ICameraDeviceCallback.hal index 753d085649..bf51da2133 100644 --- a/camera/device/3.2/ICameraDeviceCallback.hal +++ b/camera/device/3.2/ICameraDeviceCallback.hal @@ -35,7 +35,8 @@ interface ICameraDeviceCallback { /** * processCaptureResult: * - * Send results from a completed capture to the framework. + * Send results from one or more completed or partially completed captures + * to the framework. * processCaptureResult() may be invoked multiple times by the HAL in * response to a single capture request. This allows, for example, the * metadata and low-resolution buffers to be returned in one call, and @@ -61,11 +62,14 @@ interface ICameraDeviceCallback { * acceptable and expected that the buffer for request 5 for stream A may be * returned after the buffer for request 6 for stream B is. And it is * acceptable that the result metadata for request 6 for stream B is - * returned before the buffer for request 5 for stream A is. + * returned before the buffer for request 5 for stream A is. If multiple + * capture results are included in a single call, camera framework must + * process results sequentially from lower index to higher index, as if + * these results were sent to camera framework one by one, from lower index + * to higher index. * * The HAL retains ownership of result structure, which only needs to be - * valid to access during this call. The framework must copy whatever it - * needs before this call returns. + * valid to access during this call. * * The output buffers do not need to be filled yet; the framework must wait * on the stream buffer release sync fence before reading the buffer @@ -93,20 +97,23 @@ interface ICameraDeviceCallback { * * Performance requirements: * - * This is a non-blocking call. The framework must return this call in 5ms. + * This is a non-blocking call. The framework must handle each CaptureResult + * within 5ms. * * The pipeline latency (see S7 for definition) should be less than or equal to * 4 frame intervals, and must be less than or equal to 8 frame intervals. * */ - processCaptureResult(CaptureResult result); + processCaptureResult(vec results); /** * notify: * * Asynchronous notification callback from the HAL, fired for various * reasons. Only for information independent of frame capture, or that - * require specific timing. + * require specific timing. Multiple messages may be sent in one call; a + * message with a higher index must be considered to have occurred after a + * message with a lower index. * * Multiple threads may call notify() simultaneously. * @@ -119,8 +126,8 @@ interface ICameraDeviceCallback { * ------------------------------------------------------------------------ * Performance requirements: * - * This is a non-blocking call. The framework must return this call in 5ms. + * This is a non-blocking call. The framework must handle each message in 5ms. */ - notify(NotifyMsg msg); + notify(vec msgs); }; diff --git a/camera/device/3.2/ICameraDeviceSession.hal b/camera/device/3.2/ICameraDeviceSession.hal index e92d75613f..d8d317735f 100644 --- a/camera/device/3.2/ICameraDeviceSession.hal +++ b/camera/device/3.2/ICameraDeviceSession.hal @@ -171,14 +171,16 @@ interface ICameraDeviceSession { /** * processCaptureRequest: * - * Send a new capture request to the HAL. The HAL must not return from - * this call until it is ready to accept the next request to process. Only - * one call to processCaptureRequest() must be made at a time by the - * framework, and the calls must all be from the same thread. The next call - * to processCaptureRequest() must be made as soon as a new request and - * its associated buffers are available. In a normal preview scenario, this - * means the function is generally called again by the framework almost - * instantly. + * Send a list of capture requests to the HAL. The HAL must not return from + * this call until it is ready to accept the next set of requests to + * process. Only one call to processCaptureRequest() must be made at a time + * by the framework, and the calls must all be from the same thread. The + * next call to processCaptureRequest() must be made as soon as a new + * request and its associated buffers are available. In a normal preview + * scenario, this means the function is generally called again by the + * framework almost instantly. If more than one request is provided by the + * client, the HAL must process the requests in order of lowest index to + * highest index. * * The actual request processing is asynchronous, with the results of * capture being returned by the HAL through the processCaptureResult() @@ -229,10 +231,14 @@ interface ICameraDeviceSession { * If the camera device has encountered a serious error. After this * error is returned, only the close() method can be successfully * called by the framework. + * @return numRequestProcessed Number of requests successfully processed by + * camera HAL. When status is OK, this must be equal to the size of + * requests. When the call fails, this number is the number of requests + * that HAL processed successfully before HAL runs into an error. * */ - processCaptureRequest(CaptureRequest request) - generates (Status status); + processCaptureRequest(vec requests) + generates (Status status, uint32_t numRequestProcessed); /** * flush: diff --git a/camera/device/3.2/default/CameraDevice.cpp b/camera/device/3.2/default/CameraDevice.cpp index 0a457ad91d..a742335548 100644 --- a/camera/device/3.2/default/CameraDevice.cpp +++ b/camera/device/3.2/default/CameraDevice.cpp @@ -229,7 +229,18 @@ Return CameraDevice::open(const sp& callback, open_ return Void(); } - session = new CameraDeviceSession(device, callback); + struct camera_info info; + res = mModule->getCameraInfo(mCameraIdInt, &info); + if (res != OK) { + ALOGE("%s: Could not open camera: getCameraInfo failed", __FUNCTION__); + device->common.close(&device->common); + mLock.unlock(); + _hidl_cb(Status::ILLEGAL_ARGUMENT, nullptr); + return Void(); + } + + session = new CameraDeviceSession( + device, info.static_camera_characteristics, callback); if (session == nullptr) { ALOGE("%s: camera device session allocation failed", __FUNCTION__); mLock.unlock(); diff --git a/camera/device/3.2/default/CameraDeviceSession.cpp b/camera/device/3.2/default/CameraDeviceSession.cpp index ae5d576380..fb1d1ff7c1 100644 --- a/camera/device/3.2/default/CameraDeviceSession.cpp +++ b/camera/device/3.2/default/CameraDeviceSession.cpp @@ -17,6 +17,7 @@ #define LOG_TAG "CamDevSession@3.2-impl" #include +#include #include #include #include @@ -30,12 +31,25 @@ namespace V3_2 { namespace implementation { HandleImporter& CameraDeviceSession::sHandleImporter = HandleImporter::getInstance(); +const int CameraDeviceSession::ResultBatcher::NOT_BATCHED; CameraDeviceSession::CameraDeviceSession( - camera3_device_t* device, const sp& callback) : + camera3_device_t* device, + const camera_metadata_t* deviceInfo, + const sp& callback) : camera3_callback_ops({&sProcessCaptureResult, &sNotify}), mDevice(device), - mCallback(callback) { + mResultBatcher(callback) { + + mDeviceInfo = deviceInfo; + uint32_t numPartialResults = 1; + camera_metadata_entry partialResultsCount = + mDeviceInfo.find(ANDROID_REQUEST_PARTIAL_RESULT_COUNT); + if (partialResultsCount.count > 0) { + numPartialResults = partialResultsCount.data.i32[0]; + } + mResultBatcher.setNumPartialResults(numPartialResults); + mInitFail = initialize(); } @@ -177,6 +191,354 @@ void CameraDeviceSession::cleanupInflightFences( } } +CameraDeviceSession::ResultBatcher::ResultBatcher( + const sp& callback) : mCallback(callback) {}; + +bool CameraDeviceSession::ResultBatcher::InflightBatch::allDelivered() const { + if (!mShutterDelivered) return false; + + if (mPartialResultProgress < mNumPartialResults) { + return false; + } + + for (const auto& pair : mBatchBufs) { + if (!pair.second.mDelivered) { + return false; + } + } + return true; +} + +void CameraDeviceSession::ResultBatcher::setNumPartialResults(uint32_t n) { + Mutex::Autolock _l(mLock); + mNumPartialResults = n; +} + +void CameraDeviceSession::ResultBatcher::setBatchedStreams( + const std::vector& streamsToBatch) { + Mutex::Autolock _l(mLock); + mStreamsToBatch = streamsToBatch; +} + +void CameraDeviceSession::ResultBatcher::registerBatch( + const hidl_vec& requests) { + auto batch = std::make_shared(); + batch->mFirstFrame = requests[0].frameNumber; + batch->mBatchSize = requests.size(); + batch->mLastFrame = batch->mFirstFrame + batch->mBatchSize - 1; + batch->mNumPartialResults = mNumPartialResults; + for (int id : mStreamsToBatch) { + batch->mBatchBufs[id] = InflightBatch::BufferBatch(); + } + Mutex::Autolock _l(mLock); + mInflightBatches.push_back(batch); +} + +std::pair> +CameraDeviceSession::ResultBatcher::getBatch( + uint32_t frameNumber) { + Mutex::Autolock _l(mLock); + int numBatches = mInflightBatches.size(); + if (numBatches == 0) { + return std::make_pair(NOT_BATCHED, nullptr); + } + uint32_t frameMin = mInflightBatches[0]->mFirstFrame; + uint32_t frameMax = mInflightBatches[numBatches - 1]->mLastFrame; + if (frameNumber < frameMin || frameNumber > frameMax) { + return std::make_pair(NOT_BATCHED, nullptr); + } + for (int i = 0; i < numBatches; i++) { + if (frameNumber >= mInflightBatches[i]->mFirstFrame && + frameNumber <= mInflightBatches[i]->mLastFrame) { + return std::make_pair(i, mInflightBatches[i]); + } + } + return std::make_pair(NOT_BATCHED, nullptr); +} + +void CameraDeviceSession::ResultBatcher::checkAndRemoveFirstBatch() { + Mutex::Autolock _l(mLock); + if (mInflightBatches.size() > 0) { + std::shared_ptr batch = mInflightBatches[0]; + bool shouldRemove = false; + { + Mutex::Autolock _l(batch->mLock); + if (batch->allDelivered()) { + batch->mRemoved = true; + shouldRemove = true; + } + } + if (shouldRemove) { + mInflightBatches.pop_front(); + } + } +} + +void CameraDeviceSession::ResultBatcher::sendBatchShutterCbsLocked(std::shared_ptr batch) { + if (batch->mShutterDelivered) { + ALOGW("%s: batch shutter callback already sent!", __FUNCTION__); + return; + } + + mCallback->notify(batch->mShutterMsgs); + batch->mShutterDelivered = true; + batch->mShutterMsgs.clear(); +} + +void CameraDeviceSession::ResultBatcher::freeReleaseFences(hidl_vec& results) { + for (auto& result : results) { + if (result.inputBuffer.releaseFence.getNativeHandle() != nullptr) { + native_handle_t* handle = const_cast( + result.inputBuffer.releaseFence.getNativeHandle()); + native_handle_delete(handle); + } + for (auto& buf : result.outputBuffers) { + if (buf.releaseFence.getNativeHandle() != nullptr) { + native_handle_t* handle = const_cast( + buf.releaseFence.getNativeHandle()); + native_handle_delete(handle); + } + } + } + return; +} + +void CameraDeviceSession::ResultBatcher::sendBatchBuffersLocked(std::shared_ptr batch) { + sendBatchBuffersLocked(batch, mStreamsToBatch); +} + +void CameraDeviceSession::ResultBatcher::sendBatchBuffersLocked( + std::shared_ptr batch, const std::vector& streams) { + size_t batchSize = 0; + for (int streamId : streams) { + auto it = batch->mBatchBufs.find(streamId); + if (it != batch->mBatchBufs.end()) { + InflightBatch::BufferBatch& bb = it->second; + if (bb.mDelivered) { + continue; + } + if (bb.mBuffers.size() > batchSize) { + batchSize = bb.mBuffers.size(); + } + } else { + ALOGE("%s: stream ID %d is not batched!", __FUNCTION__, streamId); + return; + } + } + + if (batchSize == 0) { + ALOGW("%s: there is no buffer to be delivered for this batch.", __FUNCTION__); + for (int streamId : streams) { + InflightBatch::BufferBatch& bb = batch->mBatchBufs[streamId]; + bb.mDelivered = true; + } + return; + } + + hidl_vec results; + results.resize(batchSize); + for (size_t i = 0; i < batchSize; i++) { + results[i].frameNumber = batch->mFirstFrame + i; + results[i].partialResult = 0; // 0 for buffer only results + results[i].inputBuffer.streamId = -1; + results[i].inputBuffer.bufferId = 0; + results[i].inputBuffer.buffer = nullptr; + std::vector outBufs; + for (int streamId : streams) { + InflightBatch::BufferBatch& bb = batch->mBatchBufs[streamId]; + if (bb.mDelivered) { + continue; + } + if (i < bb.mBuffers.size()) { + outBufs.push_back(bb.mBuffers[i]); + } + } + results[i].outputBuffers = outBufs; + } + mCallback->processCaptureResult(results); + freeReleaseFences(results); + for (int streamId : streams) { + InflightBatch::BufferBatch& bb = batch->mBatchBufs[streamId]; + bb.mDelivered = true; + bb.mBuffers.clear(); + } +} + +void CameraDeviceSession::ResultBatcher::sendBatchMetadataLocked( + std::shared_ptr batch, uint32_t lastPartialResultIdx) { + if (lastPartialResultIdx <= batch->mPartialResultProgress) { + // Result has been delivered. Return + ALOGW("%s: partial result %u has been delivered", __FUNCTION__, lastPartialResultIdx); + return; + } + + std::vector results; + std::vector toBeRemovedIdxes; + for (auto& pair : batch->mResultMds) { + uint32_t partialIdx = pair.first; + if (partialIdx > lastPartialResultIdx) { + continue; + } + toBeRemovedIdxes.push_back(partialIdx); + InflightBatch::MetadataBatch& mb = pair.second; + for (const auto& p : mb.mMds) { + CaptureResult result; + result.frameNumber = p.first; + result.result = std::move(p.second); + result.inputBuffer.streamId = -1; + result.inputBuffer.bufferId = 0; + result.inputBuffer.buffer = nullptr; + result.partialResult = partialIdx; + results.push_back(std::move(result)); + } + mb.mMds.clear(); + } + mCallback->processCaptureResult(results); + batch->mPartialResultProgress = lastPartialResultIdx; + for (uint32_t partialIdx : toBeRemovedIdxes) { + batch->mResultMds.erase(partialIdx); + } +} + +void CameraDeviceSession::ResultBatcher::notifySingleMsg(NotifyMsg& msg) { + mCallback->notify({msg}); + return; +} + +void CameraDeviceSession::ResultBatcher::notify(NotifyMsg& msg) { + uint32_t frameNumber; + if (CC_LIKELY(msg.type == MsgType::SHUTTER)) { + frameNumber = msg.msg.shutter.frameNumber; + } else { + frameNumber = msg.msg.error.frameNumber; + } + + auto pair = getBatch(frameNumber); + int batchIdx = pair.first; + if (batchIdx == NOT_BATCHED) { + notifySingleMsg(msg); + return; + } + + // When error happened, stop batching for all batches earlier + if (CC_UNLIKELY(msg.type == MsgType::ERROR)) { + Mutex::Autolock _l(mLock); + for (int i = 0; i <= batchIdx; i++) { + // Send batched data up + std::shared_ptr batch = mInflightBatches[0]; + { + Mutex::Autolock _l(batch->mLock); + sendBatchShutterCbsLocked(batch); + sendBatchBuffersLocked(batch); + sendBatchMetadataLocked(batch, mNumPartialResults); + if (!batch->allDelivered()) { + ALOGE("%s: error: some batch data not sent back to framework!", + __FUNCTION__); + } + batch->mRemoved = true; + } + mInflightBatches.pop_front(); + } + // Send the error up + notifySingleMsg(msg); + return; + } + // Queue shutter callbacks for future delivery + std::shared_ptr batch = pair.second; + { + Mutex::Autolock _l(batch->mLock); + // Check if the batch is removed (mostly by notify error) before lock was acquired + if (batch->mRemoved) { + // Fall back to non-batch path + notifySingleMsg(msg); + return; + } + + batch->mShutterMsgs.push_back(msg); + if (frameNumber == batch->mLastFrame) { + sendBatchShutterCbsLocked(batch); + } + } // end of batch lock scope + + // see if the batch is complete + if (frameNumber == batch->mLastFrame) { + checkAndRemoveFirstBatch(); + } +} + +void CameraDeviceSession::ResultBatcher::processOneCaptureResult(CaptureResult& result) { + hidl_vec results = {result}; + mCallback->processCaptureResult(results); + freeReleaseFences(results); + return; +} + +void CameraDeviceSession::ResultBatcher::processCaptureResult(CaptureResult& result) { + auto pair = getBatch(result.frameNumber); + int batchIdx = pair.first; + if (batchIdx == NOT_BATCHED) { + processOneCaptureResult(result); + return; + } + std::shared_ptr batch = pair.second; + { + Mutex::Autolock _l(batch->mLock); + // Check if the batch is removed (mostly by notify error) before lock was acquired + if (batch->mRemoved) { + // Fall back to non-batch path + processOneCaptureResult(result); + return; + } + + // queue metadata + if (result.result.size() != 0) { + // Save a copy of metadata + batch->mResultMds[result.partialResult].mMds.push_back( + std::make_pair(result.frameNumber, result.result)); + } + + // queue buffer + std::vector filledStreams; + std::vector nonBatchedBuffers; + for (auto& buffer : result.outputBuffers) { + auto it = batch->mBatchBufs.find(buffer.streamId); + if (it != batch->mBatchBufs.end()) { + InflightBatch::BufferBatch& bb = it->second; + bb.mBuffers.push_back(buffer); + filledStreams.push_back(buffer.streamId); + } else { + nonBatchedBuffers.push_back(buffer); + } + } + + // send non-batched buffers up + if (nonBatchedBuffers.size() > 0 || result.inputBuffer.streamId != -1) { + CaptureResult nonBatchedResult; + nonBatchedResult.frameNumber = result.frameNumber; + nonBatchedResult.outputBuffers = nonBatchedBuffers; + nonBatchedResult.inputBuffer = result.inputBuffer; + nonBatchedResult.partialResult = 0; // 0 for buffer only results + processOneCaptureResult(nonBatchedResult); + } + + if (result.frameNumber == batch->mLastFrame) { + // Send data up + if (result.partialResult > 0) { + sendBatchMetadataLocked(batch, result.partialResult); + } + // send buffer up + if (filledStreams.size() > 0) { + sendBatchBuffersLocked(batch, filledStreams); + } + } + } // end of batch lock scope + + // see if the batch is complete + if (result.frameNumber == batch->mLastFrame) { + checkAndRemoveFirstBatch(); + } +} + // Methods from ::android::hardware::camera::device::V3_2::ICameraDeviceSession follow. Return CameraDeviceSession::constructDefaultRequestSettings( RequestTemplate type, constructDefaultRequestSettings_cb _hidl_cb) { @@ -283,6 +645,16 @@ Return CameraDeviceSession::configureStreams( ++it; } } + + // Track video streams + mVideoStreamIds.clear(); + for (const auto& stream : requestedConfiguration.streams) { + if (stream.streamType == StreamType::OUTPUT && + stream.usage & graphics::allocator::V2_0::ConsumerUsage::VIDEO_ENCODER) { + mVideoStreamIds.push_back(stream.id); + } + } + mResultBatcher.setBatchedStreams(mVideoStreamIds); } if (ret == -EINVAL) { @@ -306,7 +678,26 @@ void CameraDeviceSession::cleanupBuffersLocked(int id) { mCirculatingBuffers.erase(id); } -Return CameraDeviceSession::processCaptureRequest(const CaptureRequest& request) { +Return CameraDeviceSession::processCaptureRequest( + const hidl_vec& requests, processCaptureRequest_cb _hidl_cb) { + uint32_t numRequestProcessed = 0; + Status s = Status::OK; + for (size_t i = 0; i < requests.size(); i++, numRequestProcessed++) { + s = processOneCaptureRequest(requests[i]); + if (s != Status::OK) { + break; + } + } + + if (s == Status::OK && requests.size() > 1) { + mResultBatcher.registerBatch(requests); + } + + _hidl_cb(s, numRequestProcessed); + return Void(); +} + +Status CameraDeviceSession::processOneCaptureRequest(const CaptureRequest& request) { Status status = initStatus(); if (status != Status::OK) { ALOGE("%s: camera init failed or disconnected", __FUNCTION__); @@ -437,7 +828,7 @@ void CameraDeviceSession::sProcessCaptureResult( bool hasInputBuf = (hal_result->input_buffer != nullptr); size_t numOutputBufs = hal_result->num_output_buffers; size_t numBufs = numOutputBufs + (hasInputBuf ? 1 : 0); - { + if (numBufs > 0) { Mutex::Autolock _l(d->mInflightLock); if (hasInputBuf) { int streamId = static_cast(hal_result->input_buffer->stream)->mId; @@ -463,10 +854,7 @@ void CameraDeviceSession::sProcessCaptureResult( } // We don't need to validate/import fences here since we will be passing them to camera service // within the scope of this function - CaptureResult result; - hidl_vec releaseFences; - releaseFences.resize(numBufs); result.frameNumber = frameNumber; result.partialResult = hal_result->partial_result; convertToHidl(hal_result->result, &result.result); @@ -477,11 +865,11 @@ void CameraDeviceSession::sProcessCaptureResult( result.inputBuffer.status = (BufferStatus) hal_result->input_buffer->status; // skip acquire fence since it's no use to camera service if (hal_result->input_buffer->release_fence != -1) { - releaseFences[numOutputBufs] = native_handle_create(/*numFds*/1, /*numInts*/0); - releaseFences[numOutputBufs]->data[0] = hal_result->input_buffer->release_fence; - result.inputBuffer.releaseFence = releaseFences[numOutputBufs]; + native_handle_t* handle = native_handle_create(/*numFds*/1, /*numInts*/0); + handle->data[0] = hal_result->input_buffer->release_fence; + result.inputBuffer.releaseFence = handle; } else { - releaseFences[numOutputBufs] = nullptr; + result.inputBuffer.releaseFence = nullptr; } } else { result.inputBuffer.streamId = -1; @@ -495,11 +883,11 @@ void CameraDeviceSession::sProcessCaptureResult( result.outputBuffers[i].status = (BufferStatus) hal_result->output_buffers[i].status; // skip acquire fence since it's of no use to camera service if (hal_result->output_buffers[i].release_fence != -1) { - releaseFences[i] = native_handle_create(/*numFds*/1, /*numInts*/0); - releaseFences[i]->data[0] = hal_result->output_buffers[i].release_fence; - result.outputBuffers[i].releaseFence = releaseFences[i]; + native_handle_t* handle = native_handle_create(/*numFds*/1, /*numInts*/0); + handle->data[0] = hal_result->output_buffers[i].release_fence; + result.outputBuffers[i].releaseFence = handle; } else { - releaseFences[i] = nullptr; + result.outputBuffers[i].releaseFence = nullptr; } } @@ -507,21 +895,17 @@ void CameraDeviceSession::sProcessCaptureResult( // Do this before call back to camera service because camera service might jump to // configure_streams right after the processCaptureResult call so we need to finish // updating inflight queues first - { + if (numBufs > 0) { Mutex::Autolock _l(d->mInflightLock); if (hasInputBuf) { int streamId = static_cast(hal_result->input_buffer->stream)->mId; auto key = std::make_pair(streamId, frameNumber); - // TODO (b/34169301): currently HAL closed the fence - //sHandleImporter.closeFence(d->mInflightBuffers[key].acquire_fence); d->mInflightBuffers.erase(key); } for (size_t i = 0; i < numOutputBufs; i++) { int streamId = static_cast(hal_result->output_buffers[i].stream)->mId; auto key = std::make_pair(streamId, frameNumber); - // TODO (b/34169301): currently HAL closed the fence - //sHandleImporter.closeFence(d->mInflightBuffers[key].acquire_fence); d->mInflightBuffers.erase(key); } @@ -530,12 +914,7 @@ void CameraDeviceSession::sProcessCaptureResult( } } - d->mCallback->processCaptureResult(result); - - for (size_t i = 0; i < releaseFences.size(); i++) { - // We don't close the FD here as HAL needs to signal it later. - native_handle_delete(releaseFences[i]); - } + d->mResultBatcher.processCaptureResult(result); } void CameraDeviceSession::sNotify( @@ -545,15 +924,16 @@ void CameraDeviceSession::sNotify( const_cast(static_cast(cb)); NotifyMsg hidlMsg; convertToHidl(msg, &hidlMsg); + if (hidlMsg.type == (MsgType) CAMERA3_MSG_ERROR && hidlMsg.msg.error.errorStreamId != -1) { if (d->mStreamMap.count(hidlMsg.msg.error.errorStreamId) != 1) { ALOGE("%s: unknown stream ID %d reports an error!", __FUNCTION__, hidlMsg.msg.error.errorStreamId); + return; } - return; } - d->mCallback->notify(hidlMsg); + d->mResultBatcher.notify(hidlMsg); } } // namespace implementation diff --git a/camera/device/3.2/default/CameraDeviceSession.h b/camera/device/3.2/default/CameraDeviceSession.h index 3786e4b2ba..8923c05bcf 100644 --- a/camera/device/3.2/default/CameraDeviceSession.h +++ b/camera/device/3.2/default/CameraDeviceSession.h @@ -17,6 +17,8 @@ #ifndef ANDROID_HARDWARE_CAMERA_DEVICE_V3_2_CAMERADEVICE3SESSION_H #define ANDROID_HARDWARE_CAMERA_DEVICE_V3_2_CAMERADEVICE3SESSION_H +#include +#include #include #include "hardware/camera_common.h" #include "hardware/camera3.h" @@ -27,6 +29,7 @@ #include #include #include "HandleImporter.h" +#include "CameraMetadata.h" namespace android { namespace hardware { @@ -64,7 +67,9 @@ extern "C" { struct CameraDeviceSession : public ICameraDeviceSession, private camera3_callback_ops { - CameraDeviceSession(camera3_device_t*, const sp&); + CameraDeviceSession(camera3_device_t*, + const camera_metadata_t* deviceInfo, + const sp&); ~CameraDeviceSession(); // Call by CameraDevice to dump active device states void dumpState(const native_handle_t* fd); @@ -75,9 +80,12 @@ struct CameraDeviceSession : public ICameraDeviceSession, private camera3_callba bool isClosed(); // Methods from ::android::hardware::camera::device::V3_2::ICameraDeviceSession follow. - Return constructDefaultRequestSettings(RequestTemplate type, constructDefaultRequestSettings_cb _hidl_cb) override; - Return configureStreams(const StreamConfiguration& requestedConfiguration, configureStreams_cb _hidl_cb) override; - Return processCaptureRequest(const CaptureRequest& request) override; + Return constructDefaultRequestSettings( + RequestTemplate type, constructDefaultRequestSettings_cb _hidl_cb) override; + Return configureStreams( + const StreamConfiguration& requestedConfiguration, configureStreams_cb _hidl_cb) override; + Return processCaptureRequest( + const hidl_vec& requests, processCaptureRequest_cb _hidl_cb) override; Return flush() override; Return close() override; @@ -94,7 +102,6 @@ private: bool mDisconnected = false; camera3_device_t* mDevice; - const sp mCallback; // Stream ID -> Camera3Stream cache std::map mStreamMap; @@ -114,6 +121,104 @@ private: static HandleImporter& sHandleImporter; bool mInitFail; + + common::V1_0::helper::CameraMetadata mDeviceInfo; + + class ResultBatcher { + public: + ResultBatcher(const sp& callback); + void setNumPartialResults(uint32_t n); + void setBatchedStreams(const std::vector& streamsToBatch); + + void registerBatch(const hidl_vec& requests); + void notify(NotifyMsg& msg); + void processCaptureResult(CaptureResult& result); + + private: + struct InflightBatch { + // Protect access to entire struct. Acquire this lock before read/write any data or + // calling any methods. processCaptureResult and notify will compete for this lock + // HIDL IPCs might be issued while the lock is held + Mutex mLock; + + bool allDelivered() const; + + uint32_t mFirstFrame; + uint32_t mLastFrame; + uint32_t mBatchSize; + + bool mShutterDelivered = false; + std::vector mShutterMsgs; + + struct BufferBatch { + bool mDelivered = false; + // This currently assumes every batched request will output to the batched stream + // and since HAL must always send buffers in order, no frameNumber tracking is + // needed + std::vector mBuffers; + }; + // Stream ID -> VideoBatch + std::unordered_map mBatchBufs; + + struct MetadataBatch { + // (frameNumber, metadata) + std::vector> mMds; + }; + // Partial result IDs that has been delivered to framework + uint32_t mNumPartialResults; + uint32_t mPartialResultProgress = 0; + // partialResult -> MetadataBatch + std::map mResultMds; + + // Set to true when batch is removed from mInflightBatches + // processCaptureResult and notify must check this flag after acquiring mLock to make + // sure this batch isn't removed while waiting for mLock + bool mRemoved = false; + }; + + static const int NOT_BATCHED = -1; + + // Get the batch index and pointer to InflightBatch (nullptrt if the frame is not batched) + // Caller must acquire the InflightBatch::mLock before accessing the InflightBatch + // It's possible that the InflightBatch is removed from mInflightBatches before the + // InflightBatch::mLock is acquired (most likely caused by an error notification), so + // caller must check InflightBatch::mRemoved flag after the lock is acquried. + // This method will hold ResultBatcher::mLock briefly + std::pair> getBatch(uint32_t frameNumber); + + // Check if the first batch in mInflightBatches is ready to be removed, and remove it if so + // This method will hold ResultBatcher::mLock briefly + void checkAndRemoveFirstBatch(); + + // The following sendXXXX methods must be called while the InflightBatch::mLock is locked + // HIDL IPC methods will be called during these methods. + void sendBatchShutterCbsLocked(std::shared_ptr batch); + // send buffers for all batched streams + void sendBatchBuffersLocked(std::shared_ptr batch); + // send buffers for specified streams + void sendBatchBuffersLocked( + std::shared_ptr batch, const std::vector& streams); + void sendBatchMetadataLocked( + std::shared_ptr batch, uint32_t lastPartialResultIdx); + // End of sendXXXX methods + + // helper methods + void freeReleaseFences(hidl_vec&); + void notifySingleMsg(NotifyMsg& msg); + void processOneCaptureResult(CaptureResult& result); + + // Protect access to mInflightBatches, mNumPartialResults and mStreamsToBatch + // processCaptureRequest, processCaptureResult, notify will compete for this lock + // Do NOT issue HIDL IPCs while holding this lock (except when HAL reports error) + mutable Mutex mLock; + std::deque> mInflightBatches; + uint32_t mNumPartialResults; + std::vector mStreamsToBatch; + const sp mCallback; + } mResultBatcher; + + std::vector mVideoStreamIds; + bool initialize(); Status initStatus() const; @@ -129,6 +234,7 @@ private: void cleanupBuffersLocked(int id); + Status processOneCaptureRequest(const CaptureRequest& request); /** * Static callback forwarding methods from HAL to instance */ diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index 598127f783..a79c9fafaa 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -445,13 +445,13 @@ public: hidl_vec getCameraDeviceNames(); struct EmptyDeviceCb : public ICameraDeviceCallback { - virtual Return processCaptureResult(const CaptureResult& /*result*/) override { + virtual Return processCaptureResult(const hidl_vec& /*results*/) override { ALOGI("processCaptureResult callback"); ADD_FAILURE(); // Empty callback should not reach here return Void(); } - virtual Return notify(const NotifyMsg& /*msg*/) override { + virtual Return notify(const hidl_vec& /*msgs*/) override { ALOGI("notify callback"); ADD_FAILURE(); // Empty callback should not reach here return Void(); @@ -460,8 +460,8 @@ public: struct DeviceCb : public ICameraDeviceCallback { DeviceCb(CameraHidlTest *parent) : mParent(parent) {} - Return processCaptureResult(const CaptureResult& result) override; - Return notify(const NotifyMsg& msg) override; + Return processCaptureResult(const hidl_vec& results) override; + Return notify(const hidl_vec& msgs) override; private: CameraHidlTest *mParent; // Parent object @@ -667,12 +667,13 @@ Return CameraHidlTest::Camera1DeviceCb::handleCallbackTimestamp( } Return CameraHidlTest::DeviceCb::processCaptureResult( - const CaptureResult& result) { + const hidl_vec& results) { if (nullptr == mParent) { return Void(); } std::unique_lock l(mParent->mLock); + const CaptureResult& result = results[0]; if(mParent->mResultFrameNumber != result.frameNumber) { ALOGE("%s: Unexpected frame number! Expected: %u received: %u", @@ -695,7 +696,8 @@ Return CameraHidlTest::DeviceCb::processCaptureResult( } Return CameraHidlTest::DeviceCb::notify( - const NotifyMsg& message) { + const hidl_vec& messages) { + const NotifyMsg& message = messages[0]; if (MsgType::ERROR == message.type) { { @@ -2477,10 +2479,17 @@ TEST_F(CameraHidlTest, processCaptureRequestPreview) { mResultFrameNumber = frameNumber; } - Return returnStatus = session->processCaptureRequest( - request); + Status status = Status::INTERNAL_ERROR; + uint32_t numRequestProcessed = 0; + Return returnStatus = session->processCaptureRequest( + {request}, + [&status, &numRequestProcessed] (auto s, uint32_t n) { + status = s; + numRequestProcessed = n; + }); ASSERT_TRUE(returnStatus.isOk()); - ASSERT_EQ(Status::OK, returnStatus); + ASSERT_EQ(Status::OK, status); + ASSERT_EQ(numRequestProcessed, 1u); { std::unique_lock l(mLock); @@ -2503,9 +2512,14 @@ TEST_F(CameraHidlTest, processCaptureRequestPreview) { } returnStatus = session->processCaptureRequest( - request); + {request}, + [&status, &numRequestProcessed] (auto s, uint32_t n) { + status = s; + numRequestProcessed = n; + }); ASSERT_TRUE(returnStatus.isOk()); - ASSERT_EQ(Status::OK, returnStatus); + ASSERT_EQ(Status::OK, status); + ASSERT_EQ(numRequestProcessed, 1u); { std::unique_lock l(mLock); @@ -2563,12 +2577,19 @@ TEST_F(CameraHidlTest, processCaptureRequestInvalidSinglePreview) { emptyInputBuffer, outputBuffers}; //Settings were not correctly initialized, we should fail here - Return returnStatus = session->processCaptureRequest( - request); - ASSERT_TRUE(returnStatus.isOk()); - ASSERT_EQ(Status::INTERNAL_ERROR, returnStatus); + Status status = Status::OK; + uint32_t numRequestProcessed = 0; + Return ret = session->processCaptureRequest( + {request}, + [&status, &numRequestProcessed] (auto s, uint32_t n) { + status = s; + numRequestProcessed = n; + }); + ASSERT_TRUE(ret.isOk()); + ASSERT_EQ(Status::INTERNAL_ERROR, status); + ASSERT_EQ(numRequestProcessed, 0u); - Return ret = session->close(); + ret = session->close(); ASSERT_TRUE(ret.isOk()); } } @@ -2609,11 +2630,17 @@ TEST_F(CameraHidlTest, processCaptureRequestInvalidBuffer) { emptyInputBuffer, emptyOutputBuffers}; //Output buffers are missing, we should fail here - Return returnStatus = session->processCaptureRequest( - request); - ASSERT_TRUE(returnStatus.isOk()); - ASSERT_EQ(Status::INTERNAL_ERROR, - returnStatus); + Status status = Status::OK; + uint32_t numRequestProcessed = 0; + ret = session->processCaptureRequest( + {request}, + [&status, &numRequestProcessed] (auto s, uint32_t n) { + status = s; + numRequestProcessed = n; + }); + ASSERT_TRUE(ret.isOk()); + ASSERT_EQ(Status::INTERNAL_ERROR, status); + ASSERT_EQ(numRequestProcessed, 0u); ret = session->close(); ASSERT_TRUE(ret.isOk()); @@ -2672,12 +2699,20 @@ TEST_F(CameraHidlTest, flushPreviewRequest) { mResultFrameNumber = frameNumber; } - Return returnStatus = session->processCaptureRequest( - request); - ASSERT_TRUE(returnStatus.isOk()); - ASSERT_EQ(Status::OK, returnStatus); + Status status = Status::INTERNAL_ERROR; + uint32_t numRequestProcessed = 0; + ret = session->processCaptureRequest( + {request}, + [&status, &numRequestProcessed] (auto s, uint32_t n) { + status = s; + numRequestProcessed = n; + }); + + ASSERT_TRUE(ret.isOk()); + ASSERT_EQ(Status::OK, status); + ASSERT_EQ(numRequestProcessed, 1u); //Flush before waiting for request to complete. - returnStatus = session->flush(); + Return returnStatus = session->flush(); ASSERT_TRUE(returnStatus.isOk()); ASSERT_EQ(Status::OK, returnStatus);