camera3: Remove register_stream_buffers for HAL 3.2+
Use 'setprop camera.dev.register_stream 1' to skip the fatal NULL check
- This property will be removed before shipping L
Bug: 13301331
Bug: 13435680
Change-Id: I16aacd7b22e0a10b34f6fb8501be0256170a8cd5
diff --git a/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp b/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp
index d662cc2..2257682 100644
--- a/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp
+++ b/services/camera/libcameraservice/device3/Camera3IOStreamBase.cpp
@@ -146,6 +146,13 @@
// Inform tracker about becoming busy
if (mDequeuedBufferCount == 0 && mState != STATE_IN_CONFIG &&
mState != STATE_IN_RECONFIG) {
+ /**
+ * Avoid a spurious IDLE->ACTIVE->IDLE transition when using buffers
+ * before/after register_stream_buffers during initial configuration
+ * or re-configuration.
+ *
+ * TODO: IN_CONFIG and IN_RECONFIG checks only make sense for <HAL3.2
+ */
sp<StatusTracker> statusTracker = mStatusTracker.promote();
if (statusTracker != 0) {
statusTracker->markComponentActive(mStatusId);
@@ -224,6 +231,13 @@
mDequeuedBufferCount--;
if (mDequeuedBufferCount == 0 && mState != STATE_IN_CONFIG &&
mState != STATE_IN_RECONFIG) {
+ /**
+ * Avoid a spurious IDLE->ACTIVE->IDLE transition when using buffers
+ * before/after register_stream_buffers during initial configuration
+ * or re-configuration.
+ *
+ * TODO: IN_CONFIG and IN_RECONFIG checks only make sense for <HAL3.2
+ */
ALOGV("%s: Stream %d: All buffers returned; now idle", __FUNCTION__,
mId);
sp<StatusTracker> statusTracker = mStatusTracker.promote();
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.cpp b/services/camera/libcameraservice/device3/Camera3Stream.cpp
index 70406f1..646f286 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Stream.cpp
@@ -23,6 +23,8 @@
#include "device3/Camera3Stream.h"
#include "device3/StatusTracker.h"
+#include <cutils/properties.h>
+
namespace android {
namespace camera3 {
@@ -137,6 +139,7 @@
if (mState == STATE_CONSTRUCTED) {
mState = STATE_IN_CONFIG;
} else { // mState == STATE_CONFIGURED
+ LOG_ALWAYS_FATAL_IF(mState != STATE_CONFIGURED, "Invalid state: 0x%x", mState);
mState = STATE_IN_RECONFIG;
}
@@ -223,6 +226,14 @@
ATRACE_CALL();
Mutex::Autolock l(mLock);
+ /**
+ * TODO: Check that the state is valid first.
+ *
+ * <HAL3.2 IN_CONFIG and IN_RECONFIG in addition to CONFIGURED.
+ * >= HAL3.2 CONFIGURED only
+ *
+ * Do this for getBuffer as well.
+ */
status_t res = returnBufferLocked(buffer, timestamp);
if (res == OK) {
fireBufferListenersLocked(buffer, /*acquired*/false, /*output*/true);
@@ -314,12 +325,46 @@
status_t Camera3Stream::registerBuffersLocked(camera3_device *hal3Device) {
ATRACE_CALL();
+
+ /**
+ * >= CAMERA_DEVICE_API_VERSION_3_2:
+ *
+ * camera3_device_t->ops->register_stream_buffers() is not called and must
+ * be NULL.
+ */
+ if (hal3Device->common.version >= CAMERA_DEVICE_API_VERSION_3_2) {
+ ALOGV("%s: register_stream_buffers unused as of HAL3.2", __FUNCTION__);
+
+ /**
+ * Skip the NULL check if camera.dev.register_stream is 1.
+ *
+ * For development-validation purposes only.
+ *
+ * TODO: Remove the property check before shipping L (b/13914251).
+ */
+ char value[PROPERTY_VALUE_MAX] = { '\0', };
+ property_get("camera.dev.register_stream", value, "0");
+ int propInt = atoi(value);
+
+ if (propInt == 0 && hal3Device->ops->register_stream_buffers != NULL) {
+ ALOGE("%s: register_stream_buffers is deprecated in HAL3.2; "
+ "must be set to NULL in camera3_device::ops", __FUNCTION__);
+ return INVALID_OPERATION;
+ } else {
+ ALOGD("%s: Skipping NULL check for deprecated register_stream_buffers");
+ }
+
+ return OK;
+ } else {
+ ALOGV("%s: register_stream_buffers using deprecated code path", __FUNCTION__);
+ }
+
status_t res;
size_t bufferCount = getBufferCountLocked();
Vector<buffer_handle_t*> buffers;
- buffers.insertAt(NULL, 0, bufferCount);
+ buffers.insertAt(/*prototype_item*/NULL, /*index*/0, bufferCount);
camera3_stream_buffer_set bufferSet = camera3_stream_buffer_set();
bufferSet.stream = this;
@@ -327,7 +372,7 @@
bufferSet.buffers = buffers.editArray();
Vector<camera3_stream_buffer_t> streamBuffers;
- streamBuffers.insertAt(camera3_stream_buffer_t(), 0, bufferCount);
+ streamBuffers.insertAt(camera3_stream_buffer_t(), /*index*/0, bufferCount);
// Register all buffers with the HAL. This means getting all the buffers
// from the stream, providing them to the HAL with the
diff --git a/services/camera/libcameraservice/device3/Camera3Stream.h b/services/camera/libcameraservice/device3/Camera3Stream.h
index 6eeb721..766b772 100644
--- a/services/camera/libcameraservice/device3/Camera3Stream.h
+++ b/services/camera/libcameraservice/device3/Camera3Stream.h
@@ -82,6 +82,23 @@
* STATE_CONFIGURED => STATE_CONSTRUCTED:
* When disconnect() is called after making sure stream is idle with
* waitUntilIdle().
+ *
+ * Status Tracking:
+ * Each stream is tracked by StatusTracker as a separate component,
+ * depending on the handed out buffer count. The state must be STATE_CONFIGURED
+ * in order for the component to be marked.
+ *
+ * It's marked in one of two ways:
+ *
+ * - ACTIVE: One or more buffers have been handed out (with #getBuffer).
+ * - IDLE: All buffers have been returned (with #returnBuffer), and their
+ * respective release_fence(s) have been signaled.
+ *
+ * A typical use case is output streams. When the HAL has any buffers
+ * dequeued, the stream is marked ACTIVE. When the HAL returns all buffers
+ * (e.g. if no capture requests are active), the stream is marked IDLE.
+ * In this use case, the app consumer does not affect the component status.
+ *
*/
class Camera3Stream :
protected camera3_stream,