Merge "Camera: avoid copying of HalRequest"

1 files changed, 63 insertions, 62 deletions

diff --git a/camera/device/3.4/default/ExternalCameraDeviceSession.cpp b/camera/device/3.4/default/ExternalCameraDeviceSession.cpp
index a2f812ba76..5346f80399 100644
--- a/camera/device/3.4/default/ExternalCameraDeviceSession.cpp
+++ b/camera/device/3.4/default/ExternalCameraDeviceSession.cpp
@@ -526,15 +526,14 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques
         return Status::INTERNAL_ERROR;
     }
 
-    // TODO: reduce object copy in this path
-    HalRequest halReq = {
-            .frameNumber = request.frameNumber,
-            .setting = mLatestReqSetting,
-            .frameIn = frameIn,
-            .shutterTs = shutterTs};
-    halReq.buffers.resize(numOutputBufs);
+    std::shared_ptr<HalRequest> halReq = std::make_shared<HalRequest>();
+    halReq->frameNumber = request.frameNumber;
+    halReq->setting = mLatestReqSetting;
+    halReq->frameIn = frameIn;
+    halReq->shutterTs = shutterTs;
+    halReq->buffers.resize(numOutputBufs);
     for (size_t i = 0; i < numOutputBufs; i++) {
-        HalStreamBuffer& halBuf = halReq.buffers[i];
+        HalStreamBuffer& halBuf = halReq->buffers[i];
         int streamId = halBuf.streamId = request.outputBuffers[i].streamId;
         halBuf.bufferId = request.outputBuffers[i].bufferId;
         const Stream& stream = mStreamMap[streamId];
@@ -546,7 +545,7 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques
         halBuf.acquireFence = allFences[i];
         halBuf.fenceTimeout = false;
     }
-    mInflightFrames.insert(halReq.frameNumber);
+    mInflightFrames.insert(halReq->frameNumber);
     // Send request to OutputThread for the rest of processing
     mOutputThread->submitRequest(halReq);
     mFirstRequest = false;
@@ -572,30 +571,31 @@ void ExternalCameraDeviceSession::notifyError(
 }
 
 //TODO: refactor with processCaptureResult
