Camera: pass StreamSurfaceId instead of Surface
Also fix buffer error callback on shared surfaces.
Test: CTS
Bug: 68020997
Change-Id: I71d6a1373ff09dcb21d39f78dd56727cbde9a3ad
diff --git a/camera/camera2/CaptureRequest.cpp b/camera/camera2/CaptureRequest.cpp
index 0597950..983d29b 100644
--- a/camera/camera2/CaptureRequest.cpp
+++ b/camera/camera2/CaptureRequest.cpp
@@ -44,6 +44,8 @@
mMetadata.clear();
mSurfaceList.clear();
+ mStreamIdxList.clear();
+ mSurfaceIdxList.clear();
status_t err = OK;
@@ -53,6 +55,13 @@
}
ALOGV("%s: Read metadata from parcel", __FUNCTION__);
+ int isReprocess = 0;
+ if ((err = parcel->readInt32(&isReprocess)) != OK) {
+ ALOGE("%s: Failed to read reprocessing from parcel", __FUNCTION__);
+ return err;
+ }
+ mIsReprocess = (isReprocess != 0);
+
int32_t size;
if ((err = parcel->readInt32(&size)) != OK) {
ALOGE("%s: Failed to read surface list size from parcel", __FUNCTION__);
@@ -61,7 +70,7 @@
ALOGV("%s: Read surface list size = %d", __FUNCTION__, size);
// Do not distinguish null arrays from 0-sized arrays.
- for (int i = 0; i < size; ++i) {
+ for (int32_t i = 0; i < size; ++i) {
// Parcel.writeParcelableArray
size_t len;
const char16_t* className = parcel->readString16Inplace(&len);
@@ -88,12 +97,32 @@
mSurfaceList.push_back(surface);
}
- int isReprocess = 0;
- if ((err = parcel->readInt32(&isReprocess)) != OK) {
- ALOGE("%s: Failed to read reprocessing from parcel", __FUNCTION__);
+ int32_t streamSurfaceSize;
+ if ((err = parcel->readInt32(&streamSurfaceSize)) != OK) {
+ ALOGE("%s: Failed to read streamSurfaceSize from parcel", __FUNCTION__);
return err;
}
- mIsReprocess = (isReprocess != 0);
+
+ if (streamSurfaceSize < 0) {
+ ALOGE("%s: Bad streamSurfaceSize %d from parcel", __FUNCTION__, streamSurfaceSize);
+ return BAD_VALUE;
+ }
+
+ for (int32_t i = 0; i < streamSurfaceSize; ++i) {
+ int streamIdx;
+ if ((err = parcel->readInt32(&streamIdx)) != OK) {
+ ALOGE("%s: Failed to read stream index from parcel", __FUNCTION__);
+ return err;
+ }
+ mStreamIdxList.push_back(streamIdx);
+
+ int surfaceIdx;
+ if ((err = parcel->readInt32(&surfaceIdx)) != OK) {
+ ALOGE("%s: Failed to read surface index from parcel", __FUNCTION__);
+ return err;
+ }
+ mSurfaceIdxList.push_back(surfaceIdx);
+ }
return OK;
}
@@ -110,28 +139,43 @@
return err;
}
- int32_t size = static_cast<int32_t>(mSurfaceList.size());
+ parcel->writeInt32(mIsReprocess ? 1 : 0);
- // Send 0-sized arrays when it's empty. Do not send null arrays.
- parcel->writeInt32(size);
+ if (mSurfaceConverted) {
+ parcel->writeInt32(0); // 0-sized array
+ } else {
+ int32_t size = static_cast<int32_t>(mSurfaceList.size());
- for (int32_t i = 0; i < size; ++i) {
- // not sure if readParcelableArray does this, hard to tell from source
- parcel->writeString16(String16("android.view.Surface"));
+ // Send 0-sized arrays when it's empty. Do not send null arrays.
+ parcel->writeInt32(size);
- // Surface.writeToParcel
- view::Surface surfaceShim;
- surfaceShim.name = String16("unknown_name");
- surfaceShim.graphicBufferProducer = mSurfaceList[i]->getIGraphicBufferProducer();
- if ((err = surfaceShim.writeToParcel(parcel)) != OK) {
- ALOGE("%s: Failed to write output target Surface %d to parcel: %s (%d)",
- __FUNCTION__, i, strerror(-err), err);
- return err;
+ for (int32_t i = 0; i < size; ++i) {
+ // not sure if readParcelableArray does this, hard to tell from source
+ parcel->writeString16(String16("android.view.Surface"));
+
+ // Surface.writeToParcel
+ view::Surface surfaceShim;
+ surfaceShim.name = String16("unknown_name");
+ surfaceShim.graphicBufferProducer = mSurfaceList[i]->getIGraphicBufferProducer();
+ if ((err = surfaceShim.writeToParcel(parcel)) != OK) {
+ ALOGE("%s: Failed to write output target Surface %d to parcel: %s (%d)",
+ __FUNCTION__, i, strerror(-err), err);
+ return err;
+ }
}
}
- parcel->writeInt32(mIsReprocess ? 1 : 0);
-
+ parcel->writeInt32(mStreamIdxList.size());
+ for (size_t i = 0; i < mStreamIdxList.size(); ++i) {
+ if ((err = parcel->writeInt32(mStreamIdxList[i])) != OK) {
+ ALOGE("%s: Failed to write stream index to parcel", __FUNCTION__);
+ return err;
+ }
+ if ((err = parcel->writeInt32(mSurfaceIdxList[i])) != OK) {
+ ALOGE("%s: Failed to write surface index to parcel", __FUNCTION__);
+ return err;
+ }
+ }
return OK;
}
diff --git a/camera/include/camera/camera2/CaptureRequest.h b/camera/include/camera/camera2/CaptureRequest.h
index e39dfcf..c53799f 100644
--- a/camera/include/camera/camera2/CaptureRequest.h
+++ b/camera/include/camera/camera2/CaptureRequest.h
@@ -41,14 +41,30 @@
virtual ~CaptureRequest();
CameraMetadata mMetadata;
+
+ // Used by NDK client to pass surfaces by stream/surface index.
+ bool mSurfaceConverted = false;
+
+ // Starting in Android O, create a Surface from Parcel will take one extra
+ // IPC call.
Vector<sp<Surface> > mSurfaceList;
+ // Optional way of passing surface list since passing Surface over binder
+ // is expensive. Use the stream/surface index from current output configuration
+ // to represent an configured output Surface. When stream/surface index is used,
+ // set mSurfaceList to zero length to save unparcel time.
+ Vector<int> mStreamIdxList;
+ Vector<int> mSurfaceIdxList; // per stream surface list index
+
bool mIsReprocess;
+
void* mContext; // arbitrary user context from NDK apps, null for java apps
/**
* Keep impl up-to-date with CaptureRequest.java in frameworks/base
*/
+ // used by cameraserver to receive CaptureRequest from java/NDK client
status_t readFromParcel(const android::Parcel* parcel) override;
+ // used by NDK client to send CaptureRequest to cameraserver
status_t writeToParcel(android::Parcel* parcel) const override;
};
diff --git a/camera/ndk/impl/ACameraCaptureSession.cpp b/camera/ndk/impl/ACameraCaptureSession.cpp
index 6d1d5ce..f60e5fd 100644
--- a/camera/ndk/impl/ACameraCaptureSession.cpp
+++ b/camera/ndk/impl/ACameraCaptureSession.cpp
@@ -159,7 +159,7 @@
dev->lockDeviceForSessionOps();
{
Mutex::Autolock _l(mSessionLock);
- ret = dev->updateOutputConfiguration(output);
+ ret = dev->updateOutputConfigurationLocked(output);
}
dev->unlockDevice();
return ret;
diff --git a/camera/ndk/impl/ACameraDevice.cpp b/camera/ndk/impl/ACameraDevice.cpp
index 45fa28e..4abd267 100644
--- a/camera/ndk/impl/ACameraDevice.cpp
+++ b/camera/ndk/impl/ACameraDevice.cpp
@@ -289,7 +289,7 @@
return ACAMERA_OK;
}
-camera_status_t CameraDevice::updateOutputConfiguration(ACaptureSessionOutput *output) {
+camera_status_t CameraDevice::updateOutputConfigurationLocked(ACaptureSessionOutput *output) {
camera_status_t ret = checkCameraClosedOrErrorLocked();
if (ret != ACAMERA_OK) {
return ret;
@@ -361,6 +361,7 @@
return ACAMERA_ERROR_UNKNOWN;
}
}
+ mConfiguredOutputs[streamId] = std::make_pair(output->mWindow, outConfig);
return ACAMERA_OK;
}
@@ -373,6 +374,7 @@
req->mMetadata = request->settings->getInternalData();
req->mIsReprocess = false; // NDK does not support reprocessing yet
req->mContext = request->context;
+ req->mSurfaceConverted = true; // set to true, and fill in stream/surface idx to speed up IPC
for (auto outputTarget : request->targets->mOutputs) {
ANativeWindow* anw = outputTarget.mWindow;
@@ -383,7 +385,31 @@
return ret;
}
req->mSurfaceList.push_back(surface);
+
+ bool found = false;
+ // lookup stream/surface ID
+ for (const auto& kvPair : mConfiguredOutputs) {
+ int streamId = kvPair.first;
+ const OutputConfiguration& outConfig = kvPair.second.second;
+ const auto& gbps = outConfig.getGraphicBufferProducers();
+ for (int surfaceId = 0; surfaceId < (int) gbps.size(); surfaceId++) {
+ if (gbps[surfaceId] == surface->getIGraphicBufferProducer()) {
+ found = true;
+ req->mStreamIdxList.push_back(streamId);
+ req->mSurfaceIdxList.push_back(surfaceId);
+ break;
+ }
+ }
+ if (found) {
+ break;
+ }
+ }
+ if (!found) {
+ ALOGE("Unconfigured output target %p in capture request!", anw);
+ return ret;
+ }
}
+
outReq = req;
return ACAMERA_OK;
}
@@ -564,17 +590,11 @@
CameraDevice::getIGBPfromAnw(
ANativeWindow* anw,
sp<IGraphicBufferProducer>& out) {
- if (anw == nullptr) {
- ALOGE("Error: output ANativeWindow is null");
- return ACAMERA_ERROR_INVALID_PARAMETER;
+ sp<Surface> surface;
+ camera_status_t ret = getSurfaceFromANativeWindow(anw, surface);
+ if (ret != ACAMERA_OK) {
+ return ret;
}
- int value;
- int err = (*anw->query)(anw, NATIVE_WINDOW_CONCRETE_TYPE, &value);
- if (err != OK || value != NATIVE_WINDOW_SURFACE) {
- ALOGE("Error: ANativeWindow is not backed by Surface!");
- return ACAMERA_ERROR_INVALID_PARAMETER;
- }
- const sp<Surface> surface(static_cast<Surface*>(anw));
out = surface->getIGraphicBufferProducer();
return ACAMERA_OK;
}
@@ -809,19 +829,26 @@
setCameraDeviceErrorLocked(ACAMERA_ERROR_CAMERA_SERVICE);
return;
}
- ANativeWindow* anw = outputPairIt->second.first;
- ALOGV("Camera %s Lost output buffer for ANW %p frame %" PRId64,
- getId(), anw, frameNumber);
+ const auto& gbps = outputPairIt->second.second.getGraphicBufferProducers();
+ for (const auto& outGbp : gbps) {
+ for (auto surface : request->mSurfaceList) {
+ if (surface->getIGraphicBufferProducer() == outGbp) {
+ ANativeWindow* anw = static_cast<ANativeWindow*>(surface.get());
+ ALOGV("Camera %s Lost output buffer for ANW %p frame %" PRId64,
+ getId(), anw, frameNumber);
- sp<AMessage> msg = new AMessage(kWhatCaptureBufferLost, mHandler);
- msg->setPointer(kContextKey, cbh.mCallbacks.context);
- msg->setObject(kSessionSpKey, session);
- msg->setPointer(kCallbackFpKey, (void*) onBufferLost);
- msg->setObject(kCaptureRequestKey, request);
- msg->setPointer(kAnwKey, (void*) anw);
- msg->setInt64(kFrameNumberKey, frameNumber);
- postSessionMsgAndCleanup(msg);
+ sp<AMessage> msg = new AMessage(kWhatCaptureBufferLost, mHandler);
+ msg->setPointer(kContextKey, cbh.mCallbacks.context);
+ msg->setObject(kSessionSpKey, session);
+ msg->setPointer(kCallbackFpKey, (void*) onBufferLost);
+ msg->setObject(kCaptureRequestKey, request);
+ msg->setPointer(kAnwKey, (void*) anw);
+ msg->setInt64(kFrameNumberKey, frameNumber);
+ postSessionMsgAndCleanup(msg);
+ }
+ }
+ }
} else { // Handle other capture failures
// Fire capture failure callback if there is one registered
ACameraCaptureSession_captureCallback_failed onError = cbh.mCallbacks.onCaptureFailed;
diff --git a/camera/ndk/impl/ACameraDevice.h b/camera/ndk/impl/ACameraDevice.h
index 23cc1a1..7d45e80 100644
--- a/camera/ndk/impl/ACameraDevice.h
+++ b/camera/ndk/impl/ACameraDevice.h
@@ -123,9 +123,9 @@
/*out*/int* captureSequenceId,
bool isRepeating);
- camera_status_t updateOutputConfiguration(ACaptureSessionOutput *output);
+ camera_status_t updateOutputConfigurationLocked(ACaptureSessionOutput *output);
- static camera_status_t allocateCaptureRequest(
+ camera_status_t allocateCaptureRequest(
const ACaptureRequest* request, sp<CaptureRequest>& outReq);
static ACaptureRequest* allocateACaptureRequest(sp<CaptureRequest>& req);
diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
index a376ab4..973df19 100644
--- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
+++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp
@@ -121,6 +121,34 @@
return submitRequestList(requestList, streaming, submitInfo);
}
+binder::Status CameraDeviceClient::insertGbpLocked(const sp<IGraphicBufferProducer>& gbp,
+ SurfaceMap* outSurfaceMap,
+ Vector<int32_t>* outputStreamIds) {
+ int idx = mStreamMap.indexOfKey(IInterface::asBinder(gbp));
+
+ // Trying to submit request with surface that wasn't created
+ if (idx == NAME_NOT_FOUND) {
+ ALOGE("%s: Camera %s: Tried to submit a request with a surface that"
+ " we have not called createStream on",
+ __FUNCTION__, mCameraIdStr.string());
+ return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT,
+ "Request targets Surface that is not part of current capture session");
+ }
+
+ const StreamSurfaceId& streamSurfaceId = mStreamMap.valueAt(idx);
+ if (outSurfaceMap->find(streamSurfaceId.streamId()) == outSurfaceMap->end()) {
+ (*outSurfaceMap)[streamSurfaceId.streamId()] = std::vector<size_t>();
+ outputStreamIds->push_back(streamSurfaceId.streamId());
+ }
+ (*outSurfaceMap)[streamSurfaceId.streamId()].push_back(streamSurfaceId.surfaceId());
+
+ ALOGV("%s: Camera %s: Appending output stream %d surface %d to request",
+ __FUNCTION__, mCameraIdStr.string(), streamSurfaceId.streamId(),
+ streamSurfaceId.surfaceId());
+
+ return binder::Status::ok();
+}
+
binder::Status CameraDeviceClient::submitRequestList(
const std::vector<hardware::camera2::CaptureRequest>& requests,
bool streaming,
@@ -174,7 +202,7 @@
__FUNCTION__, mCameraIdStr.string());
return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT,
"Request settings are empty");
- } else if (request.mSurfaceList.isEmpty()) {
+ } else if (request.mSurfaceList.isEmpty() && request.mStreamIdxList.size() == 0) {
ALOGE("%s: Camera %s: Requests must have at least one surface target. "
"Rejecting request.", __FUNCTION__, mCameraIdStr.string());
return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT,
@@ -193,31 +221,44 @@
*/
SurfaceMap surfaceMap;
Vector<int32_t> outputStreamIds;
- for (const sp<Surface>& surface : request.mSurfaceList) {
- if (surface == 0) continue;
+ if (request.mSurfaceList.size() > 0) {
+ for (sp<Surface> surface : request.mSurfaceList) {
+ if (surface == 0) continue;
- sp<IGraphicBufferProducer> gbp = surface->getIGraphicBufferProducer();
- int idx = mStreamMap.indexOfKey(IInterface::asBinder(gbp));
-
- // Trying to submit request with surface that wasn't created
- if (idx == NAME_NOT_FOUND) {
- ALOGE("%s: Camera %s: Tried to submit a request with a surface that"
- " we have not called createStream on",
- __FUNCTION__, mCameraIdStr.string());
- return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT,
- "Request targets Surface that is not part of current capture session");
+ sp<IGraphicBufferProducer> gbp = surface->getIGraphicBufferProducer();
+ res = insertGbpLocked(gbp, &surfaceMap, &outputStreamIds);
+ if (!res.isOk()) {
+ return res;
+ }
}
+ } else {
+ for (size_t i = 0; i < request.mStreamIdxList.size(); i++) {
+ int streamId = request.mStreamIdxList.itemAt(i);
+ int surfaceIdx = request.mSurfaceIdxList.itemAt(i);
- const StreamSurfaceId& streamSurfaceId = mStreamMap.valueAt(idx);
- if (surfaceMap.find(streamSurfaceId.streamId()) == surfaceMap.end()) {
- surfaceMap[streamSurfaceId.streamId()] = std::vector<size_t>();
- outputStreamIds.push_back(streamSurfaceId.streamId());
+ ssize_t index = mConfiguredOutputs.indexOfKey(streamId);
+ if (index < 0) {
+ ALOGE("%s: Camera %s: Tried to submit a request with a surface that"
+ " we have not called createStream on: stream %d",
+ __FUNCTION__, mCameraIdStr.string(), streamId);
+ return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT,
+ "Request targets Surface that is not part of current capture session");
+ }
+
+ const auto& gbps = mConfiguredOutputs.valueAt(index).getGraphicBufferProducers();
+ if ((size_t)surfaceIdx >= gbps.size()) {
+ ALOGE("%s: Camera %s: Tried to submit a request with a surface that"
+ " we have not called createStream on: stream %d, surfaceIdx %d",
+ __FUNCTION__, mCameraIdStr.string(), streamId, surfaceIdx);
+ return STATUS_ERROR(CameraService::ERROR_ILLEGAL_ARGUMENT,
+ "Request targets Surface has invalid surface index");
+ }
+
+ res = insertGbpLocked(gbps[surfaceIdx], &surfaceMap, &outputStreamIds);
+ if (!res.isOk()) {
+ return res;
+ }
}
- surfaceMap[streamSurfaceId.streamId()].push_back(streamSurfaceId.surfaceId());
-
- ALOGV("%s: Camera %s: Appending output stream %d surface %d to request",
- __FUNCTION__, mCameraIdStr.string(), streamSurfaceId.streamId(),
- streamSurfaceId.surfaceId());
}
metadata.update(ANDROID_REQUEST_OUTPUT_STREAMS, &outputStreamIds[0],
@@ -439,6 +480,8 @@
mStreamMap.removeItem(surface);
}
+ mConfiguredOutputs.removeItem(streamId);
+
if (dIndex != NAME_NOT_FOUND) {
mDeferredStreams.removeItemsAt(dIndex);
}
@@ -550,6 +593,7 @@
i++;
}
+ mConfiguredOutputs.add(streamId, outputConfiguration);
mStreamInfoMap[streamId] = streamInfo;
ALOGV("%s: Camera %s: Successfully created a new stream ID %d for output surface"
@@ -842,6 +886,8 @@
StreamSurfaceId(streamId, outputMap.valueAt(i)));
}
+ mConfiguredOutputs.replaceValueFor(streamId, outputConfiguration);
+
ALOGV("%s: Camera %s: Successful stream ID %d update",
__FUNCTION__, mCameraIdStr.string(), streamId);
}
@@ -1412,6 +1458,7 @@
mDeferredStreams.removeItemsAt(deferredStreamIndex);
}
mStreamInfoMap[streamId].finalized = true;
+ mConfiguredOutputs.replaceValueFor(streamId, outputConfiguration);
} else if (err == NO_INIT) {
res = STATUS_ERROR_FMT(CameraService::ERROR_ILLEGAL_ARGUMENT,
"Camera %s: Deferred surface is invalid: %s (%d)",
diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.h b/services/camera/libcameraservice/api2/CameraDeviceClient.h
index e1a11db..18f5c87 100644
--- a/services/camera/libcameraservice/api2/CameraDeviceClient.h
+++ b/services/camera/libcameraservice/api2/CameraDeviceClient.h
@@ -255,9 +255,18 @@
binder::Status createSurfaceFromGbp(OutputStreamInfo& streamInfo, bool isStreamInfoValid,
sp<Surface>& surface, const sp<IGraphicBufferProducer>& gbp);
+
+ // Utility method to insert the surface into SurfaceMap
+ binder::Status insertGbpLocked(const sp<IGraphicBufferProducer>& gbp,
+ /*out*/SurfaceMap* surfaceMap,
+ /*out*/Vector<int32_t>* streamIds);
+
// IGraphicsBufferProducer binder -> Stream ID + Surface ID for output streams
KeyedVector<sp<IBinder>, StreamSurfaceId> mStreamMap;
+ // Stream ID -> OutputConfiguration. Used for looking up Surface by stream/surface index
+ KeyedVector<int32_t, hardware::camera2::params::OutputConfiguration> mConfiguredOutputs;
+
struct InputStreamConfiguration {
bool configured;
int32_t width;