From e82d7e476759fb76dc280ff4b8a4a4f246deb103 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 19 Oct 2021 17:17:59 +0530 Subject: android: camera_request: Don't embed full camera3_stream_buffer_t The camera3_stream_buffer_t structure is meant to communicate between the camera service and the HAL. They are short-live structures that don't outlive the .process_capture_request() operation (when queuing requests) or the .process_capture_result() callback. We currently store copies of the camera3_stream_buffer_t passed to .process_capture_request() in Camera3RequestDescriptor::StreamBuffer to store the structure members that the HAL need, and reuse them when calling the .process_capture_result() callback. This is conceptually not right, as the camera3_stream_buffer_t pass to the callback are not the same objects as the ones received in .process_capture_request(). Store individual fields of the camera3_stream_buffer_t in StreamBuffer instead of copying the whole structure. This gives the HAL full control of how data is stored, and properly decouples request queueing from result reporting. Signed-off-by: Laurent Pinchart Signed-off-by: Umang Jain Reviewed-by: Umang Jain Reviewed-by: Jacopo Mondi --- src/android/camera_device.cpp | 73 +++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 34 deletions(-) (limited to 'src/android/camera_device.cpp') diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 0bb547ae..0a2d3826 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -801,16 +801,8 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const { notifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST); - for (auto &buffer : descriptor->buffers_) { - /* - * Signal to the framework it has to handle fences that have not - * been waited on by setting the release fence to the acquire - * fence value. - */ - buffer.buffer.release_fence = buffer.buffer.acquire_fence; - buffer.buffer.acquire_fence = -1; - buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; - } + for (auto &buffer : descriptor->buffers_) + buffer.status = Camera3RequestDescriptor::Status::Error; descriptor->status_ = Camera3RequestDescriptor::Status::Error; } @@ -908,8 +900,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques << " with " << descriptor->buffers_.size() << " streams"; for (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) { - camera3_stream *camera3Stream = buffer.buffer.stream; - CameraStream *cameraStream = static_cast(camera3Stream->priv); + CameraStream *cameraStream = buffer.stream; + camera3_stream_t *camera3Stream = cameraStream->camera3Stream(); std::stringstream ss; ss << i << " - (" << camera3Stream->width << "x" @@ -944,11 +936,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * lifetime management only. */ buffer.frameBuffer = - createFrameBuffer(*buffer.buffer.buffer, + createFrameBuffer(*buffer.camera3Buffer, cameraStream->configuration().pixelFormat, cameraStream->configuration().size); frameBuffer = buffer.frameBuffer.get(); - acquireFence = buffer.buffer.acquire_fence; + acquireFence = buffer.fence; LOG(HAL, Debug) << ss.str() << " (direct)"; break; @@ -1055,12 +1047,11 @@ void CameraDevice::requestComplete(Request *request) /* * Prepare the capture result for the Android camera stack. * - * The buffer status is set to OK and later changed to ERROR if + * The buffer status is set to Success and later changed to Error if * post-processing/compression fails. */ for (auto &buffer : descriptor->buffers_) { - CameraStream *cameraStream = - static_cast(buffer.buffer.stream->priv); + CameraStream *stream = buffer.stream; /* * Streams of type Direct have been queued to the @@ -1073,10 +1064,9 @@ void CameraDevice::requestComplete(Request *request) * \todo Instrument the CameraWorker to set the acquire * fence to -1 once it has handled it and remove this check. */ - if (cameraStream->type() == CameraStream::Type::Direct) - buffer.buffer.acquire_fence = -1; - buffer.buffer.release_fence = -1; - buffer.buffer.status = CAMERA3_BUFFER_STATUS_OK; + if (stream->type() == CameraStream::Type::Direct) + buffer.fence = -1; + buffer.status = Camera3RequestDescriptor::Status::Success; } /* @@ -1128,33 +1118,32 @@ void CameraDevice::requestComplete(Request *request) /* Handle post-processing. */ for (auto &buffer : descriptor->buffers_) { - CameraStream *cameraStream = - static_cast(buffer.buffer.stream->priv); + CameraStream *stream = buffer.stream; - if (cameraStream->type() == CameraStream::Type::Direct) + if (stream->type() == CameraStream::Type::Direct) continue; - FrameBuffer *src = request->findBuffer(cameraStream->stream()); + FrameBuffer *src = request->findBuffer(stream->stream()); if (!src) { LOG(HAL, Error) << "Failed to find a source stream buffer"; - buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; - notifyError(descriptor->frameNumber_, buffer.buffer.stream, + buffer.status = Camera3RequestDescriptor::Status::Error; + notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); continue; } - int ret = cameraStream->process(*src, buffer, descriptor); + int ret = stream->process(*src, buffer, descriptor); /* * Return the FrameBuffer to the CameraStream now that we're * done processing it. */ - if (cameraStream->type() == CameraStream::Type::Internal) - cameraStream->putBuffer(src); + if (stream->type() == CameraStream::Type::Internal) + stream->putBuffer(src); if (ret) { - buffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR; - notifyError(descriptor->frameNumber_, buffer.buffer.stream, + buffer.status = Camera3RequestDescriptor::Status::Error; + notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); } } @@ -1185,8 +1174,24 @@ void CameraDevice::sendCaptureResults() captureResult.result = descriptor->resultMetadata_->get(); std::vector resultBuffers; - for (const auto &buffer : descriptor->buffers_) - resultBuffers.emplace_back(buffer.buffer); + resultBuffers.reserve(descriptor->buffers_.size()); + + for (const auto &buffer : descriptor->buffers_) { + camera3_buffer_status status = CAMERA3_BUFFER_STATUS_ERROR; + + if (buffer.status == Camera3RequestDescriptor::Status::Success) + status = CAMERA3_BUFFER_STATUS_OK; + + /* + * Pass the buffer fence back to the camera framework as + * a release fence. This instructs the framework to wait + * on the acquire fence in case we haven't done so + * ourselves for any reason. + */ + resultBuffers.push_back({ buffer.stream->camera3Stream(), + buffer.camera3Buffer, status, + -1, buffer.fence }); + } captureResult.num_output_buffers = resultBuffers.size(); captureResult.output_buffers = resultBuffers.data(); -- cgit v1.2.1