Merge "Camera: fix release fence FD leaks" into oc-dev (6e7ca4e1) · Commits · e / os / android_hardware_interfaces

camera/device/3.2/default/CameraDeviceSession.cpp

+59 −11

Original line number	Diff line number	Diff line
		@@ -341,7 +341,7 @@ void CameraDeviceSession::ResultBatcher::registerBatch(
		batch->mLastFrame = batch->mFirstFrame + batch->mBatchSize - 1;
		batch->mNumPartialResults = mNumPartialResults;
		for (int id : mStreamsToBatch) {
		batch->mBatchBufs[id] = InflightBatch::BufferBatch();
		batch->mBatchBufs.emplace(id, batch->mBatchSize);
		}
		Mutex::Autolock _l(mLock);
		mInflightBatches.push_back(batch);
		@@ -418,6 +418,29 @@ void CameraDeviceSession::ResultBatcher::freeReleaseFences(hidl_vec<CaptureResul
		return;
		}

		void CameraDeviceSession::ResultBatcher::moveStreamBuffer(StreamBuffer&& src, StreamBuffer& dst) {
		// Only dealing with releaseFence here. Assume buffer/acquireFence are null
		const native_handle_t* handle = src.releaseFence.getNativeHandle();
		src.releaseFence = nullptr;
		dst = src;
		dst.releaseFence = handle;
		if (handle != dst.releaseFence.getNativeHandle()) {
		ALOGE("%s: native handle cloned!", __FUNCTION__);
		}
		}

		void CameraDeviceSession::ResultBatcher::pushStreamBuffer(
		StreamBuffer&& src, std::vector<StreamBuffer>& dst) {
		// Only dealing with releaseFence here. Assume buffer/acquireFence are null
		const native_handle_t* handle = src.releaseFence.getNativeHandle();
		src.releaseFence = nullptr;
		dst.push_back(src);
		dst.back().releaseFence = handle;
		if (handle != dst.back().releaseFence.getNativeHandle()) {
		ALOGE("%s: native handle cloned!", __FUNCTION__);
		}
		}

		void CameraDeviceSession::ResultBatcher::sendBatchBuffersLocked(std::shared_ptr<InflightBatch> batch) {
		sendBatchBuffersLocked(batch, mStreamsToBatch);
		}
		@@ -444,7 +467,12 @@ void CameraDeviceSession::ResultBatcher::sendBatchBuffersLocked(
		if (batchSize == 0) {
		ALOGW("%s: there is no buffer to be delivered for this batch.", __FUNCTION__);
		for (int streamId : streams) {
		InflightBatch::BufferBatch& bb = batch->mBatchBufs[streamId];
		auto it = batch->mBatchBufs.find(streamId);
		if (it == batch->mBatchBufs.end()) {
		ALOGE("%s: cannot find stream %d in batched buffers!", __FUNCTION__, streamId);
		return;
		}
		InflightBatch::BufferBatch& bb = it->second;
		bb.mDelivered = true;
		}
		return;
		@@ -460,21 +488,35 @@ void CameraDeviceSession::ResultBatcher::sendBatchBuffersLocked(
		results[i].inputBuffer.bufferId = 0;
		results[i].inputBuffer.buffer = nullptr;
		std::vector<StreamBuffer> outBufs;
		outBufs.reserve(streams.size());
		for (int streamId : streams) {
		InflightBatch::BufferBatch& bb = batch->mBatchBufs[streamId];
		auto it = batch->mBatchBufs.find(streamId);
		if (it == batch->mBatchBufs.end()) {
		ALOGE("%s: cannot find stream %d in batched buffers!", __FUNCTION__, streamId);
		return;
		}
		InflightBatch::BufferBatch& bb = it->second;
		if (bb.mDelivered) {
		continue;
		}
		if (i < bb.mBuffers.size()) {
		outBufs.push_back(bb.mBuffers[i]);
		pushStreamBuffer(std::move(bb.mBuffers[i]), outBufs);
		}
		}
		results[i].outputBuffers = outBufs;
		results[i].outputBuffers.resize(outBufs.size());
		for (size_t j = 0; j < outBufs.size(); j++) {
		moveStreamBuffer(std::move(outBufs[j]), results[i].outputBuffers[j]);
		}
		}
		invokeProcessCaptureResultCallback(results, /* tryWriteFmq */false);
		freeReleaseFences(results);
		for (int streamId : streams) {
		InflightBatch::BufferBatch& bb = batch->mBatchBufs[streamId];
		auto it = batch->mBatchBufs.find(streamId);
		if (it == batch->mBatchBufs.end()) {
		ALOGE("%s: cannot find stream %d in batched buffers!", __FUNCTION__, streamId);
		return;
		}
		InflightBatch::BufferBatch& bb = it->second;
		bb.mDelivered = true;
		bb.mBuffers.clear();
		}
		@@ -613,7 +655,9 @@ void CameraDeviceSession::ResultBatcher::invokeProcessCaptureResultCallback(
		}

		void CameraDeviceSession::ResultBatcher::processOneCaptureResult(CaptureResult& result) {
		hidl_vec<CaptureResult> results = {result};
		hidl_vec<CaptureResult> results;
		results.resize(1);
		results[0] = std::move(result);
		invokeProcessCaptureResultCallback(results, /* tryWriteFmq */true);
		freeReleaseFences(results);
		return;
		@@ -650,10 +694,10 @@ void CameraDeviceSession::ResultBatcher::processCaptureResult(CaptureResult& res
		auto it = batch->mBatchBufs.find(buffer.streamId);
		if (it != batch->mBatchBufs.end()) {
		InflightBatch::BufferBatch& bb = it->second;
		bb.mBuffers.push_back(buffer);
		pushStreamBuffer(std::move(buffer), bb.mBuffers);
		filledStreams.push_back(buffer.streamId);
		} else {
		nonBatchedBuffers.push_back(buffer);
		pushStreamBuffer(std::move(buffer), nonBatchedBuffers);
		}
		}

		@@ -662,8 +706,12 @@ void CameraDeviceSession::ResultBatcher::processCaptureResult(CaptureResult& res
		CaptureResult nonBatchedResult;
		nonBatchedResult.frameNumber = result.frameNumber;
		nonBatchedResult.fmqResultSize = 0;
		nonBatchedResult.outputBuffers = nonBatchedBuffers;
		nonBatchedResult.inputBuffer = result.inputBuffer;
		nonBatchedResult.outputBuffers.resize(nonBatchedBuffers.size());
		for (size_t i = 0; i < nonBatchedBuffers.size(); i++) {
		moveStreamBuffer(
		std::move(nonBatchedBuffers[i]), nonBatchedResult.outputBuffers[i]);
		}
		moveStreamBuffer(std::move(result.inputBuffer), nonBatchedResult.inputBuffer);
		nonBatchedResult.partialResult = 0; // 0 for buffer only results
		processOneCaptureResult(nonBatchedResult);
		}

camera/device/3.2/default/CameraDeviceSession.h

+8 −0

Original line number	Diff line number	Diff line
		@@ -184,6 +184,9 @@ private:
		std::vector<NotifyMsg> mShutterMsgs;

		struct BufferBatch {
		BufferBatch(uint32_t batchSize) {
		mBuffers.reserve(batchSize);
		}
		bool mDelivered = false;
		// This currently assumes every batched request will output to the batched stream
		// and since HAL must always send buffers in order, no frameNumber tracking is
		@@ -241,6 +244,11 @@ private:
		void processOneCaptureResult(CaptureResult& result);
		void invokeProcessCaptureResultCallback(hidl_vec<CaptureResult> &results, bool tryWriteFmq);

		// move/push function avoids "hidl_handle& operator=(hidl_handle&)", which clones native
		// handle
		void moveStreamBuffer(StreamBuffer&& src, StreamBuffer& dst);
		void pushStreamBuffer(StreamBuffer&& src, std::vector<StreamBuffer>& dst);

		// Protect access to mInflightBatches, mNumPartialResults and mStreamsToBatch
		// processCaptureRequest, processCaptureResult, notify will compete for this lock
		// Do NOT issue HIDL IPCs while holding this lock (except when HAL reports error)