From 26abaf466f053b033ca678c8f4972c51a4b38d39 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Tue, 28 Aug 2018 15:41:20 -0700 Subject: [PATCH] Camera: Make sure timestamp is increasing Guard against case where timestamp of stream buffers doesn't increase. Timestamp source is either monotonic or boottime, both of which are guaranteed to increase. There are 2 exceptions where timestamp may go back in time: - For requests where HAL ZSL is enabled, and - For reprocessing requests Test: Camera CTS Bug: 113340974 Change-Id: I0ae9290696d651168150fa5ed7aa13c140a0294f --- .../device3/Camera3Device.cpp | 24 +++++++++++++------ .../libcameraservice/device3/Camera3Device.h | 16 +++++++++---- .../device3/Camera3OutputStream.cpp | 5 ++++ .../device3/Camera3Stream.cpp | 12 ++++++++-- .../libcameraservice/device3/Camera3Stream.h | 3 ++- .../device3/Camera3StreamInterface.h | 2 +- 6 files changed, 46 insertions(+), 16 deletions(-) diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp index 765640742e..cbcc85b798 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.cpp +++ b/services/camera/libcameraservice/device3/Camera3Device.cpp @@ -2742,13 +2742,14 @@ void Camera3Device::setErrorStateLockedV(const char *fmt, va_list args) { status_t Camera3Device::registerInFlight(uint32_t frameNumber, int32_t numBuffers, CaptureResultExtras resultExtras, bool hasInput, bool hasAppCallback, nsecs_t maxExpectedDuration, - std::set& physicalCameraIds, bool isStillCapture) { + std::set& physicalCameraIds, bool isStillCapture, + bool isZslCapture) { ATRACE_CALL(); Mutex::Autolock l(mInFlightLock); ssize_t res; res = mInFlightMap.add(frameNumber, InFlightRequest(numBuffers, resultExtras, hasInput, - hasAppCallback, maxExpectedDuration, physicalCameraIds, isStillCapture)); + hasAppCallback, maxExpectedDuration, physicalCameraIds, isStillCapture, isZslCapture)); if (res < 0) return res; if (mInFlightMap.size() == 1) { @@ -2766,11 +2767,12 @@ status_t Camera3Device::registerInFlight(uint32_t frameNumber, void Camera3Device::returnOutputBuffers( const camera3_stream_buffer_t *outputBuffers, size_t numBuffers, - nsecs_t timestamp) { + nsecs_t timestamp, bool timestampIncreasing) { + for (size_t i = 0; i < numBuffers; i++) { Camera3Stream *stream = Camera3Stream::cast(outputBuffers[i].stream); - status_t res = stream->returnBuffer(outputBuffers[i], timestamp); + status_t res = stream->returnBuffer(outputBuffers[i], timestamp, timestampIncreasing); // Note: stream may be deallocated at this point, if this buffer was // the last reference to it. if (res != OK) { @@ -3214,8 +3216,9 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) { request.pendingOutputBuffers.appendArray(result->output_buffers, result->num_output_buffers); } else { + bool timestampIncreasing = !(request.zslCapture || request.hasInputBuffer); returnOutputBuffers(result->output_buffers, - result->num_output_buffers, shutterTimestamp); + result->num_output_buffers, shutterTimestamp, timestampIncreasing); } if (result->result != NULL && !isPartialResult) { @@ -3421,8 +3424,9 @@ void Camera3Device::notifyShutter(const camera3_shutter_msg_t &msg, r.collectedPartialResult, msg.frame_number, r.hasInputBuffer, r.physicalMetadatas); } + bool timestampIncreasing = !(r.zslCapture || r.hasInputBuffer); returnOutputBuffers(r.pendingOutputBuffers.array(), - r.pendingOutputBuffers.size(), r.shutterTimestamp); + r.pendingOutputBuffers.size(), r.shutterTimestamp, timestampIncreasing); r.pendingOutputBuffers.clear(); removeInFlightRequestIfReadyLocked(idx); @@ -4948,6 +4952,7 @@ status_t Camera3Device::RequestThread::prepareHalRequests() { hasCallback = false; } bool isStillCapture = false; + bool isZslCapture = false; if (!mNextRequests[0].captureRequest->mSettingsList.begin()->metadata.isEmpty()) { camera_metadata_ro_entry_t e = camera_metadata_ro_entry_t(); find_camera_metadata_ro_entry(halRequest->settings, ANDROID_CONTROL_CAPTURE_INTENT, &e); @@ -4955,13 +4960,18 @@ status_t Camera3Device::RequestThread::prepareHalRequests() { isStillCapture = true; ATRACE_ASYNC_BEGIN("still capture", mNextRequests[i].halRequest.frame_number); } + + find_camera_metadata_ro_entry(halRequest->settings, ANDROID_CONTROL_ENABLE_ZSL, &e); + if ((e.count > 0) && (e.data.u8[0] == ANDROID_CONTROL_ENABLE_ZSL_TRUE)) { + isZslCapture = true; + } } res = parent->registerInFlight(halRequest->frame_number, totalNumBuffers, captureRequest->mResultExtras, /*hasInput*/halRequest->input_buffer != NULL, hasCallback, calculateMaxExpectedDuration(halRequest->settings), - requestedPhysicalCameras, isStillCapture); + requestedPhysicalCameras, isStillCapture, isZslCapture); ALOGVV("%s: registered in flight requestId = %" PRId32 ", frameNumber = %" PRId64 ", burstId = %" PRId32 ".", __FUNCTION__, diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h index 85f9614481..159f2caf97 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.h +++ b/services/camera/libcameraservice/device3/Camera3Device.h @@ -999,6 +999,9 @@ class Camera3Device : // Indicates a still capture request. bool stillCapture; + // Indicates a ZSL capture request + bool zslCapture; + // Default constructor needed by KeyedVector InFlightRequest() : shutterTimestamp(0), @@ -1010,12 +1013,14 @@ class Camera3Device : hasCallback(true), maxExpectedDuration(kDefaultExpectedDuration), skipResultMetadata(false), - stillCapture(false) { + stillCapture(false), + zslCapture(false) { } InFlightRequest(int numBuffers, CaptureResultExtras extras, bool hasInput, bool hasAppCallback, nsecs_t maxDuration, - const std::set& physicalCameraIdSet, bool isStillCapture) : + const std::set& physicalCameraIdSet, bool isStillCapture, + bool isZslCapture) : shutterTimestamp(0), sensorTimestamp(0), requestStatus(OK), @@ -1027,7 +1032,8 @@ class Camera3Device : maxExpectedDuration(maxDuration), skipResultMetadata(false), physicalCameraIds(physicalCameraIdSet), - stillCapture(isStillCapture) { + stillCapture(isStillCapture), + zslCapture(isZslCapture) { } }; @@ -1044,7 +1050,7 @@ class Camera3Device : status_t registerInFlight(uint32_t frameNumber, int32_t numBuffers, CaptureResultExtras resultExtras, bool hasInput, bool callback, nsecs_t maxExpectedDuration, std::set& physicalCameraIds, - bool isStillCapture); + bool isStillCapture, bool isZslCapture); /** * Returns the maximum expected time it'll take for all currently in-flight @@ -1154,7 +1160,7 @@ class Camera3Device : // helper function to return the output buffers to the streams. void returnOutputBuffers(const camera3_stream_buffer_t *outputBuffers, - size_t numBuffers, nsecs_t timestamp); + size_t numBuffers, nsecs_t timestamp, bool timestampIncreasing = true); // Send a partial capture result. void sendPartialCaptureResult(const camera_metadata_t * partialResult, diff --git a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp index 2c020a2797..ec3f35f060 100644 --- a/services/camera/libcameraservice/device3/Camera3OutputStream.cpp +++ b/services/camera/libcameraservice/device3/Camera3OutputStream.cpp @@ -268,6 +268,11 @@ status_t Camera3OutputStream::returnBufferCheckedLocked( mTraceFirstBuffer = false; } + if (timestamp == 0) { + ALOGE("%s: Stream %d: timestamp shouldn't be 0", __FUNCTION__, mId); + return BAD_VALUE; + } + /* Certain consumers (such as AudioSource or HardwareComposer) use * MONOTONIC time, causing time misalignment if camera timestamp is * in BOOTTIME. Do the conversion if necessary. */ diff --git a/services/camera/libcameraservice/device3/Camera3Stream.cpp b/services/camera/libcameraservice/device3/Camera3Stream.cpp index 6030d15f50..53ff9fe080 100644 --- a/services/camera/libcameraservice/device3/Camera3Stream.cpp +++ b/services/camera/libcameraservice/device3/Camera3Stream.cpp @@ -66,7 +66,8 @@ Camera3Stream::Camera3Stream(int id, mBufferLimitLatency(kBufferLimitLatencyBinSize), mFormatOverridden(false), mOriginalFormat(-1), - mPhysicalCameraId(physicalCameraId) { + mPhysicalCameraId(physicalCameraId), + mLastTimestamp(0) { camera3_stream::stream_type = type; camera3_stream::width = width; @@ -649,7 +650,7 @@ void Camera3Stream::removeOutstandingBuffer(const camera3_stream_buffer &buffer) } status_t Camera3Stream::returnBuffer(const camera3_stream_buffer &buffer, - nsecs_t timestamp) { + nsecs_t timestamp, bool timestampIncreasing) { ATRACE_CALL(); Mutex::Autolock l(mLock); @@ -661,6 +662,13 @@ status_t Camera3Stream::returnBuffer(const camera3_stream_buffer &buffer, removeOutstandingBuffer(buffer); + if (timestampIncreasing && timestamp != 0 && timestamp <= mLastTimestamp) { + ALOGE("%s: Stream %d: timestamp %" PRId64 " is not increasing. Prev timestamp %" PRId64, + __FUNCTION__, mId, timestamp, mLastTimestamp); + return BAD_VALUE; + } + mLastTimestamp = timestamp; + /** * TODO: Check that the state is valid first. * diff --git a/services/camera/libcameraservice/device3/Camera3Stream.h b/services/camera/libcameraservice/device3/Camera3Stream.h index 4ddcf1ab85..e30fc598c4 100644 --- a/services/camera/libcameraservice/device3/Camera3Stream.h +++ b/services/camera/libcameraservice/device3/Camera3Stream.h @@ -320,7 +320,7 @@ class Camera3Stream : * For bidirectional streams, this method applies to the output-side buffers */ status_t returnBuffer(const camera3_stream_buffer &buffer, - nsecs_t timestamp); + nsecs_t timestamp, bool timestampIncreasing); /** * Fill in the camera3_stream_buffer with the next valid buffer for this @@ -559,6 +559,7 @@ class Camera3Stream : android_dataspace mOriginalDataSpace; String8 mPhysicalCameraId; + nsecs_t mLastTimestamp; }; // class Camera3Stream }; // namespace camera3 diff --git a/services/camera/libcameraservice/device3/Camera3StreamInterface.h b/services/camera/libcameraservice/device3/Camera3StreamInterface.h index 9ed7184203..bddd2e7c7d 100644 --- a/services/camera/libcameraservice/device3/Camera3StreamInterface.h +++ b/services/camera/libcameraservice/device3/Camera3StreamInterface.h @@ -246,7 +246,7 @@ class Camera3StreamInterface : public virtual RefBase { * For bidirectional streams, this method applies to the output-side buffers */ virtual status_t returnBuffer(const camera3_stream_buffer &buffer, - nsecs_t timestamp) = 0; + nsecs_t timestamp, bool timestampIncreasing = true) = 0; /** * Fill in the camera3_stream_buffer with the next valid buffer for this