Camera: add buffer freed notification interface
To cleanup caches of obsolete buffers.
This CL addressed the input stream bit, the output
stream hook will be a followup CL.
Also cleanup some dead API in CameraDeviceBase.h
Test: fix CTS ReprocessCaptureTest
Bug: 34461678
Change-Id: I801cd81c29becaa45630ed0a5c2dab8df1278a6a
diff --git a/services/camera/libcameraservice/common/CameraDeviceBase.h b/services/camera/libcameraservice/common/CameraDeviceBase.h
index f9b062a..4f788ae 100644
--- a/services/camera/libcameraservice/common/CameraDeviceBase.h
+++ b/services/camera/libcameraservice/common/CameraDeviceBase.h
@@ -145,12 +145,6 @@
int32_t format, /*out*/ int32_t *id) = 0;
/**
- * Create an input reprocess stream that uses buffers from an existing
- * output stream.
- */
- virtual status_t createReprocessStreamFromStream(int outputId, int *id) = 0;
-
- /**
* Get information about a given stream.
*/
virtual status_t getStreamInfo(int id,
@@ -169,12 +163,6 @@
virtual status_t deleteStream(int id) = 0;
/**
- * Delete reprocess stream. Must not be called if there are requests in
- * flight which reference that stream.
- */
- virtual status_t deleteReprocessStream(int id) = 0;
-
- /**
* Take the currently-defined set of streams and configure the HAL to use
* them. This is a long-running operation (may be several hundered ms).
*
@@ -289,21 +277,6 @@
virtual status_t triggerPrecaptureMetering(uint32_t id) = 0;
/**
- * Abstract interface for clients that want to listen to reprocess buffer
- * release events
- */
- struct BufferReleasedListener : public virtual RefBase {
- virtual void onBufferReleased(buffer_handle_t *handle) = 0;
- };
-
- /**
- * Push a buffer to be reprocessed into a reprocessing stream, and
- * provide a listener to call once the buffer is returned by the HAL
- */
- virtual status_t pushReprocessBuffer(int reprocessStreamId,
- buffer_handle_t *buffer, wp<BufferReleasedListener> listener) = 0;
-
- /**
* Flush all pending and in-flight requests. Blocks until flush is
* complete.
* Output lastFrameNumber is the last frame number of the previous streaming request.
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 1de2edc..8aa73c6 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -1399,15 +1399,6 @@
return OK;
}
-status_t Camera3Device::createReprocessStreamFromStream(int outputId, int *id) {
- ATRACE_CALL();
- (void)outputId; (void)id;
-
- CLOGE("Unimplemented");
- return INVALID_OPERATION;
-}
-
-
status_t Camera3Device::getStreamInfo(int id,
uint32_t *width, uint32_t *height,
uint32_t *format, android_dataspace *dataSpace) {
@@ -1523,14 +1514,6 @@
return res;
}
-status_t Camera3Device::deleteReprocessStream(int id) {
- ATRACE_CALL();
- (void)id;
-
- CLOGE("Unimplemented");
- return INVALID_OPERATION;
-}
-
status_t Camera3Device::configureStreams(int operatingMode) {
ATRACE_CALL();
ALOGV("%s: E", __FUNCTION__);
@@ -1856,15 +1839,6 @@
sizeof(trigger)/sizeof(trigger[0]));
}
-status_t Camera3Device::pushReprocessBuffer(int reprocessStreamId,
- buffer_handle_t *buffer, wp<BufferReleasedListener> listener) {
- ATRACE_CALL();
- (void)reprocessStreamId; (void)buffer; (void)listener;
-
- CLOGE("Unimplemented");
- return INVALID_OPERATION;
-}
-
status_t Camera3Device::flush(int64_t *frameNumber) {
ATRACE_CALL();
ALOGV("%s: Camera %s: Flushing all requests", __FUNCTION__, mId.string());
@@ -3154,7 +3128,9 @@
Stream &dst = requestedConfiguration.streams[i];
camera3_stream_t *src = config->streams[i];
- int streamId = Camera3Stream::cast(src)->getId();
+ Camera3Stream* cam3stream = Camera3Stream::cast(src);
+ cam3stream->setBufferFreedListener(this);
+ int streamId = cam3stream->getId();
StreamType streamType;
switch (src->stream_type) {
case CAMERA3_STREAM_OUTPUT:
@@ -3359,9 +3335,21 @@
wrapAsHidlRequest(requests[i], /*out*/&captureRequests[i], /*out*/&handlesCreated);
}
+ std::vector<device::V3_2::BufferCache> cachesToRemove;
+ {
+ std::lock_guard<std::mutex> lock(mBufferIdMapLock);
+ for (auto& pair : mFreedBuffers) {
+ // The stream might have been removed since onBufferFreed
+ if (mBufferIdMaps.find(pair.first) != mBufferIdMaps.end()) {
+ cachesToRemove.push_back({pair.first, pair.second});
+ }
+ }
+ mFreedBuffers.clear();
+ }
+
common::V1_0::Status status = common::V1_0::Status::INTERNAL_ERROR;
*numRequestProcessed = 0;
- mHidlSession->processCaptureRequest(captureRequests,
+ mHidlSession->processCaptureRequest(captureRequests, cachesToRemove,
[&status, &numRequestProcessed] (auto s, uint32_t n) {
status = s;
*numRequestProcessed = n;
@@ -3469,12 +3457,40 @@
auto it = bIdMap.find(buf);
if (it == bIdMap.end()) {
bIdMap[buf] = mNextBufferId++;
+ ALOGV("stream %d now have %zu buffer caches, buf %p",
+ streamId, bIdMap.size(), buf);
return std::make_pair(true, mNextBufferId - 1);
} else {
return std::make_pair(false, it->second);
}
}
+void Camera3Device::HalInterface::onBufferFreed(
+ int streamId, const native_handle_t* handle) {
+ std::lock_guard<std::mutex> lock(mBufferIdMapLock);
+ uint64_t bufferId = BUFFER_ID_NO_BUFFER;
+ auto mapIt = mBufferIdMaps.find(streamId);
+ if (mapIt == mBufferIdMaps.end()) {
+ // streamId might be from a deleted stream here
+ ALOGI("%s: stream %d has been removed",
+ __FUNCTION__, streamId);
+ return;
+ }
+ BufferIdMap& bIdMap = mapIt->second;
+ auto it = bIdMap.find(handle);
+ if (it == bIdMap.end()) {
+ ALOGW("%s: cannot find buffer %p in stream %d",
+ __FUNCTION__, handle, streamId);
+ return;
+ } else {
+ bufferId = it->second;
+ bIdMap.erase(it);
+ ALOGV("%s: stream %d now have %zu buffer caches after removing buf %p",
+ __FUNCTION__, streamId, bIdMap.size(), handle);
+ }
+ mFreedBuffers.push_back(std::make_pair(streamId, bufferId));
+}
+
/**
* RequestThread inner class methods
*/
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index d873b27..692a97d 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -125,7 +125,6 @@
status_t createInputStream(
uint32_t width, uint32_t height, int format,
int *id) override;
- status_t createReprocessStreamFromStream(int outputId, int *id) override;
status_t getStreamInfo(int id,
uint32_t *width, uint32_t *height,
@@ -133,7 +132,6 @@
status_t setStreamTransform(int id, int transform) override;
status_t deleteStream(int id) override;
- status_t deleteReprocessStream(int id) override;
status_t configureStreams(int operatingMode =
static_cast<int>(hardware::camera::device::V3_2::StreamConfigurationMode::NORMAL_MODE))
@@ -155,9 +153,6 @@
status_t triggerCancelAutofocus(uint32_t id) override;
status_t triggerPrecaptureMetering(uint32_t id) override;
- status_t pushReprocessBuffer(int reprocessStreamId,
- buffer_handle_t *buffer, wp<BufferReleasedListener> listener) override;
-
status_t flush(int64_t *lastFrameNumber = NULL) override;
status_t prepare(int streamId) override;
@@ -228,7 +223,7 @@
* Adapter for legacy HAL / HIDL HAL interface calls; calls either into legacy HALv3 or the
* HIDL HALv3 interfaces.
*/
- class HalInterface {
+ class HalInterface : public camera3::Camera3StreamBufferFreedListener {
public:
HalInterface(camera3_device_t *device);
HalInterface(sp<hardware::camera::device::V3_2::ICameraDeviceSession> &session);
@@ -326,6 +321,10 @@
// buffer_handle_t's FD won't change.
// return pair of (newlySeenBuffer?, bufferId)
std::pair<bool, uint64_t> getBufferId(const buffer_handle_t& buf, int streamId);
+
+ virtual void onBufferFreed(int streamId, const native_handle_t* handle) override;
+
+ std::vector<std::pair<int, uint64_t>> mFreedBuffers;
};
std::unique_ptr<HalInterface> mInterface;
diff --git a/services/camera/libcameraservice/device3/Camera3InputStream.cpp b/services/camera/libcameraservice/device3/Camera3InputStream.cpp
index 1469b74..4eb15ad 100644
--- a/services/camera/libcameraservice/device3/Camera3InputStream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3InputStream.cpp
@@ -263,6 +263,8 @@
mConsumer->setName(String8::format("Camera3-InputStream-%d", mId));
mProducer = producer;
+
+ mConsumer->setBufferFreedListener(this);
}
res = mConsumer->setDefaultBufferSize(camera3_stream::width,
@@ -288,6 +290,17 @@
return OK;
}
+void Camera3InputStream::onBufferFreed(const wp<GraphicBuffer>& gb) {
+ const sp<GraphicBuffer> buffer = gb.promote();
+ if (buffer != nullptr) {
+ if (mBufferFreedListener != nullptr) {
+ mBufferFreedListener->onBufferFreed(mId, buffer->handle);
+ }
+ } else {
+ ALOGE("%s: GraphicBuffer is freed before onBufferFreed callback finishes!", __FUNCTION__);
+ }
+}
+
}; // namespace camera3
}; // namespace android
diff --git a/services/camera/libcameraservice/device3/Camera3InputStream.h b/services/camera/libcameraservice/device3/Camera3InputStream.h
index 9f3de10..8f5b431 100644
--- a/services/camera/libcameraservice/device3/Camera3InputStream.h
+++ b/services/camera/libcameraservice/device3/Camera3InputStream.h
@@ -34,7 +34,8 @@
* buffers by feeding them into the HAL, as well as releasing the buffers back
* the buffers once the HAL is done with them.
*/
-class Camera3InputStream : public Camera3IOStreamBase {
+class Camera3InputStream : public Camera3IOStreamBase,
+ public BufferItemConsumer::BufferFreedListener {
public:
/**
* Set up a stream for formats that have fixed size, such as RAW and YUV.
@@ -77,6 +78,11 @@
virtual status_t getEndpointUsage(uint32_t *usage) const;
+ /**
+ * BufferItemConsumer::BufferFreedListener interface
+ */
+ virtual void onBufferFreed(const wp<GraphicBuffer>&) override;
+
}; // class Camera3InputStream
}; // namespace camera3
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.cpp b/services/camera/libcameraservice/device3/Camera3Stream.cpp
index 53a3168..2b1a899 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Stream.cpp
@@ -812,6 +812,18 @@
}
}
+void Camera3Stream::setBufferFreedListener(
+ Camera3StreamBufferFreedListener* listener) {
+ Mutex::Autolock l(mLock);
+ // Only allow set listener during stream configuration because stream is guaranteed to be IDLE
+ // at this state, so setBufferFreedListener won't collide with onBufferFreed callbacks
+ if (mState != STATE_IN_CONFIG && mState != STATE_IN_RECONFIG) {
+ ALOGE("%s: listener must be set during stream configuration!",__FUNCTION__);
+ return;
+ }
+ mBufferFreedListener = listener;
+}
+
}; // namespace camera3
}; // namespace android
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.h b/services/camera/libcameraservice/device3/Camera3Stream.h
index 56cb827..27ef86d 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.h
+++ b/services/camera/libcameraservice/device3/Camera3Stream.h
@@ -365,6 +365,11 @@
void removeBufferListener(
const sp<Camera3StreamBufferListener>& listener);
+
+ // Setting listener will remove previous listener (if exists)
+ virtual void setBufferFreedListener(
+ Camera3StreamBufferFreedListener* listener) override;
+
/**
* Return if the buffer queue of the stream is abandoned.
*/
@@ -408,6 +413,8 @@
android_dataspace dataSpace, camera3_stream_rotation_t rotation,
int setId);
+ Camera3StreamBufferFreedListener* mBufferFreedListener;
+
/**
* Interface to be implemented by derived classes
*/
diff --git a/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h b/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h
new file mode 100644
index 0000000..478a752
--- /dev/null
+++ b/services/camera/libcameraservice/device3/Camera3StreamBufferFreedListener.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_SERVERS_CAMERA3_STREAMBUFFERFREEDLISTENER_H
+#define ANDROID_SERVERS_CAMERA3_STREAMBUFFERFREEDLISTENER_H
+
+#include <gui/Surface.h>
+#include <utils/RefBase.h>
+
+namespace android {
+
+namespace camera3 {
+
+class Camera3StreamBufferFreedListener {
+public:
+ // onBufferFreed is called when a buffer is no longer being managed
+ // by this stream. This will not be called in events when all
+ // buffers are freed due to stream disconnection.
+ //
+ // The input handle may be deleted after this callback ends, so attempting
+ // to dereference handle post this callback is illegal and might lead to
+ // crash.
+ //
+ // This callback will be called while holding Camera3Stream's lock, so
+ // calling into other Camera3Stream APIs within this callback will
+ // lead to deadlock.
+ virtual void onBufferFreed(int streamId, const native_handle_t* handle) = 0;
+
+ virtual ~Camera3StreamBufferFreedListener() {}
+};
+
+}; //namespace camera3
+}; //namespace android
+
+#endif
diff --git a/services/camera/libcameraservice/device3/Camera3StreamInterface.h b/services/camera/libcameraservice/device3/Camera3StreamInterface.h
index f7b092f..37b7c36 100644
--- a/services/camera/libcameraservice/device3/Camera3StreamInterface.h
+++ b/services/camera/libcameraservice/device3/Camera3StreamInterface.h
@@ -19,6 +19,7 @@
#include <utils/RefBase.h>
#include "Camera3StreamBufferListener.h"
+#include "Camera3StreamBufferFreedListener.h"
struct camera3_stream_buffer;
@@ -287,6 +288,15 @@
wp<Camera3StreamBufferListener> listener) = 0;
virtual void removeBufferListener(
const sp<Camera3StreamBufferListener>& listener) = 0;
+
+ /**
+ * Setting listner will remove previous listener (if exists)
+ * Only allow set listener during stream configuration because stream is guaranteed to be IDLE
+ * at this state, so setBufferFreedListener won't collide with onBufferFreed callbacks.
+ * Client is responsible to keep the listener object alive throughout the lifecycle of this
+ * Camera3Stream.
+ */
+ virtual void setBufferFreedListener(Camera3StreamBufferFreedListener* listener) = 0;
};
} // namespace camera3