Merge "Camera service: Restructure buffer returns to avoid locks" into main (8be94d0f) · Commits · e / os / android_frameworks_av

camera/camera_platform.aconfig

+7 −0

Original line number	Diff line number	Diff line
		@@ -90,3 +90,10 @@ flag {
		description: "Only trigger lazy HAL instantiation when the HAL is needed for an operation."
		bug: "319735068"
		}

		flag {
		namespace: "camera_platform"
		name: "return_buffers_outside_locks"
		description: "Enable returning graphics buffers to buffer queues without holding the in-flight mutex"
		bug: "315526878"
		}

services/camera/libcameraservice/device3/Camera3OutputUtils.cpp

+102 −33

Original line number	Diff line number	Diff line
		@@ -499,7 +499,8 @@ void removeInFlightMapEntryLocked(CaptureOutputStates& states, int idx) {
		states.inflightIntf.onInflightEntryRemovedLocked(duration);
		}

		void removeInFlightRequestIfReadyLocked(CaptureOutputStates& states, int idx) {
		void removeInFlightRequestIfReadyLocked(CaptureOutputStates& states, int idx,
		std::vector<BufferToReturn> *returnableBuffers) {
		InFlightRequestMap& inflightMap = states.inflightMap;
		const InFlightRequest &request = inflightMap.valueAt(idx);
		const uint32_t frameNumber = inflightMap.keyAt(idx);
		@@ -537,12 +538,13 @@ void removeInFlightRequestIfReadyLocked(CaptureOutputStates& states, int idx) {
		assert(request.requestStatus != OK \|\|
		request.pendingOutputBuffers.size() == 0);

		returnOutputBuffers(
		collectReturnableOutputBuffers(
		states.useHalBufManager, states.halBufManagedStreamIds,
		states.listener,
		request.pendingOutputBuffers.array(),
		request.pendingOutputBuffers.size(), /timestamp/0, /readoutTimestamp/0,
		/requested/true, request.requestTimeNs, states.sessionStatsBuilder,
		/out/ returnableBuffers,
		/timestampIncreasing/true,
		request.outputSurfaces, request.resultExtras,
		request.errorBufStrategy, request.transform);
		@@ -637,6 +639,7 @@ void processCaptureResult(CaptureOutputStates& states, const camera_capture_resu
		// 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.
		std::vector<BufferToReturn> returnableBuffers{};
		nsecs_t shutterTimestamp = 0;
		{
		std::lock_guard<std::mutex> l(states.inflightLock);
		@@ -798,10 +801,11 @@ void processCaptureResult(CaptureOutputStates& states, const camera_capture_resu
		request.pendingOutputBuffers.appendArray(result->output_buffers,
		result->num_output_buffers);
		if (shutterTimestamp != 0) {
		returnAndRemovePendingOutputBuffers(
		collectAndRemovePendingOutputBuffers(
		states.useHalBufManager, states.halBufManagedStreamIds,
		states.listener,
		request, states.sessionStatsBuilder);
		request, states.sessionStatsBuilder,
		/out/ &returnableBuffers);
		}

		if (result->result != NULL && !isPartialResult) {
		@@ -826,9 +830,18 @@ void processCaptureResult(CaptureOutputStates& states, const camera_capture_resu
		request.physicalMetadatas);
		}
		}
		removeInFlightRequestIfReadyLocked(states, idx);
		removeInFlightRequestIfReadyLocked(states, idx, &returnableBuffers);
		if (!flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}
		} // scope for states.inFlightLock

		if (flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}

		if (result->input_buffer != NULL) {
		if (hasInputBufferInRequest) {
		Camera3Stream *stream =
		@@ -849,17 +862,17 @@ void processCaptureResult(CaptureOutputStates& states, const camera_capture_resu
		}
		}

		void returnOutputBuffers(
		void collectReturnableOutputBuffers(
		bool useHalBufManager,
		const std::set<int32_t> &halBufferManagedStreams,
		sp<NotificationListener> listener,
		const camera_stream_buffer_t *outputBuffers, size_t numBuffers,
		nsecs_t timestamp, nsecs_t readoutTimestamp, bool requested,
		nsecs_t requestTimeNs, SessionStatsBuilder& sessionStatsBuilder,
		/out/ std::vector<BufferToReturn> *returnableBuffers,
		bool timestampIncreasing, const SurfaceMap& outputSurfaces,
		const CaptureResultExtras &inResultExtras,
		const CaptureResultExtras &resultExtras,
		ERROR_BUF_STRATEGY errorBufStrategy, int32_t transform) {

		for (size_t i = 0; i < numBuffers; i++)
		{
		Camera3StreamInterface *stream = Camera3Stream::cast(outputBuffers[i].stream);
		@@ -869,7 +882,7 @@ void returnOutputBuffers(
		if (outputBuffers[i].status == CAMERA_BUFFER_STATUS_ERROR &&
		errorBufStrategy == ERROR_BUF_RETURN_NOTIFY) {
		if (listener != nullptr) {
		CaptureResultExtras extras = inResultExtras;
		CaptureResultExtras extras = resultExtras;
		extras.errorStreamId = streamId;
		listener->notifyError(
		hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_BUFFER,
		@@ -894,22 +907,35 @@ void returnOutputBuffers(
		}

		const auto& it = outputSurfaces.find(streamId);
		status_t res = OK;

		// Do not return the buffer if the buffer status is error, and the error
		// buffer strategy is CACHE.
		if (outputBuffers[i].status != CAMERA_BUFFER_STATUS_ERROR \|\|
		errorBufStrategy != ERROR_BUF_CACHE) {
		if (it != outputSurfaces.end()) {
		res = stream->returnBuffer(
		returnableBuffers->emplace_back(stream,
		outputBuffers[i], timestamp, readoutTimestamp, timestampIncreasing,
		it->second, inResultExtras.frameNumber, transform);
		it->second, resultExtras,
		transform, requested ? requestTimeNs : 0);
		} else {
		res = stream->returnBuffer(
		returnableBuffers->emplace_back(stream,
		outputBuffers[i], timestamp, readoutTimestamp, timestampIncreasing,
		std::vector<size_t> (), inResultExtras.frameNumber, transform);
		std::vector<size_t> (), resultExtras,
		transform, requested ? requestTimeNs : 0 );
		}
		}
		}
		}

		void finishReturningOutputBuffers(const std::vector<BufferToReturn> &returnableBuffers,
		sp<NotificationListener> listener, SessionStatsBuilder& sessionStatsBuilder) {
		for (auto& b : returnableBuffers) {
		const int streamId = b.stream->getId();

		status_t res = b.stream->returnBuffer(b.buffer, b.timestamp,
		b.readoutTimestamp, b.timestampIncreasing,
		b.surfaceIds, b.resultExtras.frameNumber, b.transform);

		// Note: stream may be deallocated at this point, if this buffer was
		// the last reference to it.
		bool dropped = false;
		@@ -920,51 +946,55 @@ void returnOutputBuffers(
		ALOGE("Can't return buffer to its stream: %s (%d)", strerror(-res), res);
		dropped = true;
		} else {
		if (outputBuffers[i].status == CAMERA_BUFFER_STATUS_ERROR \|\| timestamp == 0) {
		if (b.buffer.status == CAMERA_BUFFER_STATUS_ERROR \|\| b.timestamp == 0) {
		dropped = true;
		}
		}
		if (requested) {
		if (b.requestTimeNs > 0) {
		nsecs_t bufferTimeNs = systemTime();
		int32_t captureLatencyMs = ns2ms(bufferTimeNs - requestTimeNs);
		int32_t captureLatencyMs = ns2ms(bufferTimeNs - b.requestTimeNs);
		sessionStatsBuilder.incCounter(streamId, dropped, captureLatencyMs);
		}

		// Long processing consumers can cause returnBuffer timeout for shared stream
		// If that happens, cancel the buffer and send a buffer error to client
		if (it != outputSurfaces.end() && res == TIMED_OUT &&
		outputBuffers[i].status == CAMERA_BUFFER_STATUS_OK) {
		if (b.surfaceIds.size() > 0 && res == TIMED_OUT &&
		b.buffer.status == CAMERA_BUFFER_STATUS_OK) {
		// cancel the buffer
		camera_stream_buffer_t sb = outputBuffers[i];
		camera_stream_buffer_t sb = b.buffer;
		sb.status = CAMERA_BUFFER_STATUS_ERROR;
		stream->returnBuffer(sb, /timestamp/0, /readoutTimestamp/0,
		timestampIncreasing, std::vector<size_t> (),
		inResultExtras.frameNumber, transform);
		b.stream->returnBuffer(sb, /timestamp/0, /readoutTimestamp/0,
		b.timestampIncreasing, std::vector<size_t> (),
		b.resultExtras.frameNumber, b.transform);

		if (listener != nullptr) {
		CaptureResultExtras extras = inResultExtras;
		CaptureResultExtras extras = b.resultExtras;
		extras.errorStreamId = streamId;
		listener->notifyError(
		hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_BUFFER,
		extras);
		}
		}

		}
		}

		void returnAndRemovePendingOutputBuffers(bool useHalBufManager,
		void collectAndRemovePendingOutputBuffers(bool useHalBufManager,
		const std::set<int32_t> &halBufferManagedStreams,
		sp<NotificationListener> listener, InFlightRequest& request,
		SessionStatsBuilder& sessionStatsBuilder) {
		SessionStatsBuilder& sessionStatsBuilder,
		std::vector<BufferToReturn> *returnableBuffers) {
		bool timestampIncreasing =
		!((request.zslCapture && request.stillCapture) \|\| request.hasInputBuffer);
		nsecs_t readoutTimestamp = request.resultExtras.hasReadoutTimestamp ?
		request.resultExtras.readoutTimestamp : 0;
		returnOutputBuffers(useHalBufManager, halBufferManagedStreams, listener,
		collectReturnableOutputBuffers(useHalBufManager, halBufferManagedStreams, listener,
		request.pendingOutputBuffers.array(),
		request.pendingOutputBuffers.size(),
		request.shutterTimestamp, readoutTimestamp,
		/requested/true, request.requestTimeNs, sessionStatsBuilder, timestampIncreasing,
		/requested/true, request.requestTimeNs, sessionStatsBuilder,
		/out/ returnableBuffers,
		timestampIncreasing,
		request.outputSurfaces, request.resultExtras,
		request.errorBufStrategy, request.transform);

		@@ -984,6 +1014,8 @@ void notifyShutter(CaptureOutputStates& states, const camera_shutter_msg_t &msg)
		ATRACE_CALL();
		ssize_t idx;

		std::vector<BufferToReturn> returnableBuffers{};

		// Set timestamp for the request in the in-flight tracking
		// and get the request ID to send upstream
		{
		@@ -1061,17 +1093,30 @@ void notifyShutter(CaptureOutputStates& states, const camera_shutter_msg_t &msg)
		r.hasInputBuffer, r.zslCapture && r.stillCapture,
		r.rotateAndCropAuto, cameraIdsWithZoom, r.physicalMetadatas);
		}
		returnAndRemovePendingOutputBuffers(
		collectAndRemovePendingOutputBuffers(
		states.useHalBufManager, states.halBufManagedStreamIds,
		states.listener, r, states.sessionStatsBuilder);
		states.listener, r, states.sessionStatsBuilder, &returnableBuffers);

		if (!flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}

		removeInFlightRequestIfReadyLocked(states, idx, &returnableBuffers);

		removeInFlightRequestIfReadyLocked(states, idx);
		}
		}
		if (idx < 0) {
		SET_ERR("Shutter notification for non-existent frame number %d",
		msg.frame_number);
		}
		// With no locks held, finish returning buffers to streams, which may take a while since
		// binder calls are involved
		if (flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}

		}

		void notifyError(CaptureOutputStates& states, const camera_error_msg_t &msg) {
		@@ -1117,6 +1162,8 @@ void notifyError(CaptureOutputStates& states, const camera_error_msg_t &msg) {
		break;
		case hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_REQUEST:
		case hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_RESULT:
		{
		std::vector<BufferToReturn> returnableBuffers{};
		{
		std::lock_guard<std::mutex> l(states.inflightLock);
		ssize_t idx = states.inflightMap.indexOfKey(msg.frame_number);
		@@ -1153,7 +1200,12 @@ void notifyError(CaptureOutputStates& states, const camera_error_msg_t &msg) {

		// Check whether the buffers returned. If they returned,
		// remove inflight request.
		removeInFlightRequestIfReadyLocked(states, idx);
		removeInFlightRequestIfReadyLocked(states, idx, &returnableBuffers);
		if (!flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}

		}
		} else {
		resultExtras.frameNumber = msg.frame_number;
		@@ -1162,6 +1214,12 @@ void notifyError(CaptureOutputStates& states, const camera_error_msg_t &msg) {
		resultExtras.frameNumber);
		}
		}

		if (flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}

		resultExtras.errorStreamId = streamId;
		if (states.listener != nullptr) {
		states.listener->notifyError(errorCode, resultExtras);
		@@ -1170,6 +1228,7 @@ void notifyError(CaptureOutputStates& states, const camera_error_msg_t &msg) {
		states.cameraId.c_str(), __FUNCTION__);
		}
		break;
		}
		case hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_BUFFER:
		// Do not depend on HAL ERROR_CAMERA_BUFFER to send buffer error
		// callback to the app. Rather, use STATUS_ERROR of image buffers.
		@@ -1199,18 +1258,24 @@ void notify(CaptureOutputStates& states, const camera_notify_msg *msg) {

		void flushInflightRequests(FlushInflightReqStates& states) {
		ATRACE_CALL();
		std::vector<BufferToReturn> returnableBuffers{};
		{ // First return buffers cached in inFlightMap
		std::lock_guard<std::mutex> l(states.inflightLock);
		for (size_t idx = 0; idx < states.inflightMap.size(); idx++) {
		const InFlightRequest &request = states.inflightMap.valueAt(idx);
		returnOutputBuffers(
		collectReturnableOutputBuffers(
		states.useHalBufManager, states.halBufManagedStreamIds,
		states.listener,
		request.pendingOutputBuffers.array(),
		request.pendingOutputBuffers.size(), /timestamp/0, /readoutTimestamp/0,
		/requested/true, request.requestTimeNs, states.sessionStatsBuilder,
		/out/ &returnableBuffers,
		/timestampIncreasing/true, request.outputSurfaces, request.resultExtras,
		request.errorBufStrategy);
		if (!flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}
		ALOGW("%s: Frame %d \| Timestamp: %" PRId64 ", metadata"
		" arrived: %s, buffers left: %d.\n", __FUNCTION__,
		states.inflightMap.keyAt(idx), request.shutterTimestamp,
		@@ -1221,6 +1286,10 @@ void flushInflightRequests(FlushInflightReqStates& states) {
		states.inflightMap.clear();
		states.inflightIntf.onInflightMapFlushedLocked();
		}
		if (flags::return_buffers_outside_locks()) {
		finishReturningOutputBuffers(returnableBuffers,
		states.listener, states.sessionStatsBuilder);
		}

		// Then return all inflight buffers not returned by HAL
		std::vector<std::pair<int32_t, int32_t>> inflightKeys;

services/camera/libcameraservice/device3/Camera3OutputUtils.h

+53 −9

Original line number	Diff line number	Diff line
		@@ -44,16 +44,50 @@ namespace camera3 {
		* Helper methods shared between Camera3Device/Camera3OfflineSession for HAL callbacks
		*/

		// helper function to return the output buffers to output streams. The
		// function also optionally calls notify(ERROR_BUFFER).
		void returnOutputBuffers(
		struct BufferToReturn {
		Camera3StreamInterface *stream;
		camera_stream_buffer_t buffer;
		nsecs_t timestamp;
		nsecs_t readoutTimestamp;
		bool timestampIncreasing;
		std::vector<size_t> surfaceIds;
		const CaptureResultExtras resultExtras;
		int32_t transform;
		nsecs_t requestTimeNs;

		BufferToReturn(Camera3StreamInterface *stream,
		camera_stream_buffer_t buffer,
		nsecs_t timestamp, nsecs_t readoutTimestamp,
		bool timestampIncreasing, std::vector<size_t> surfaceIds,
		const CaptureResultExtras &resultExtras,
		int32_t transform, nsecs_t requestTimeNs):
		stream(stream),
		buffer(buffer),
		timestamp(timestamp),
		readoutTimestamp(readoutTimestamp),
		timestampIncreasing(timestampIncreasing),
		surfaceIds(surfaceIds),
		resultExtras(resultExtras),
		transform(transform),
		requestTimeNs(requestTimeNs) {}
		};

		// helper function to return the output buffers to output
		// streams. The function also optionally calls
		// notify(ERROR_BUFFER). Returns the list of buffers to hand back
		// to streams in returnableBuffers. Does not make any two-way
		// binder calls, so suitable for use when critical locks are being
		// held
		void collectReturnableOutputBuffers(
		bool useHalBufManager,
		const std::set<int32_t> &halBufferManagedStreams,
		sp<NotificationListener> listener, // Only needed when outputSurfaces is not empty
		const camera_stream_buffer_t *outputBuffers,
		size_t numBuffers, nsecs_t timestamp,
		nsecs_t readoutTimestamp, bool requested, nsecs_t requestTimeNs,
		SessionStatsBuilder& sessionStatsBuilder, bool timestampIncreasing = true,
		SessionStatsBuilder& sessionStatsBuilder,
		/out/ std::vector<BufferToReturn> *returnableBuffers,
		bool timestampIncreasing = true,
		// The following arguments are only meant for surface sharing use case
		const SurfaceMap& outputSurfaces = SurfaceMap{},
		// Used to send buffer error callback when failing to return buffer
		@@ -61,14 +95,24 @@ namespace camera3 {
		ERROR_BUF_STRATEGY errorBufStrategy = ERROR_BUF_RETURN,
		int32_t transform = -1);

		// helper function to return the output buffers to output streams, and
		// remove the returned buffers from the inflight request's pending buffers
		// vector.
		void returnAndRemovePendingOutputBuffers(
		// helper function to collect the output buffers ready to be
		// returned to output streams, and to remove these buffers from
		// the inflight request's pending buffers vector. Does not make
		// any two-way binder calls, so suitable for use when critical
		// locks are being held
		void collectAndRemovePendingOutputBuffers(
		bool useHalBufManager,
		const std::set<int32_t> &halBufferManagedStreams,
		sp<NotificationListener> listener, // Only needed when outputSurfaces is not empty
		InFlightRequest& request, SessionStatsBuilder& sessionStatsBuilder);
		InFlightRequest& request, SessionStatsBuilder& sessionStatsBuilder,
		/out/ std::vector<BufferToReturn> *returnableBuffers);

		// Actually return filled output buffers to the consumer to use, using the list
		// provided by collectReturnableOutputBuffers / collectAndRemovePendingOutputBuffers
		// Makes two-way binder calls to applications, so do not hold any critical locks when
		// calling.
		void finishReturningOutputBuffers(const std::vector<BufferToReturn> &returnableBuffers,
		sp<NotificationListener> listener, SessionStatsBuilder& sessionStatsBuilder);

		// Camera3Device/Camera3OfflineSession internal states used in notify/processCaptureResult
		// callbacks

services/camera/libcameraservice/device3/Camera3OutputUtilsTemplated.h

+6 −1

Original line number	Diff line number	Diff line
		@@ -345,10 +345,15 @@ void returnStreamBuffersT(ReturnBufferStates& states,
		continue;
		}
		streamBuffer.stream = stream->asHalStream();
		returnOutputBuffers(states.useHalBufManager, states.halBufManagedStreamIds,
		std::vector<BufferToReturn> returnableBuffers{};
		collectReturnableOutputBuffers(states.useHalBufManager, states.halBufManagedStreamIds,
		/listener/nullptr, &streamBuffer, /size/1, /timestamp/ 0,
		/readoutTimestamp/0, /requested/false, /requestTimeNs/0,
		states.sessionStatsBuilder,
		/out/&returnableBuffers);
		finishReturningOutputBuffers(returnableBuffers, /listener/ nullptr,
		states.sessionStatsBuilder);

		}
		}

services/camera/libcameraservice/device3/aidl/AidlCamera3OutputUtils.cpp

+9 −4

Original line number	Diff line number	Diff line
		@@ -319,10 +319,15 @@ void requestStreamBuffers(RequestBufferStates& states,
		sb.acquire_fence = -1;
		sb.status = CAMERA_BUFFER_STATUS_ERROR;
		}
		returnOutputBuffers(states.useHalBufManager,states.halBufManagedStreamIds, nullptr,
		streamBuffers.data(), numAllocatedBuffers, 0,
		0, false,
		0, states.sessionStatsBuilder);
		std::vector<BufferToReturn> returnableBuffers{};
		collectReturnableOutputBuffers(states.useHalBufManager, states.halBufManagedStreamIds,
		/listener/ nullptr,
		streamBuffers.data(), numAllocatedBuffers, /timestamp/ 0,
		/readoutTimestamp/ 0, /requested/ false,
		/requestTimeNs/ 0, states.sessionStatsBuilder,
		/out/ &returnableBuffers);
		finishReturningOutputBuffers(returnableBuffers, /listener/ nullptr,
		states.sessionStatsBuilder);
		for (auto buf : newBuffers) {
		states.bufferRecordsIntf.removeOneBufferCache(streamId, buf);
		}