-Status ExternalCameraDeviceSession::processCaptureRequestError(const HalRequest& req) {
+Status ExternalCameraDeviceSession::processCaptureRequestError(
+        const std::shared_ptr<HalRequest>& req) {
     // Return V4L2 buffer to V4L2 buffer queue
-    enqueueV4l2Frame(req.frameIn);
+    enqueueV4l2Frame(req->frameIn);
 
     // NotifyShutter
-    notifyShutter(req.frameNumber, req.shutterTs);
+    notifyShutter(req->frameNumber, req->shutterTs);
 
-    notifyError(/*frameNum*/req.frameNumber, /*stream*/-1, ErrorCode::ERROR_REQUEST);
+    notifyError(/*frameNum*/req->frameNumber, /*stream*/-1, ErrorCode::ERROR_REQUEST);
 
     // Fill output buffers
     hidl_vec<CaptureResult> results;
     results.resize(1);
     CaptureResult& result = results[0];
-    result.frameNumber = req.frameNumber;
+    result.frameNumber = req->frameNumber;
     result.partialResult = 1;
     result.inputBuffer.streamId = -1;
-    result.outputBuffers.resize(req.buffers.size());
-    for (size_t i = 0; i < req.buffers.size(); i++) {
-        result.outputBuffers[i].streamId = req.buffers[i].streamId;
-        result.outputBuffers[i].bufferId = req.buffers[i].bufferId;
+    result.outputBuffers.resize(req->buffers.size());
+    for (size_t i = 0; i < req->buffers.size(); i++) {
+        result.outputBuffers[i].streamId = req->buffers[i].streamId;
+        result.outputBuffers[i].bufferId = req->buffers[i].bufferId;
         result.outputBuffers[i].status = BufferStatus::ERROR;
-        if (req.buffers[i].acquireFence >= 0) {
+        if (req->buffers[i].acquireFence >= 0) {
             native_handle_t* handle = native_handle_create(/*numFds*/1, /*numInts*/0);
-            handle->data[0] = req.buffers[i].acquireFence;
+            handle->data[0] = req->buffers[i].acquireFence;
             result.outputBuffers[i].releaseFence.setTo(handle, /*shouldOwn*/false);
         }
     }
@@ -603,7 +603,7 @@ Status ExternalCameraDeviceSession::processCaptureRequestError(const HalRequest&
     // update inflight records
     {
         Mutex::Autolock _l(mLock);
-        mInflightFrames.erase(req.frameNumber);
+        mInflightFrames.erase(req->frameNumber);
     }
 
     // Callback into framework
@@ -612,51 +612,51 @@ Status ExternalCameraDeviceSession::processCaptureRequestError(const HalRequest&
     return Status::OK;
 }
 
-Status ExternalCameraDeviceSession::processCaptureResult(HalRequest& req) {
+Status ExternalCameraDeviceSession::processCaptureResult(std::shared_ptr<HalRequest>& req) {
     // Return V4L2 buffer to V4L2 buffer queue
-    enqueueV4l2Frame(req.frameIn);
+    enqueueV4l2Frame(req->frameIn);
 
     // NotifyShutter
-    notifyShutter(req.frameNumber, req.shutterTs);
+    notifyShutter(req->frameNumber, req->shutterTs);
 
     // Fill output buffers
     hidl_vec<CaptureResult> results;
     results.resize(1);
     CaptureResult& result = results[0];
-    result.frameNumber = req.frameNumber;
+    result.frameNumber = req->frameNumber;
     result.partialResult = 1;
     result.inputBuffer.streamId = -1;
-    result.outputBuffers.resize(req.buffers.size());
-    for (size_t i = 0; i < req.buffers.size(); i++) {
-        result.outputBuffers[i].streamId = req.buffers[i].streamId;
-        result.outputBuffers[i].bufferId = req.buffers[i].bufferId;
-        if (req.buffers[i].fenceTimeout) {
+    result.outputBuffers.resize(req->buffers.size());
+    for (size_t i = 0; i < req->buffers.size(); i++) {
+        result.outputBuffers[i].streamId = req->buffers[i].streamId;
+        result.outputBuffers[i].bufferId = req->buffers[i].bufferId;
+        if (req->buffers[i].fenceTimeout) {
             result.outputBuffers[i].status = BufferStatus::ERROR;
             native_handle_t* handle = native_handle_create(/*numFds*/1, /*numInts*/0);
-            handle->data[0] = req.buffers[i].acquireFence;
+            handle->data[0] = req->buffers[i].acquireFence;
             result.outputBuffers[i].releaseFence.setTo(handle, /*shouldOwn*/false);
-            notifyError(req.frameNumber, req.buffers[i].streamId, ErrorCode::ERROR_BUFFER);
+            notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);
         } else {
             result.outputBuffers[i].status = BufferStatus::OK;
             // TODO: refactor
-            if (req.buffers[i].acquireFence > 0) {
+            if (req->buffers[i].acquireFence > 0) {
                 native_handle_t* handle = native_handle_create(/*numFds*/1, /*numInts*/0);
-                handle->data[0] = req.buffers[i].acquireFence;
+                handle->data[0] = req->buffers[i].acquireFence;
                 result.outputBuffers[i].releaseFence.setTo(handle, /*shouldOwn*/false);
             }
         }
     }
 
     // Fill capture result metadata
-    fillCaptureResult(req.setting, req.shutterTs);
-    const camera_metadata_t *rawResult = req.setting.getAndLock();
+    fillCaptureResult(req->setting, req->shutterTs);
+    const camera_metadata_t *rawResult = req->setting.getAndLock();
     V3_2::implementation::convertToHidl(rawResult, &result.result);
-    req.setting.unlock(rawResult);
+    req->setting.unlock(rawResult);
 
     // update inflight records
     {
         Mutex::Autolock _l(mLock);
-        mInflightFrames.erase(req.frameNumber);
+        mInflightFrames.erase(req->frameNumber);
     }
 
     // Callback into framework
@@ -1387,7 +1387,7 @@ ssize_t ExternalCameraDeviceSession::getJpegBufferSize(
 
 int ExternalCameraDeviceSession::OutputThread::createJpegLocked(
         HalStreamBuffer &halBuf,
-        HalRequest &req)
+        const std::shared_ptr<HalRequest>& req)
 {
     int ret;
     auto lfail = [&](auto... args) {
@@ -1414,17 +1414,17 @@ int ExternalCameraDeviceSession::OutputThread::createJpegLocked(
     int jpegQuality, thumbQuality;
     Size thumbSize;
 
-    if (req.setting.exists(ANDROID_JPEG_QUALITY)) {
+    if (req->setting.exists(ANDROID_JPEG_QUALITY)) {
         camera_metadata_entry entry =
-            req.setting.find(ANDROID_JPEG_QUALITY);
+            req->setting.find(ANDROID_JPEG_QUALITY);
         jpegQuality = entry.data.u8[0];
     } else {
         return lfail("%s: ANDROID_JPEG_QUALITY not set",__FUNCTION__);
     }
 
-    if (req.setting.exists(ANDROID_JPEG_THUMBNAIL_QUALITY)) {
+    if (req->setting.exists(ANDROID_JPEG_THUMBNAIL_QUALITY)) {
         camera_metadata_entry entry =
-            req.setting.find(ANDROID_JPEG_THUMBNAIL_QUALITY);
+            req->setting.find(ANDROID_JPEG_THUMBNAIL_QUALITY);
         thumbQuality = entry.data.u8[0];
     } else {
         return lfail(
@@ -1432,9 +1432,9 @@ int ExternalCameraDeviceSession::OutputThread::createJpegLocked(
             __FUNCTION__);
     }
 
-    if (req.setting.exists(ANDROID_JPEG_THUMBNAIL_SIZE)) {
+    if (req->setting.exists(ANDROID_JPEG_THUMBNAIL_SIZE)) {
         camera_metadata_entry entry =
-            req.setting.find(ANDROID_JPEG_THUMBNAIL_SIZE);
+            req->setting.find(ANDROID_JPEG_THUMBNAIL_SIZE);
         thumbSize = Size { static_cast<uint32_t>(entry.data.i32[0]),
                            static_cast<uint32_t>(entry.data.i32[1])
         };
@@ -1494,7 +1494,7 @@ int ExternalCameraDeviceSession::OutputThread::createJpegLocked(
     /* Combine camera characteristics with request settings to form EXIF
      * metadata */
     common::V1_0::helper::CameraMetadata meta(parent->mCameraCharacteristics);
-    meta.append(req.setting);
+    meta.append(req->setting);
 
     /* Generate EXIF object */
     std::unique_ptr<ExifUtils> utils(ExifUtils::create());
@@ -1558,7 +1558,7 @@ int ExternalCameraDeviceSession::OutputThread::createJpegLocked(
 }
 
 bool ExternalCameraDeviceSession::OutputThread::threadLoop() {
-    HalRequest req;
+    std::shared_ptr<HalRequest> req;
     auto parent = mParent.promote();
     if (parent == nullptr) {
        ALOGE("%s: session has been disconnected!", __FUNCTION__);
@@ -1569,7 +1569,7 @@ bool ExternalCameraDeviceSession::OutputThread::threadLoop() {
     //       regularly to prevent v4l buffer queue filled with stale buffers
     //       when app doesn't program a preveiw request
     waitForNextRequest(&req);
-    if (req.frameIn == nullptr) {
+    if (req == nullptr) {
         // No new request, wait again
         return true;
     }
@@ -1577,17 +1577,17 @@ bool ExternalCameraDeviceSession::OutputThread::threadLoop() {
     auto onDeviceError = [&](auto... args) {
         ALOGE(args...);
         parent->notifyError(
-                req.frameNumber, /*stream*/-1, ErrorCode::ERROR_DEVICE);
+                req->frameNumber, /*stream*/-1, ErrorCode::ERROR_DEVICE);
         signalRequestDone();
         return false;
     };
 
-    if (req.frameIn->mFourcc != V4L2_PIX_FMT_MJPEG) {
+    if (req->frameIn->mFourcc != V4L2_PIX_FMT_MJPEG) {
         return onDeviceError("%s: do not support V4L2 format %c%c%c%c", __FUNCTION__,
-                req.frameIn->mFourcc & 0xFF,
-                (req.frameIn->mFourcc >> 8) & 0xFF,
-                (req.frameIn->mFourcc >> 16) & 0xFF,
-                (req.frameIn->mFourcc >> 24) & 0xFF);
+                req->frameIn->mFourcc & 0xFF,
+                (req->frameIn->mFourcc >> 8) & 0xFF,
+                (req->frameIn->mFourcc >> 16) & 0xFF,
+                (req->frameIn->mFourcc >> 24) & 0xFF);
     }
 
     std::unique_lock<std::mutex> lk(mBufferLock);
@@ -1595,7 +1595,7 @@ bool ExternalCameraDeviceSession::OutputThread::threadLoop() {
     // TODO: see if we can save some computation by converting to YV12 here
     uint8_t* inData;
     size_t inDataSize;
-    req.frameIn->map(&inData, &inDataSize);
+    req->frameIn->map(&inData, &inDataSize);
     // TODO: profile
     // TODO: in some special case maybe we can decode jpg directly to gralloc output?
     int res = libyuv::MJPGToI420(
@@ -1623,7 +1623,7 @@ bool ExternalCameraDeviceSession::OutputThread::threadLoop() {
 
     ALOGV("%s processing new request", __FUNCTION__);
     const int kSyncWaitTimeoutMs = 500;
-    for (auto& halBuf : req.buffers) {
+    for (auto& halBuf : req->buffers) {
         if (halBuf.acquireFence != -1) {
             int ret = sync_wait(halBuf.acquireFence, kSyncWaitTimeoutMs);
             if (ret) {
@@ -1780,9 +1780,9 @@ Status ExternalCameraDeviceSession::OutputThread::allocateIntermediateBuffers(
     return Status::OK;
 }
 
-Status ExternalCameraDeviceSession::OutputThread::submitRequest(const HalRequest& req) {
+Status ExternalCameraDeviceSession::OutputThread::submitRequest(
+        const std::shared_ptr<HalRequest>& req) {
     std::unique_lock<std::mutex> lk(mRequestListLock);
-    // TODO: reduce object copy in this path
     mRequestList.push_back(req);
     lk.unlock();
     mRequestCond.notify_one();
@@ -1797,7 +1797,7 @@ void ExternalCameraDeviceSession::OutputThread::flush() {
     }
 
     std::unique_lock<std::mutex> lk(mRequestListLock);
-    std::list<HalRequest> reqs = mRequestList;
+    std::list<std::shared_ptr<HalRequest>> reqs = std::move(mRequestList);
     mRequestList.clear();
     if (mProcessingRequest) {
         std::chrono::seconds timeout = std::chrono::seconds(kFlushWaitTimeoutSec);
@@ -1813,7 +1813,8 @@ void ExternalCameraDeviceSession::OutputThread::flush() {
     }
 }
 
-void ExternalCameraDeviceSession::OutputThread::waitForNextRequest(HalRequest* out) {
+void ExternalCameraDeviceSession::OutputThread::waitForNextRequest(
+        std::shared_ptr<HalRequest>* out) {
     if (out == nullptr) {
         ALOGE("%s: out is null", __FUNCTION__);
         return;
@@ -1838,7 +1839,7 @@ void ExternalCameraDeviceSession::OutputThread::waitForNextRequest(HalRequest* o
     *out = mRequestList.front();
     mRequestList.pop_front();
     mProcessingRequest = true;
-    mProcessingFrameNumer = out->frameNumber;
+    mProcessingFrameNumer = (*out)->frameNumber;
 }
 
 void ExternalCameraDeviceSession::OutputThread::signalRequestDone() {
@@ -1858,7 +1859,7 @@ void ExternalCameraDeviceSession::OutputThread::dump(int fd) {
     }
     dprintf(fd, "OutputThread request list contains frame: ");
     for (const auto& req : mRequestList) {
-        dprintf(fd, "%d, ", req.frameNumber);
+        dprintf(fd, "%d, ", req->frameNumber);
     }
     dprintf(fd, "\n");
 }