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
This commit is contained in:
Shuzhen Wang
2019-07-31 12:09:58 -07:00
parent c34e6683b1
commit a9bd7a4e4f
3 changed files with 97 additions and 30 deletions

View File

@@ -23,7 +23,8 @@ import @3.2::CameraBlobId;
/**
* 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.
*/

View File

@@ -622,7 +622,7 @@ public:
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 @@ public:
/* 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 @@ protected:
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 @@ protected:
// return from HAL but framework.
::android::Vector<StreamBuffer> resultOutputBuffers;
std::unordered_set<string> expectedPhysicalResults;
InFlightRequest() :
shutterTimestamp(0),
errorCodeValid(false),
@@ -907,6 +911,24 @@ protected:
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 @@ bool CameraHidlTest::DeviceCb::processCaptureResultLocked(const CaptureResult& r
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 @@ bool CameraHidlTest::DeviceCb::processCaptureResultLocked(const CaptureResult& r
}
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 @@ Return<void> CameraHidlTest::DeviceCb::notify(
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 @@ Return<void> CameraHidlTest::DeviceCb::notify(
__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 @@ Return<void> CameraHidlTest::DeviceCb::requestStreamBuffers(
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 @@ Return<void> CameraHidlTest::DeviceCb::requestStreamBuffers(
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 @@ Return<void> CameraHidlTest::DeviceCb::requestStreamBuffers(
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 @@ Return<void> CameraHidlTest::DeviceCb::returnStreamBuffers(
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 @@ TEST_F(CameraHidlTest, processMultiCaptureRequestPreview) {
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 @@ TEST_F(CameraHidlTest, processMultiCaptureRequestPreview) {
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 @@ void CameraHidlTest::configurePreviewStreams3_4(const std::string &name, int32_t
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 @@ void CameraHidlTest::configurePreviewStream(const std::string &name, int32_t dev
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);
}

View File

@@ -572,6 +572,7 @@ efbb061c969fa9553d243da6ee23b83fe5d4aa663a7b8896adc52e2b015bc2f3 android.hardwar
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