Camera3Device: support shutter after result
Move the code to remove in-flight requests from processCaptureResult
to a separate function so it can be called when the framework
receives a result or a shutter event. An in-flight request will only
be removed when both results and the shutter event arrive in the
case of a successful request.
Also send out results only after the shutter event receives.
Bug: 18135776
Change-Id: I340db1a495c711b0913784d43fd0f144871e4420
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 5281ea6..bba3905 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -427,7 +427,7 @@
InFlightRequest r = mInFlightMap.valueAt(i);
lines.appendFormat(" Frame %d | Timestamp: %" PRId64 ", metadata"
" arrived: %s, buffers left: %d\n", mInFlightMap.keyAt(i),
- r.captureTimestamp, r.haveResultMetadata ? "true" : "false",
+ r.shutterTimestamp, r.haveResultMetadata ? "true" : "false",
r.numBuffersLeft);
}
}
@@ -1880,6 +1880,131 @@
return true;
}
+
+void Camera3Device::returnOutputBuffers(
+ const camera3_stream_buffer_t *outputBuffers, size_t numBuffers,
+ nsecs_t timestamp) {
+ for (size_t i = 0; i < numBuffers; i++)
+ {
+ Camera3Stream *stream = Camera3Stream::cast(outputBuffers[i].stream);
+ status_t res = stream->returnBuffer(outputBuffers[i], timestamp);
+ // Note: stream may be deallocated at this point, if this buffer was
+ // the last reference to it.
+ if (res != OK) {
+ ALOGE("Can't return buffer to its stream: %s (%d)",
+ strerror(-res), res);
+ }
+ }
+}
+
+
+void Camera3Device::removeInFlightRequestIfReadyLocked(int idx) {
+
+ const InFlightRequest &request = mInFlightMap.valueAt(idx);
+ const uint32_t frameNumber = mInFlightMap.keyAt(idx);
+
+ nsecs_t sensorTimestamp = request.sensorTimestamp;
+ nsecs_t shutterTimestamp = request.shutterTimestamp;
+
+ // Check if it's okay to remove the request from InFlightMap:
+ // In the case of a successful request:
+ // all input and output buffers, all result metadata, shutter callback
+ // arrived.
+ // In the case of a unsuccessful request:
+ // all input and output buffers arrived.
+ if (request.numBuffersLeft == 0 &&
+ (request.requestStatus != OK ||
+ (request.haveResultMetadata && shutterTimestamp != 0))) {
+ ATRACE_ASYNC_END("frame capture", frameNumber);
+
+ // Sanity check - if sensor timestamp matches shutter timestamp
+ if (request.requestStatus == OK &&
+ sensorTimestamp != shutterTimestamp) {
+ SET_ERR("sensor timestamp (%" PRId64
+ ") for frame %d doesn't match shutter timestamp (%" PRId64 ")",
+ sensorTimestamp, frameNumber, shutterTimestamp);
+ }
+
+ // for an unsuccessful request, it may have pending output buffers to
+ // return.
+ assert(request.requestStatus != OK ||
+ request.pendingOutputBuffers.size() == 0);
+ returnOutputBuffers(request.pendingOutputBuffers.array(),
+ request.pendingOutputBuffers.size(), 0);
+
+ mInFlightMap.removeItemsAt(idx, 1);
+
+ ALOGVV("%s: removed frame %d from InFlightMap", __FUNCTION__, frameNumber);
+ }
+
+ // Sanity check - if we have too many in-flight frames, something has
+ // likely gone wrong
+ if (mInFlightMap.size() > kInFlightWarnLimit) {
+ CLOGE("In-flight list too large: %zu", mInFlightMap.size());
+ }
+}
+
+
+void Camera3Device::sendCaptureResult(CameraMetadata &pendingMetadata,
+ CaptureResultExtras &resultExtras,
+ CameraMetadata &collectedPartialResult,
+ uint32_t frameNumber) {
+ if (pendingMetadata.isEmpty())
+ return;
+
+ Mutex::Autolock l(mOutputLock);
+
+ // TODO: need to track errors for tighter bounds on expected frame number
+ if (frameNumber < mNextResultFrameNumber) {
+ SET_ERR("Out-of-order capture result metadata submitted! "
+ "(got frame number %d, expecting %d)",
+ frameNumber, mNextResultFrameNumber);
+ return;
+ }
+ mNextResultFrameNumber = frameNumber + 1;
+
+ CaptureResult captureResult;
+ captureResult.mResultExtras = resultExtras;
+ captureResult.mMetadata = pendingMetadata;
+
+ if (captureResult.mMetadata.update(ANDROID_REQUEST_FRAME_COUNT,
+ (int32_t*)&frameNumber, 1) != OK) {
+ SET_ERR("Failed to set frame# in metadata (%d)",
+ frameNumber);
+ return;
+ } else {
+ ALOGVV("%s: Camera %d: Set frame# in metadata (%d)",
+ __FUNCTION__, mId, frameNumber);
+ }
+
+ // Append any previous partials to form a complete result
+ if (mUsePartialResult && !collectedPartialResult.isEmpty()) {
+ captureResult.mMetadata.append(collectedPartialResult);
+ }
+
+ captureResult.mMetadata.sort();
+
+ // Check that there's a timestamp in the result metadata
+ camera_metadata_entry entry =
+ captureResult.mMetadata.find(ANDROID_SENSOR_TIMESTAMP);
+ if (entry.count == 0) {
+ SET_ERR("No timestamp provided by HAL for frame %d!",
+ frameNumber);
+ return;
+ }
+
+ // Valid result, insert into queue
+ List<CaptureResult>::iterator queuedResult =
+ mResultQueue.insert(mResultQueue.end(), CaptureResult(captureResult));
+ ALOGVV("%s: result requestId = %" PRId32 ", frameNumber = %" PRId64
+ ", burstId = %" PRId32, __FUNCTION__,
+ queuedResult->mResultExtras.requestId,
+ queuedResult->mResultExtras.frameNumber,
+ queuedResult->mResultExtras.burstId);
+
+ mResultSignal.signal();
+}
+
/**
* Camera HAL device callback methods
*/
@@ -1914,11 +2039,14 @@
CaptureResultExtras resultExtras;
bool hasInputBufferInRequest = false;
- // Get capture timestamp and resultExtras from list of in-flight requests,
- // where it was added by the shutter notification for this frame.
- // Then update the in-flight status and remove the in-flight entry if
- // all result data has been received.
- nsecs_t timestamp = 0;
+ // Get shutter timestamp and resultExtras from list of in-flight requests,
+ // where it was added by the shutter notification for this frame. If the
+ // shutter timestamp isn't received yet, append the output buffers to the
+ // in-flight request and they will be returned when the shutter timestamp
+ // arrives. Update the in-flight status and remove the in-flight entry if
+ // all result data and shutter timestamp have been received.
+ nsecs_t shutterTimestamp = 0;
+
{
Mutex::Autolock l(mInFlightLock);
ssize_t idx = mInFlightMap.indexOfKey(frameNumber);
@@ -1928,13 +2056,17 @@
return;
}
InFlightRequest &request = mInFlightMap.editValueAt(idx);
- ALOGVV("%s: got InFlightRequest requestId = %" PRId32 ", frameNumber = %" PRId64
- ", burstId = %" PRId32,
- __FUNCTION__, request.resultExtras.requestId, request.resultExtras.frameNumber,
- request.resultExtras.burstId);
- // Always update the partial count to the latest one. When framework aggregates adjacent
- // partial results into one, the latest partial count will be used.
- request.resultExtras.partialResultCount = result->partial_result;
+ ALOGVV("%s: got InFlightRequest requestId = %" PRId32
+ ", frameNumber = %" PRId64 ", burstId = %" PRId32
+ ", partialResultCount = %d",
+ __FUNCTION__, request.resultExtras.requestId,
+ request.resultExtras.frameNumber, request.resultExtras.burstId,
+ result->partial_result);
+ // Always update the partial count to the latest one if it's not 0
+ // (buffers only). When framework aggregates adjacent partial results
+ // into one, the latest partial count will be used.
+ if (result->partial_result != 0)
+ request.resultExtras.partialResultCount = result->partial_result;
// Check if this result carries only partial metadata
if (mUsePartialResult && result->result != NULL) {
@@ -1978,22 +2110,9 @@
}
}
- timestamp = request.captureTimestamp;
- resultExtras = request.resultExtras;
+ shutterTimestamp = request.shutterTimestamp;
hasInputBufferInRequest = request.hasInputBuffer;
- /**
- * One of the following must happen before it's legal to call process_capture_result,
- * unless partial metadata is being provided:
- * - CAMERA3_MSG_SHUTTER (expected during normal operation)
- * - CAMERA3_MSG_ERROR (expected during flush)
- */
- if (request.requestStatus == OK && timestamp == 0 && !isPartialResult) {
- SET_ERR("Called before shutter notify for frame %d",
- frameNumber);
- return;
- }
-
// Did we get the (final) result metadata for this capture?
if (result->result != NULL && !isPartialResult) {
if (request.haveResultMetadata) {
@@ -2026,103 +2145,38 @@
return;
}
- // Check if everything has arrived for this result (buffers and metadata), remove it from
- // InFlightMap if both arrived or HAL reports error for this request (i.e. during flush).
- // For per-frame error notifications, camera3.h requirements state that all the
- // buffer handles for a failed frame capture must be returned via process_capture_result()
- // call(s). Hence, Camera3Device needs to ensure that the frame entry is not deleted from
- // mInFlightMap until all buffers for that frame have been returned by HAL.
- if ((request.numBuffersLeft == 0) &&
- ((request.requestStatus != OK) || (request.haveResultMetadata))) {
- ATRACE_ASYNC_END("frame capture", frameNumber);
- mInFlightMap.removeItemsAt(idx, 1);
+ camera_metadata_ro_entry_t entry;
+ res = find_camera_metadata_ro_entry(result->result,
+ ANDROID_SENSOR_TIMESTAMP, &entry);
+ if (res == OK && entry.count == 1) {
+ request.sensorTimestamp = entry.data.i64[0];
}
- // Sanity check - if we have too many in-flight frames, something has
- // likely gone wrong
- if (mInFlightMap.size() > kInFlightWarnLimit) {
- CLOGE("In-flight list too large: %zu", mInFlightMap.size());
- }
-
- }
-
- // Process the result metadata, if provided
- bool gotResult = false;
- if (result->result != NULL && !isPartialResult) {
- Mutex::Autolock l(mOutputLock);
-
- gotResult = true;
-
- // TODO: need to track errors for tighter bounds on expected frame number
- if (frameNumber < mNextResultFrameNumber) {
- SET_ERR("Out-of-order capture result metadata submitted! "
- "(got frame number %d, expecting %d)",
- frameNumber, mNextResultFrameNumber);
- return;
- }
- mNextResultFrameNumber = frameNumber + 1;
-
- CaptureResult captureResult;
- captureResult.mResultExtras = resultExtras;
- captureResult.mMetadata = result->result;
-
- if (captureResult.mMetadata.update(ANDROID_REQUEST_FRAME_COUNT,
- (int32_t*)&frameNumber, 1) != OK) {
- SET_ERR("Failed to set frame# in metadata (%d)",
- frameNumber);
- gotResult = false;
+ // If shutter event isn't received yet, append the output buffers to
+ // the in-flight request. Otherwise, return the output buffers to
+ // streams.
+ if (shutterTimestamp == 0) {
+ request.pendingOutputBuffers.appendArray(result->output_buffers,
+ result->num_output_buffers);
} else {
- ALOGVV("%s: Camera %d: Set frame# in metadata (%d)",
- __FUNCTION__, mId, frameNumber);
+ returnOutputBuffers(result->output_buffers,
+ result->num_output_buffers, shutterTimestamp);
}
- // Append any previous partials to form a complete result
- if (mUsePartialResult && !collectedPartialResult.isEmpty()) {
- captureResult.mMetadata.append(collectedPartialResult);
+ if (result->result != NULL && !isPartialResult) {
+ if (shutterTimestamp == 0) {
+ request.pendingMetadata = result->result;
+ request.partialResult.collectedResult = collectedPartialResult;
+ } else {
+ CameraMetadata metadata;
+ metadata = result->result;
+ sendCaptureResult(metadata, request.resultExtras,
+ collectedPartialResult, frameNumber);
+ }
}
- captureResult.mMetadata.sort();
-
- // Check that there's a timestamp in the result metadata
-
- camera_metadata_entry entry =
- captureResult.mMetadata.find(ANDROID_SENSOR_TIMESTAMP);
- if (entry.count == 0) {
- SET_ERR("No timestamp provided by HAL for frame %d!",
- frameNumber);
- gotResult = false;
- } else if (timestamp != entry.data.i64[0]) {
- SET_ERR("Timestamp mismatch between shutter notify and result"
- " metadata for frame %d (%" PRId64 " vs %" PRId64 " respectively)",
- frameNumber, timestamp, entry.data.i64[0]);
- gotResult = false;
- }
-
- if (gotResult) {
- // Valid result, insert into queue
- List<CaptureResult>::iterator queuedResult =
- mResultQueue.insert(mResultQueue.end(), CaptureResult(captureResult));
- ALOGVV("%s: result requestId = %" PRId32 ", frameNumber = %" PRId64
- ", burstId = %" PRId32, __FUNCTION__,
- queuedResult->mResultExtras.requestId,
- queuedResult->mResultExtras.frameNumber,
- queuedResult->mResultExtras.burstId);
- }
- } // scope for mOutputLock
-
- // Return completed buffers to their streams with the timestamp
-
- for (size_t i = 0; i < result->num_output_buffers; i++) {
- Camera3Stream *stream =
- Camera3Stream::cast(result->output_buffers[i].stream);
- res = stream->returnBuffer(result->output_buffers[i], timestamp);
- // Note: stream may be deallocated at this point, if this buffer was the
- // last reference to it.
- if (res != OK) {
- ALOGE("Can't return buffer %zu for frame %d to its stream: "
- " %s (%d)", i, frameNumber, strerror(-res), res);
- }
- }
+ removeInFlightRequestIfReadyLocked(idx);
+ } // scope for mInFlightLock
if (result->input_buffer != NULL) {
if (hasInputBufferInRequest) {
@@ -2142,13 +2196,6 @@
__FUNCTION__);
}
}
-
- // Finally, signal any waiters for new frames
-
- if (gotResult) {
- mResultSignal.signal();
- }
-
}
void Camera3Device::notify(const camera3_notify_msg *msg) {
@@ -2266,8 +2313,6 @@
mNextShutterFrameNumber = msg.frame_number + 1;
}
- CaptureResultExtras resultExtras;
-
// Set timestamp for the request in the in-flight tracking
// and get the request ID to send upstream
{
@@ -2275,21 +2320,30 @@
idx = mInFlightMap.indexOfKey(msg.frame_number);
if (idx >= 0) {
InFlightRequest &r = mInFlightMap.editValueAt(idx);
- r.captureTimestamp = msg.timestamp;
- resultExtras = r.resultExtras;
+
+ ALOGVV("Camera %d: %s: Shutter fired for frame %d (id %d) at %" PRId64,
+ mId, __FUNCTION__,
+ msg.frame_number, r.resultExtras.requestId, msg.timestamp);
+ // Call listener, if any
+ if (listener != NULL) {
+ listener->notifyShutter(r.resultExtras, msg.timestamp);
+ }
+
+ r.shutterTimestamp = msg.timestamp;
+
+ // send pending result and buffers
+ sendCaptureResult(r.pendingMetadata, r.resultExtras,
+ r.partialResult.collectedResult, msg.frame_number);
+ returnOutputBuffers(r.pendingOutputBuffers.array(),
+ r.pendingOutputBuffers.size(), r.shutterTimestamp);
+ r.pendingOutputBuffers.clear();
+
+ removeInFlightRequestIfReadyLocked(idx);
}
}
if (idx < 0) {
SET_ERR("Shutter notification for non-existent frame number %d",
msg.frame_number);
- return;
- }
- ALOGVV("Camera %d: %s: Shutter fired for frame %d (id %d) at %" PRId64,
- mId, __FUNCTION__,
- msg.frame_number, resultExtras.requestId, msg.timestamp);
- // Call listener, if any
- if (listener != NULL) {
- listener->notifyShutter(resultExtras, msg.timestamp);
}
}
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index ec6bba1..ec8dc10 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -521,7 +521,9 @@
struct InFlightRequest {
// Set by notify() SHUTTER call.
- nsecs_t captureTimestamp;
+ nsecs_t shutterTimestamp;
+ // Set by process_capture_result().
+ nsecs_t sensorTimestamp;
int requestStatus;
// Set by process_capture_result call with valid metadata
bool haveResultMetadata;
@@ -532,6 +534,21 @@
// If this request has any input buffer
bool hasInputBuffer;
+
+ // The last metadata that framework receives from HAL and
+ // not yet send out because the shutter event hasn't arrived.
+ // It's added by process_capture_result and sent when framework
+ // receives the shutter event.
+ CameraMetadata pendingMetadata;
+
+ // Buffers are added by process_capture_result when output buffers
+ // return from HAL but framework has not yet received the shutter
+ // event. They will be returned to the streams when framework receives
+ // the shutter event.
+ Vector<camera3_stream_buffer_t> pendingOutputBuffers;
+
+
+
// Fields used by the partial result only
struct PartialResultInFlight {
// Set by process_capture_result once 3A has been sent to clients
@@ -546,7 +563,8 @@
// Default constructor needed by KeyedVector
InFlightRequest() :
- captureTimestamp(0),
+ shutterTimestamp(0),
+ sensorTimestamp(0),
requestStatus(OK),
haveResultMetadata(false),
numBuffersLeft(0),
@@ -554,7 +572,8 @@
}
InFlightRequest(int numBuffers) :
- captureTimestamp(0),
+ shutterTimestamp(0),
+ sensorTimestamp(0),
requestStatus(OK),
haveResultMetadata(false),
numBuffersLeft(numBuffers),
@@ -562,7 +581,8 @@
}
InFlightRequest(int numBuffers, CaptureResultExtras extras) :
- captureTimestamp(0),
+ shutterTimestamp(0),
+ sensorTimestamp(0),
requestStatus(OK),
haveResultMetadata(false),
numBuffersLeft(numBuffers),
@@ -571,7 +591,8 @@
}
InFlightRequest(int numBuffers, CaptureResultExtras extras, bool hasInput) :
- captureTimestamp(0),
+ shutterTimestamp(0),
+ sensorTimestamp(0),
requestStatus(OK),
haveResultMetadata(false),
numBuffersLeft(numBuffers),
@@ -639,6 +660,24 @@
void notifyShutter(const camera3_shutter_msg_t &msg,
NotificationListener *listener);
+ // helper function to return the output buffers to the streams.
+ void returnOutputBuffers(const camera3_stream_buffer_t *outputBuffers,
+ size_t numBuffers, nsecs_t timestamp);
+
+ // Insert the capture result given the pending metadata, result extras,
+ // partial results, and the frame number to the result queue.
+ void sendCaptureResult(CameraMetadata &pendingMetadata,
+ CaptureResultExtras &resultExtras,
+ CameraMetadata &collectedPartialResult, uint32_t frameNumber);
+
+ /**** Scope for mInFlightLock ****/
+
+ // Remove the in-flight request of the given index from mInFlightMap
+ // if it's no longer needed. It must only be called with mInFlightLock held.
+ void removeInFlightRequestIfReadyLocked(int idx);
+
+ /**** End scope for mInFlightLock ****/
+
/**
* Static callback forwarding methods from HAL to instance
*/