Camera: Timing requirement between notify and processCaptureResult
For physical sub-camera result error, require the notify() call is made
before the final processCaptureResult.
Test: Updated VTS test passes
Bug: 138727686
Change-Id: Ifba247bc1aa35ecdba651ea888dd26902a31647e
diff --git a/camera/device/3.5/types.hal b/camera/device/3.5/types.hal
index 6d861e2..38493b4 100644
--- a/camera/device/3.5/types.hal
+++ b/camera/device/3.5/types.hal
@@ -23,7 +23,8 @@
/**
* If the result metadata cannot be produced for a physical camera device part of a logical
* multi-camera, then HAL must invoke the notification callback and pass a message with ERROR_RESULT
- * code and errorStreamId that contains the stream id associated with that physical device.
+ * code and errorStreamId that contains the stream id associated with that physical device. Such
+ * callback must be made before the final processCaptureResult() call for the corresponding request.
* The behavior during absent result metadata remains unchanged for a logical or a non-logical
* camera device and the errorStreamId must be set to -1.
*/
diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp
index a5369e7..e0d41ba 100644
--- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp
+++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp
@@ -622,7 +622,7 @@
Return<void> returnStreamBuffers(const hidl_vec<StreamBuffer>& buffers) override;
- void setCurrentStreamConfig(const hidl_vec<V3_2::Stream>& streams,
+ void setCurrentStreamConfig(const hidl_vec<V3_4::Stream>& streams,
const hidl_vec<V3_2::HalStream>& halStreams);
void waitForBuffersReturned();
@@ -639,7 +639,7 @@
/* members for requestStreamBuffers() and returnStreamBuffers()*/
std::mutex mLock; // protecting members below
bool mUseHalBufManager = false;
- hidl_vec<V3_2::Stream> mStreams;
+ hidl_vec<V3_4::Stream> mStreams;
hidl_vec<V3_2::HalStream> mHalStreams;
uint64_t mNextBufferId = 1;
using OutstandingBuffers = std::unordered_map<uint64_t, hidl_handle>;
@@ -865,6 +865,8 @@
int32_t partialResultCount;
// For buffer drop errors, the stream ID for the stream that lost a buffer.
+ // For physical sub-camera result errors, the Id of the physical stream
+ // for the physical sub-camera.
// Otherwise -1.
int32_t errorStreamId;
@@ -878,6 +880,8 @@
// return from HAL but framework.
::android::Vector<StreamBuffer> resultOutputBuffers;
+ std::unordered_set<string> expectedPhysicalResults;
+
InFlightRequest() :
shutterTimestamp(0),
errorCodeValid(false),
@@ -907,6 +911,24 @@
partialResultCount(0),
errorStreamId(-1),
hasInputBuffer(hasInput) {}
+
+ InFlightRequest(ssize_t numBuffers, bool hasInput,
+ bool partialResults, uint32_t partialCount,
+ const std::unordered_set<string>& extraPhysicalResult,
+ std::shared_ptr<ResultMetadataQueue> queue = nullptr) :
+ shutterTimestamp(0),
+ errorCodeValid(false),
+ errorCode(ErrorCode::ERROR_BUFFER),
+ usePartialResult(partialResults),
+ numPartialResults(partialCount),
+ resultQueue(queue),
+ haveResultMetadata(false),
+ numBuffersLeft(numBuffers),
+ frameNumber(0),
+ partialResultCount(0),
+ errorStreamId(-1),
+ hasInputBuffer(hasInput),
+ expectedPhysicalResults(extraPhysicalResult) {}
};
// Map from frame number to the in-flight request state
@@ -1124,6 +1146,13 @@
return notify;
}
+ if (physicalCameraMetadata.size() != request->expectedPhysicalResults.size()) {
+ ALOGE("%s: Frame %d: Returned physical metadata count %zu "
+ "must be equal to expected count %zu", __func__, frameNumber,
+ physicalCameraMetadata.size(), request->expectedPhysicalResults.size());
+ ADD_FAILURE();
+ return notify;
+ }
std::vector<::android::hardware::camera::device::V3_2::CameraMetadata> physResultMetadata;
physResultMetadata.resize(physicalCameraMetadata.size());
for (size_t i = 0; i < physicalCameraMetadata.size(); i++) {
@@ -1251,11 +1280,11 @@
}
void CameraHidlTest::DeviceCb::setCurrentStreamConfig(
- const hidl_vec<V3_2::Stream>& streams, const hidl_vec<V3_2::HalStream>& halStreams) {
+ const hidl_vec<V3_4::Stream>& streams, const hidl_vec<V3_2::HalStream>& halStreams) {
ASSERT_EQ(streams.size(), halStreams.size());
ASSERT_NE(streams.size(), 0);
for (size_t i = 0; i < streams.size(); i++) {
- ASSERT_EQ(streams[i].id, halStreams[i].id);
+ ASSERT_EQ(streams[i].v3_2.id, halStreams[i].id);
}
std::lock_guard<std::mutex> l(mLock);
mUseHalBufManager = true;
@@ -1293,16 +1322,6 @@
std::lock_guard<std::mutex> l(mParent->mLock);
for (size_t i = 0; i < messages.size(); i++) {
- ssize_t idx = mParent->mInflightMap.indexOfKey(
- messages[i].msg.shutter.frameNumber);
- if (::android::NAME_NOT_FOUND == idx) {
- ALOGE("%s: Unexpected frame number! received: %u",
- __func__, messages[i].msg.shutter.frameNumber);
- ADD_FAILURE();
- break;
- }
- InFlightRequest *r = mParent->mInflightMap.editValueAt(idx);
-
switch(messages[i].type) {
case MsgType::ERROR:
if (ErrorCode::ERROR_DEVICE == messages[i].msg.error.errorCode) {
@@ -1310,13 +1329,59 @@
__func__);
ADD_FAILURE();
} else {
- r->errorCodeValid = true;
- r->errorCode = messages[i].msg.error.errorCode;
- r->errorStreamId = messages[i].msg.error.errorStreamId;
+ ssize_t idx = mParent->mInflightMap.indexOfKey(
+ messages[i].msg.error.frameNumber);
+ if (::android::NAME_NOT_FOUND == idx) {
+ ALOGE("%s: Unexpected error frame number! received: %u",
+ __func__, messages[i].msg.error.frameNumber);
+ ADD_FAILURE();
+ break;
+ }
+ InFlightRequest *r = mParent->mInflightMap.editValueAt(idx);
+
+ if (ErrorCode::ERROR_RESULT == messages[i].msg.error.errorCode &&
+ messages[i].msg.error.errorStreamId != -1) {
+ if (r->haveResultMetadata) {
+ ALOGE("%s: Camera must report physical camera result error before "
+ "the final capture result!", __func__);
+ ADD_FAILURE();
+ } else {
+ for (size_t j = 0; j < mStreams.size(); j++) {
+ if (mStreams[j].v3_2.id == messages[i].msg.error.errorStreamId) {
+ hidl_string physicalCameraId = mStreams[j].physicalCameraId;
+ bool idExpected = r->expectedPhysicalResults.find(
+ physicalCameraId) != r->expectedPhysicalResults.end();
+ if (!idExpected) {
+ ALOGE("%s: ERROR_RESULT's error stream's physicalCameraId "
+ "%s must be expected", __func__,
+ physicalCameraId.c_str());
+ ADD_FAILURE();
+ } else {
+ r->expectedPhysicalResults.erase(physicalCameraId);
+ }
+ break;
+ }
+ }
+ }
+ } else {
+ r->errorCodeValid = true;
+ r->errorCode = messages[i].msg.error.errorCode;
+ r->errorStreamId = messages[i].msg.error.errorStreamId;
+ }
}
break;
case MsgType::SHUTTER:
+ {
+ ssize_t idx = mParent->mInflightMap.indexOfKey(messages[i].msg.shutter.frameNumber);
+ if (::android::NAME_NOT_FOUND == idx) {
+ ALOGE("%s: Unexpected shutter frame number! received: %u",
+ __func__, messages[i].msg.shutter.frameNumber);
+ ADD_FAILURE();
+ break;
+ }
+ InFlightRequest *r = mParent->mInflightMap.editValueAt(idx);
r->shutterTimestamp = messages[i].msg.shutter.timestamp;
+ }
break;
default:
ALOGE("%s: Unsupported notify message %d", __func__,
@@ -1357,7 +1422,7 @@
for (size_t i = 0; i < bufReqs.size(); i++) {
bool found = false;
for (size_t idx = 0; idx < mStreams.size(); idx++) {
- if (bufReqs[i].streamId == mStreams[idx].id) {
+ if (bufReqs[i].streamId == mStreams[idx].v3_2.id) {
found = true;
indexes[i] = idx;
break;
@@ -1381,7 +1446,7 @@
const auto& halStream = mHalStreams[idx];
const V3_5::BufferRequest& bufReq = bufReqs[i];
if (mOutstandingBufferIds[idx].size() + bufReq.numBuffersRequested > halStream.maxBuffers) {
- bufRets[i].streamId = stream.id;
+ bufRets[i].streamId = stream.v3_2.id;
bufRets[i].val.error(StreamBufferRequestError::MAX_BUFFER_EXCEEDED);
allStreamOk = false;
continue;
@@ -1390,17 +1455,17 @@
hidl_vec<StreamBuffer> tmpRetBuffers(bufReq.numBuffersRequested);
for (size_t j = 0; j < bufReq.numBuffersRequested; j++) {
hidl_handle buffer_handle;
- mParent->allocateGraphicBuffer(stream.width, stream.height,
+ mParent->allocateGraphicBuffer(stream.v3_2.width, stream.v3_2.height,
android_convertGralloc1To0Usage(
halStream.producerUsage, halStream.consumerUsage),
halStream.overrideFormat, &buffer_handle);
- tmpRetBuffers[j] = {stream.id, mNextBufferId, buffer_handle, BufferStatus::OK,
+ tmpRetBuffers[j] = {stream.v3_2.id, mNextBufferId, buffer_handle, BufferStatus::OK,
nullptr, nullptr};
mOutstandingBufferIds[idx].insert(std::make_pair(mNextBufferId++, buffer_handle));
}
atLeastOneStreamOk = true;
- bufRets[i].streamId = stream.id;
+ bufRets[i].streamId = stream.v3_2.id;
bufRets[i].val.buffers(std::move(tmpRetBuffers));
}
@@ -1430,7 +1495,7 @@
for (const auto& buf : buffers) {
bool found = false;
for (size_t idx = 0; idx < mOutstandingBufferIds.size(); idx++) {
- if (mStreams[idx].id == buf.streamId &&
+ if (mStreams[idx].v3_2.id == buf.streamId &&
mOutstandingBufferIds[idx].count(buf.bufferId) == 1) {
mOutstandingBufferIds[idx].erase(buf.bufferId);
// TODO: check do we need to close/delete native handle or assume we have enough
@@ -4157,7 +4222,7 @@
ASSERT_TRUE(resultQueueRet.isOk());
InFlightRequest inflightReq = {static_cast<ssize_t> (halStreamConfig.streams.size()), false,
- supportsPartialResults, partialResultCount, resultQueue};
+ supportsPartialResults, partialResultCount, physicalIds, resultQueue};
std::vector<hidl_handle> graphicBuffers;
graphicBuffers.reserve(halStreamConfig.streams.size());
@@ -4236,7 +4301,7 @@
request.v3_2.outputBuffers[0].buffer = nullptr;
mInflightMap.clear();
inflightReq = {static_cast<ssize_t> (physicalIds.size()), false,
- supportsPartialResults, partialResultCount, resultQueue};
+ supportsPartialResults, partialResultCount, physicalIds, resultQueue};
mInflightMap.add(request.v3_2.frameNumber, &inflightReq);
}
@@ -5315,10 +5380,10 @@
ASSERT_EQ(physicalIds.size(), halConfig.streams.size());
*halStreamConfig = halConfig;
if (*useHalBufManager) {
- hidl_vec<V3_2::Stream> streams(physicalIds.size());
+ hidl_vec<V3_4::Stream> streams(physicalIds.size());
hidl_vec<V3_2::HalStream> halStreams(physicalIds.size());
for (size_t i = 0; i < physicalIds.size(); i++) {
- streams[i] = streams3_4[i].v3_2;
+ streams[i] = streams3_4[i];
halStreams[i] = halConfig.streams[i].v3_3.v3_2;
}
cb->setCurrentStreamConfig(streams, halStreams);
@@ -5493,9 +5558,9 @@
halStreamConfig->streams.resize(1);
halStreamConfig->streams[0] = halConfig.streams[0].v3_3.v3_2;
if (*useHalBufManager) {
- hidl_vec<V3_2::Stream> streams(1);
+ hidl_vec<V3_4::Stream> streams(1);
hidl_vec<V3_2::HalStream> halStreams(1);
- streams[0] = stream3_2;
+ streams[0] = config3_4.streams[0];
halStreams[0] = halConfig.streams[0].v3_3.v3_2;
cb->setCurrentStreamConfig(streams, halStreams);
}
diff --git a/current.txt b/current.txt
index fbb9752..c7b0d9e 100644
--- a/current.txt
+++ b/current.txt
@@ -572,6 +572,7 @@
cfa81f229b69f9011c58f48264fcb552447430fe68610eac514e811e65bc306a android.hardware.wifi.supplicant@1.2::types
# ABI preserving changes to HALs during Android R
+2410dd02d67786a732d36e80b0f8ccf55086604ef37f9838e2013ff2c571e404 android.hardware.camera.device@3.5::types
b69a7615c508acf5c5201efd1bfa3262167874fc3594e2db5a3ff93addd8ac75 android.hardware.keymaster@4.0::IKeymasterDevice
ad431c8de51c07934a068e3043d8dd0537ac4d3158627706628b123f42df48dc android.hardware.neuralnetworks@1.0::IPreparedModel
aafcc10cf04ab247e86d4582586c71c6b4c2b8c479241ffa7fe37deb659fc942 android.hardware.neuralnetworks@1.2::IPreparedModel