Refactor capture processing to be async friendly
Set up a series of methods that can be producers/consumers/callbacks.
For now, they call each other synchronously.
TEST: manually tested with app
BUG: 29334616
Change-Id: Icb6cf9d9970521d5291c54f02dc5032f43b26616
diff --git a/modules/camera/3_4/camera.cpp b/modules/camera/3_4/camera.cpp
index a1f7e86..7a34af5 100644
--- a/modules/camera/3_4/camera.cpp
+++ b/modules/camera/3_4/camera.cpp
@@ -383,121 +383,119 @@
return mTemplates[type]->getAndLock();
}
-int Camera::processCaptureRequest(camera3_capture_request_t *request)
+int Camera::processCaptureRequest(camera3_capture_request_t *temp_request)
{
int res;
- ALOGV("%s:%d: request=%p", __func__, mId, request);
+ ALOGV("%s:%d: request=%p", __func__, mId, temp_request);
ATRACE_CALL();
- if (request == NULL) {
+ if (temp_request == NULL) {
ALOGE("%s:%d: NULL request recieved", __func__, mId);
return -EINVAL;
}
- ALOGV("%s:%d: Request Frame:%d Settings:%p", __func__, mId,
- request->frame_number, request->settings);
+ // Make a persistent copy of request, since otherwise it won't live
+ // past the end of this method.
+ std::shared_ptr<CaptureRequest> request = std::make_shared<CaptureRequest>(temp_request);
- // NULL indicates use last settings
- if (request->settings == NULL && !mSettingsSet) {
- ALOGE("%s:%d: NULL settings without previous set Frame:%d Req:%p",
- __func__, mId, request->frame_number, request);
+ ALOGV("%s:%d: Request Frame:%d", __func__, mId,
+ request->frame_number);
+
+ // Null/Empty indicates use last settings
+ if (request->settings.isEmpty() && !mSettingsSet) {
+ ALOGE("%s:%d: NULL settings without previous set Frame:%d",
+ __func__, mId, request->frame_number);
return -EINVAL;
}
if (request->input_buffer != NULL) {
ALOGV("%s:%d: Reprocessing input buffer %p", __func__, mId,
- request->input_buffer);
-
- if (!isValidReprocessSettings(request->settings)) {
- ALOGE("%s:%d: Invalid settings for reprocess request: %p",
- __func__, mId, request->settings);
- return -EINVAL;
- }
+ request->input_buffer.get());
} else {
ALOGV("%s:%d: Capturing new frame.", __func__, mId);
-
- // Make a copy since CameraMetadata doesn't support weak ownership,
- // but |request| is supposed to maintain ownership.
- android::CameraMetadata request_settings;
- request_settings = request->settings;
- if (!isValidCaptureSettings(request_settings)) {
- ALOGE("%s:%d: Invalid settings for capture request: %p",
- __func__, mId, request->settings);
- return -EINVAL;
- }
- // Settings are valid, go ahead and set them.
- res = setSettings(request_settings);
- if (res) {
- ALOGE("%s:%d: Failed to set valid settings for capture request: %p",
- __func__, mId, request->settings);
- return res;
- }
- mSettingsSet = true;
}
- // Setup and process output buffers.
- if (request->num_output_buffers <= 0) {
- ALOGE("%s:%d: Invalid number of output buffers: %d", __func__, mId,
- request->num_output_buffers);
+ if (!isValidRequest(*request)) {
+ ALOGE("%s:%d: Invalid request.", __func__, mId);
return -EINVAL;
}
- camera3_capture_result result;
- result.num_output_buffers = request->num_output_buffers;
- std::vector<camera3_stream_buffer_t> output_buffers(
- result.num_output_buffers);
- for (unsigned int i = 0; i < request->num_output_buffers; i++) {
- res = processCaptureBuffer(&request->output_buffers[i],
- &output_buffers[i]);
+ // Valid settings have been provided (mSettingsSet is a misnomer;
+ // all that matters is that a previous request with valid settings
+ // has been passed to the device, not that they've been set).
+ mSettingsSet = true;
+
+ // Pre-process output buffers.
+ if (request->output_buffers.size() <= 0) {
+ ALOGE("%s:%d: Invalid number of output buffers: %d", __func__, mId,
+ request->output_buffers.size());
+ return -EINVAL;
+ }
+ for (auto& output_buffer : request->output_buffers) {
+ res = preprocessCaptureBuffer(&output_buffer);
if (res)
return -ENODEV;
}
- result.output_buffers = &output_buffers[0];
- // Get metadata for this frame. Since the framework guarantees only
- // one call to process_capture_request at a time, this call is guaranteed
- // to correspond with the most recently enqueued buffer.
+ // Send the request off to the device for completion.
+ enqueueRequest(request);
- android::CameraMetadata result_metadata;
- uint64_t timestamp = 0;
- // TODO(b/29334616): this may also want to use a callback, since
- // the shutter may not happen immediately.
- res = getResultSettings(&result_metadata, ×tamp);
- if (res) {
- return res;
- }
- result.result = result_metadata.release();
-
- // Notify the framework with the shutter time.
- result.frame_number = request->frame_number;
- notifyShutter(result.frame_number, timestamp);
-
- // TODO(b/29334616): asynchronously return results (the following should
- // be done once all enqueued buffers for the request complete and callback).
- result.partial_result = 1;
- mCallbackOps->process_capture_result(mCallbackOps, &result);
-
+ // Request is now in flight. The device will call completeRequest
+ // asynchronously when it is done filling buffers and metadata.
+ // TODO(b/31653306): Track requests in flight to ensure not too many are
+ // sent at a time, and so they can be dumped even if the device loses them.
return 0;
}
-bool Camera::isValidReprocessSettings(const camera_metadata_t* /*settings*/)
+void Camera::completeRequest(std::shared_ptr<CaptureRequest> request, int err)
{
- // TODO: reject settings that cannot be reprocessed
- // input buffers unimplemented, use this to reject reprocessing requests
- ALOGE("%s:%d: Input buffer reprocessing not implemented", __func__, mId);
- return false;
+ // TODO(b/31653306): make sure this is actually a request in flight,
+ // and not a random new one or a cancelled one. If so, stop tracking.
+
+ if (err) {
+ ALOGE("%s:%d: Error completing request for frame %d.",
+ __func__, mId, request->frame_number);
+ // TODO(b/31653322): Send REQUEST error.
+ return;
+ }
+
+ // Notify the framework with the shutter time (extracted from the result).
+ int64_t timestamp = 0;
+ // TODO(b/31360070): The general metadata methods should be part of the
+ // default_camera_hal namespace, not the v4l2_camera_hal namespace.
+ int res = v4l2_camera_hal::SingleTagValue(
+ request->settings, ANDROID_SENSOR_TIMESTAMP, ×tamp);
+ if (res) {
+ // TODO(b/31653322): Send RESULT error.
+ ALOGE("%s:%d: Request for frame %d is missing required metadata.",
+ __func__, mId, request->frame_number);
+ return;
+ }
+ notifyShutter(request->frame_number, timestamp);
+
+ // TODO(b/31653322): Check all returned buffers for errors
+ // (if any, send BUFFER error).
+
+ // Fill in the result struct
+ // (it only needs to live until the end of the framework callback).
+ camera3_capture_result_t result {
+ request->frame_number,
+ request->settings.getAndLock(),
+ request->output_buffers.size(),
+ request->output_buffers.data(),
+ request->input_buffer.get(),
+ 1 // Total result; only 1 part.
+ };
+ mCallbackOps->process_capture_result(mCallbackOps, &result);
}
-int Camera::processCaptureBuffer(const camera3_stream_buffer_t *in,
- camera3_stream_buffer_t *out)
+int Camera::preprocessCaptureBuffer(camera3_stream_buffer_t *buffer)
{
int res;
- // TODO(b/29334616): This probably should be non-blocking (currently blocks
- // here and on gralloc lock). Perhaps caller should put "in" in a queue
- // initially, then have a thread that dequeues from there and calls this
- // function.
- if (in->acquire_fence != -1) {
- res = sync_wait(in->acquire_fence, CAMERA_SYNC_TIMEOUT);
+ // TODO(b/29334616): This probably should be non-blocking; part
+ // of the asynchronous request processing.
+ if (buffer->acquire_fence != -1) {
+ res = sync_wait(buffer->acquire_fence, CAMERA_SYNC_TIMEOUT);
if (res == -ETIME) {
ALOGE("%s:%d: Timeout waiting on buffer acquire fence",
__func__, mId);
@@ -509,41 +507,17 @@
}
}
- out->stream = in->stream;
- out->buffer = in->buffer;
- out->status = CAMERA3_BUFFER_STATUS_OK;
+ // Acquire fence has been waited upon.
+ buffer->acquire_fence = -1;
+ // No release fence waiting unless the device sets it.
+ buffer->release_fence = -1;
- // Enqueue buffer for software-painting
- res = enqueueBuffer(out);
- if (res) {
- return res;
- }
-
- // TODO(b/29334616): This should be part of a callback made when the
- // enqueued buffer finishes painting.
- // TODO: use driver-backed release fences
- out->acquire_fence = -1;
- out->release_fence = -1;
+ buffer->status = CAMERA3_BUFFER_STATUS_OK;
return 0;
}
void Camera::notifyShutter(uint32_t frame_number, uint64_t timestamp)
{
- int res;
- struct timespec ts;
-
- // If timestamp is 0, get timestamp from right now instead
- if (timestamp == 0) {
- ALOGW("%s:%d: No timestamp provided, using CLOCK_BOOTTIME",
- __func__, mId);
- res = clock_gettime(CLOCK_BOOTTIME, &ts);
- if (res == 0) {
- timestamp = ts.tv_sec * 1000000000ULL + ts.tv_nsec;
- } else {
- ALOGE("%s:%d: No timestamp and failed to get CLOCK_BOOTTIME %s(%d)",
- __func__, mId, strerror(errno), errno);
- }
- }
camera3_notify_msg_t m;
memset(&m, 0, sizeof(m));
m.type = CAMERA3_MSG_SHUTTER;
diff --git a/modules/camera/3_4/camera.h b/modules/camera/3_4/camera.h
index 158c36d..3413683 100644
--- a/modules/camera/3_4/camera.h
+++ b/modules/camera/3_4/camera.h
@@ -24,6 +24,7 @@
#include <hardware/camera3.h>
#include <utils/Mutex.h>
+#include "capture_request.h"
#include "metadata/metadata.h"
#include "stream.h"
@@ -51,7 +52,7 @@
int initialize(const camera3_callback_ops_t *callback_ops);
int configureStreams(camera3_stream_configuration_t *stream_list);
const camera_metadata_t *constructDefaultRequestSettings(int type);
- int processCaptureRequest(camera3_capture_request_t *request);
+ int processCaptureRequest(camera3_capture_request_t *temp_request);
void dump(int fd);
@@ -73,22 +74,19 @@
// Set up the device for a stream, and get the maximum number of
// buffers that stream can handle (max_buffers is an output parameter)
virtual int setupStream(Stream* stream, uint32_t* max_buffers) = 0;
- // Verify settings are valid for a capture
- virtual bool isValidCaptureSettings(
- const android::CameraMetadata& settings) = 0;
- // Set settings for a capture
- virtual int setSettings(
- const android::CameraMetadata& new_settings) = 0;
+ // Verify settings are valid for a capture or reprocessing
+ virtual bool isValidRequest(const CaptureRequest& request) = 0;
// Separate initialization method for individual devices when opened
virtual int initDevice() = 0;
- // Enqueue a buffer to receive data from the camera
- virtual int enqueueBuffer(
- const camera3_stream_buffer_t *camera_buffer) = 0;
- // Get the shutter time and updated settings for the most recent frame.
- // The metadata parameter is both an input and output; frame-specific
- // result fields should be appended to what is passed in.
- virtual int getResultSettings(android::CameraMetadata* metadata,
- uint64_t *timestamp) = 0;
+ // Enqueue a request to receive data from the camera
+ virtual int enqueueRequest(
+ std::shared_ptr<CaptureRequest> request) = 0;
+
+ // Callback for when the device has filled in the requested data.
+ // Fills in the result struct, validates the data, sends appropriate
+ // notifications, and returns the result to the framework.
+ void completeRequest(
+ std::shared_ptr<CaptureRequest> request, int err);
// Prettyprint template names
const char* templateToString(int type);
@@ -105,9 +103,8 @@
int setupStreams(Stream **array, int count);
// Verify settings are valid for reprocessing an input buffer
bool isValidReprocessSettings(const camera_metadata_t *settings);
- // Process an output buffer
- int processCaptureBuffer(const camera3_stream_buffer_t *in,
- camera3_stream_buffer_t *out);
+ // Pre-process an output buffer
+ int preprocessCaptureBuffer(camera3_stream_buffer_t *buffer);
// Send a shutter notify message with start of exposure time
void notifyShutter(uint32_t frame_number, uint64_t timestamp);
// Is type a valid template type (and valid index into mTemplates)
diff --git a/modules/camera/3_4/v4l2_camera.cpp b/modules/camera/3_4/v4l2_camera.cpp
index 521f835..f7c75d2 100644
--- a/modules/camera/3_4/v4l2_camera.cpp
+++ b/modules/camera/3_4/v4l2_camera.cpp
@@ -164,70 +164,130 @@
return 0;
}
-int V4L2Camera::enqueueBuffer(const camera3_stream_buffer_t* camera_buffer) {
+int V4L2Camera::enqueueRequest(
+ std::shared_ptr<default_camera_hal::CaptureRequest> request) {
HAL_LOG_ENTER();
- int res = device_->EnqueueBuffer(camera_buffer);
- if (res) {
- HAL_LOGE("Device failed to enqueue buffer.");
- return res;
+ // Assume request validated before calling this function.
+ // (For now, always exactly 1 output buffer, no inputs).
+
+ {
+ std::lock_guard<std::mutex> guard(request_queue_lock_);
+ request_queue_.push(request);
}
- // Turn on the stream.
- // TODO(b/29334616): Lock around stream on/off access, only start stream
- // if not already on. (For now, since it's synchronous, it will always be
- // turned off before another call to this function).
- res = device_->StreamOn();
- if (res) {
- HAL_LOGE("Device failed to turn on stream.");
- return res;
- }
-
- // TODO(b/29334616): Enqueueing and dequeueing should be separate worker
- // threads, not in the same function.
-
- // Dequeue the buffer.
- v4l2_buffer result_buffer;
- res = device_->DequeueBuffer(&result_buffer);
- if (res) {
- HAL_LOGE("Device failed to dequeue buffer.");
- return res;
- }
-
- // All done, cleanup.
- // TODO(b/29334616): Lock around stream on/off access, only stop stream if
- // buffer queue is empty (synchronously, there's only ever 1 buffer in the
- // queue at a time, so this is safe).
- res = device_->StreamOff();
- if (res) {
- HAL_LOGE("Device failed to turn off stream.");
- return res;
- }
+ // TODO(b/29334616): Enqueueing of request buffers should be
+ // done by a consumer thread loop instead of here
+ enqueueRequestBuffers();
return 0;
}
-int V4L2Camera::getResultSettings(android::CameraMetadata* metadata,
- uint64_t* timestamp) {
- HAL_LOG_ENTER();
+std::shared_ptr<default_camera_hal::CaptureRequest>
+V4L2Camera::dequeueRequest() {
+ std::lock_guard<std::mutex> guard(request_queue_lock_);
+ if (request_queue_.empty()) {
+ return nullptr;
+ }
+ std::shared_ptr<default_camera_hal::CaptureRequest> request =
+ request_queue_.front();
+ request_queue_.pop();
+ return request;
+}
- // Get the results.
- int res = metadata_->FillResultMetadata(metadata);
- if (res) {
- HAL_LOGE("Failed to fill result metadata.");
- return res;
+void V4L2Camera::enqueueRequestBuffers() {
+ // Get a request from the queue.
+ std::shared_ptr<default_camera_hal::CaptureRequest> request =
+ dequeueRequest();
+ if (!request) {
+ return;
}
- // Extract the timestamp.
- int64_t frame_time = 0;
- res = SingleTagValue(*metadata, ANDROID_SENSOR_TIMESTAMP, &frame_time);
- if (res) {
- HAL_LOGE("Failed to extract timestamp from result metadata");
- return res;
- }
- *timestamp = static_cast<uint64_t>(frame_time);
+ // Assume request validated before being added to the queue
+ // (For now, always exactly 1 output buffer, no inputs).
+ {
+ std::lock_guard<std::mutex> guard(in_flight_lock_);
- return 0;
+ // Set the requested settings
+ int res = metadata_->SetRequestSettings(request->settings);
+ if (res) {
+ HAL_LOGE("Failed to set settings.");
+ completeRequest(request, res);
+ return;
+ }
+
+ // Replace the requested settings with a snapshot of
+ // the used settings/state.
+ res = metadata_->FillResultMetadata(&request->settings);
+ if (res) {
+ HAL_LOGE("Failed to fill result metadata.");
+ completeRequest(request, res);
+ return;
+ }
+
+ // Actually enqueue the buffer for capture.
+ res = device_->EnqueueBuffer(&request->output_buffers[0]);
+ if (res) {
+ HAL_LOGE("Device failed to enqueue buffer.");
+ completeRequest(request, res);
+ return;
+ }
+
+ if (in_flight_.empty()) {
+ // Nothing in flight, stream should be off, so it needs to be turned on.
+ res = device_->StreamOn();
+ if (res) {
+ HAL_LOGE("Device failed to turn on stream.");
+ completeRequest(request, res);
+ return;
+ }
+ }
+ in_flight_.push(request);
+ }
+
+ // TODO(b/29334616): Dequeueing of request buffers should be
+ // done by a consumer thread loop instead of here
+ dequeueRequestBuffers();
+}
+
+void V4L2Camera::dequeueRequestBuffers() {
+ std::shared_ptr<default_camera_hal::CaptureRequest> request;
+ {
+ std::lock_guard<std::mutex> guard(in_flight_lock_);
+ if (in_flight_.empty()) {
+ return;
+ }
+
+ request = in_flight_.front();
+ in_flight_.pop();
+
+ // Dequeue the buffer. For now, since each request has only 1 buffer,
+ // and the in_flight_lock_ is held while enqueueing and dequeuing
+ // from the device, just assume that the dequeued buffer corresponds
+ // to the dequeued request.
+ // TODO(b/31657008): in_flight_ should map buffer handles to requests;
+ // this consumer should just dequeue whatever it gets, then match
+ // that to a handle.
+ v4l2_buffer result_buffer;
+ int res = device_->DequeueBuffer(&result_buffer);
+ if (res) {
+ HAL_LOGE("Device failed to dequeue buffer.");
+ completeRequest(request, res);
+ return;
+ }
+
+ // All done, turn off the stream if it's empty.
+ if (in_flight_.empty()) {
+ res = device_->StreamOff();
+ if (res) {
+ HAL_LOGE("Device failed to turn off stream.");
+ completeRequest(request, res);
+ return;
+ }
+ }
+ }
+
+ completeRequest(request, 0);
}
bool V4L2Camera::isSupportedStreamSet(default_camera_hal::Stream** streams,
@@ -356,17 +416,22 @@
return 0;
}
-bool V4L2Camera::isValidCaptureSettings(
- const android::CameraMetadata& settings) {
+bool V4L2Camera::isValidRequest(
+ const default_camera_hal::CaptureRequest& request) {
HAL_LOG_ENTER();
- return metadata_->IsValidRequest(settings);
-}
+ if (request.input_buffer != nullptr) {
+ HAL_LOGE("Input buffer reprocessing not implemented.");
+ return false;
+ } else if (request.output_buffers.size() > 1) {
+ HAL_LOGE("Only 1 output buffer allowed per request.");
+ return false;
+ } else if (!metadata_->IsValidRequest(request.settings)) {
+ HAL_LOGE("Invalid request settings.");
+ return false;
+ }
-int V4L2Camera::setSettings(const android::CameraMetadata& new_settings) {
- HAL_LOG_ENTER();
-
- return metadata_->SetRequestSettings(new_settings);
+ return true;
}
} // namespace v4l2_camera_hal
diff --git a/modules/camera/3_4/v4l2_camera.h b/modules/camera/3_4/v4l2_camera.h
index 804012b..d299b9f 100644
--- a/modules/camera/3_4/v4l2_camera.h
+++ b/modules/camera/3_4/v4l2_camera.h
@@ -20,6 +20,7 @@
#define V4L2_CAMERA_HAL_V4L2_CAMERA_H_
#include <array>
+#include <queue>
#include <string>
#include <camera/CameraMetadata.h>
@@ -70,21 +71,30 @@
// buffers that stream can handle (max_buffers is an output parameter).
int setupStream(default_camera_hal::Stream* stream,
uint32_t* max_buffers) override;
- // Verify settings are valid for a capture with this device.
- bool isValidCaptureSettings(const android::CameraMetadata& settings) override;
- // Set settings for a capture.
- int setSettings(const android::CameraMetadata& new_settings) override;
- // Enqueue a buffer to receive data from the camera.
- int enqueueBuffer(const camera3_stream_buffer_t* camera_buffer) override;
- // Get the shutter time and updated settings for the most recent frame.
- // The metadata parameter is both an input and output; frame-specific
- // result fields should be appended to what is passed in.
- int getResultSettings(android::CameraMetadata* metadata, uint64_t* timestamp);
+ // Verify settings are valid for a capture or reprocessing.
+ bool isValidRequest(
+ const default_camera_hal::CaptureRequest& request) override;
+ // Enqueue a request to receive data from the camera.
+ int enqueueRequest(
+ std::shared_ptr<default_camera_hal::CaptureRequest> request) override;
+
+ // Async request processing.
+ // Dequeue a request from the waiting queue.
+ std::shared_ptr<default_camera_hal::CaptureRequest> dequeueRequest();
+ // Pass buffers for enqueued requests to the device.
+ void enqueueRequestBuffers();
+ // Retreive buffers from the device.
+ void dequeueRequestBuffers();
// V4L2 helper.
std::shared_ptr<V4L2Wrapper> device_;
std::unique_ptr<V4L2Wrapper::Connection> connection_;
std::unique_ptr<Metadata> metadata_;
+ std::mutex request_queue_lock_;
+ std::queue<std::shared_ptr<default_camera_hal::CaptureRequest>>
+ request_queue_;
+ std::mutex in_flight_lock_;
+ std::queue<std::shared_ptr<default_camera_hal::CaptureRequest>> in_flight_;
int32_t max_input_streams_;
std::array<int, 3> max_output_streams_; // {raw, non-stalling, stalling}.