Camera: patching treble camera HAL
Bug fixes like deadlock resolution, wrong enum usage etc.
Bug: 30985004
Test: run Camera2 API CTS tests on Angler
Change-Id: I661fa9197f66344ddecca8f68d343c891806eca1
diff --git a/camera/device/3.2/default/CameraDevice.cpp b/camera/device/3.2/default/CameraDevice.cpp
index 51a29ec..18e0e7b 100644
--- a/camera/device/3.2/default/CameraDevice.cpp
+++ b/camera/device/3.2/default/CameraDevice.cpp
@@ -212,7 +212,7 @@
if (res != OK) {
ALOGE("%s: cannot open camera %s!", __FUNCTION__, mCameraId.c_str());
mLock.unlock();
- _hidl_cb(Status::INTERNAL_ERROR, nullptr);
+ _hidl_cb(getHidlStatus(res), nullptr);
return Void();
}
diff --git a/camera/device/3.2/default/CameraDeviceSession.cpp b/camera/device/3.2/default/CameraDeviceSession.cpp
index 7e43cd7..de61d83 100644
--- a/camera/device/3.2/default/CameraDeviceSession.cpp
+++ b/camera/device/3.2/default/CameraDeviceSession.cpp
@@ -94,7 +94,12 @@
if (handle == nullptr || handle->numFds == 0) {
fd = -1;
} else if (handle->numFds == 1) {
+//TODO(b/34110242): make this hidl transport agnostic
+#ifdef BINDERIZED
fd = dup(handle->data[0]);
+#else
+ fd = handle->data[0];
+#endif
if (fd < 0) {
ALOGE("failed to dup fence fd %d", handle->data[0]);
return false;
@@ -110,9 +115,13 @@
void closeFence(int fd)
{
+#ifdef BINDERIZED
if (fd >= 0) {
close(fd);
}
+#else
+ (void) fd;
+#endif
}
private:
@@ -226,7 +235,6 @@
camera3_callback_ops({&sProcessCaptureResult, &sNotify}),
mDevice(device),
mCallback(callback) {
-
// For now, we init sHandleImporter but do not cleanup (keep it alive until
// HAL process ends)
sHandleImporter.initialize();
@@ -293,31 +301,45 @@
Status CameraDeviceSession::importRequest(
const CaptureRequest& request,
- hidl_vec<buffer_handle_t>& allBufs,
+ hidl_vec<buffer_handle_t*>& allBufPtrs,
hidl_vec<int>& allFences) {
bool hasInputBuf = (request.inputBuffer.streamId != -1 &&
- request.inputBuffer.buffer.getNativeHandle() == nullptr);
+ request.inputBuffer.buffer.getNativeHandle() != nullptr);
size_t numOutputBufs = request.outputBuffers.size();
size_t numBufs = numOutputBufs + (hasInputBuf ? 1 : 0);
// Validate all I/O buffers
+ hidl_vec<buffer_handle_t> allBufs;
allBufs.resize(numBufs);
+ allBufPtrs.resize(numBufs);
allFences.resize(numBufs);
+ std::vector<int32_t> streamIds(numBufs);
+
for (size_t i = 0; i < numOutputBufs; i++) {
allBufs[i] = request.outputBuffers[i].buffer.getNativeHandle();
+ allBufPtrs[i] = &allBufs[i];
+ streamIds[i] = request.outputBuffers[i].streamId;
}
if (hasInputBuf) {
allBufs[numOutputBufs] = request.inputBuffer.buffer.getNativeHandle();
+ allBufPtrs[numOutputBufs] = &allBufs[numOutputBufs];
+ streamIds[numOutputBufs] = request.inputBuffer.streamId;
}
+
for (size_t i = 0; i < numBufs; i++) {
buffer_handle_t buf = allBufs[i];
- sHandleImporter.importBuffer(buf);
- if (buf == nullptr) {
- ALOGE("%s: output buffer %zu is invalid!", __FUNCTION__, i);
- cleanupInflightBufferFences(allBufs, i, allFences, 0);
- return Status::INTERNAL_ERROR;
- } else {
- allBufs[i] = buf;
+ CirculatingBuffers& cbs = mCirculatingBuffers[streamIds[i]];
+ if (cbs.count(buf) == 0) {
+ // Register a newly seen buffer
+ buffer_handle_t importedBuf = buf;
+ sHandleImporter.importBuffer(importedBuf);
+ if (importedBuf == nullptr) {
+ ALOGE("%s: output buffer %zu is invalid!", __FUNCTION__, i);
+ return Status::INTERNAL_ERROR;
+ } else {
+ cbs[buf] = importedBuf;
+ }
}
+ allBufPtrs[i] = &cbs[buf];
}
// All buffers are imported. Now validate output buffer acquire fences
@@ -325,7 +347,7 @@
if (!sHandleImporter.importFence(
request.outputBuffers[i].acquireFence, allFences[i])) {
ALOGE("%s: output buffer %zu acquire fence is invalid", __FUNCTION__, i);
- cleanupInflightBufferFences(allBufs, numBufs, allFences, i);
+ cleanupInflightFences(allFences, i);
return Status::INTERNAL_ERROR;
}
}
@@ -335,19 +357,15 @@
if (!sHandleImporter.importFence(
request.inputBuffer.acquireFence, allFences[numOutputBufs])) {
ALOGE("%s: input buffer acquire fence is invalid", __FUNCTION__);
- cleanupInflightBufferFences(allBufs, numBufs, allFences, numOutputBufs);
+ cleanupInflightFences(allFences, numOutputBufs);
return Status::INTERNAL_ERROR;
}
}
return Status::OK;
}
-void CameraDeviceSession::cleanupInflightBufferFences(
- hidl_vec<buffer_handle_t>& allBufs, size_t numBufs,
+void CameraDeviceSession::cleanupInflightFences(
hidl_vec<int>& allFences, size_t numFences) {
- for (size_t j = 0; j < numBufs; j++) {
- sHandleImporter.freeBuffer(allBufs[j]);
- }
for (size_t j = 0; j < numFences; j++) {
sHandleImporter.closeFence(allFences[j]);
}
@@ -378,13 +396,20 @@
Return<void> CameraDeviceSession::configureStreams(
const StreamConfiguration& requestedConfiguration, configureStreams_cb _hidl_cb) {
Status status = initStatus();
+ HalStreamConfiguration outStreams;
+
+ // hold the inflight lock for entire configureStreams scope since there must not be any
+ // inflight request/results during stream configuration.
+ Mutex::Autolock _l(mInflightLock);
if (!mInflightBuffers.empty()) {
ALOGE("%s: trying to configureStreams while there are still %zu inflight buffers!",
__FUNCTION__, mInflightBuffers.size());
- status = Status::INTERNAL_ERROR;
+ _hidl_cb(Status::INTERNAL_ERROR, outStreams);
+ return Void();
}
- HalStreamConfiguration outStreams;
+
+
if (status == Status::OK) {
camera3_stream_configuration_t stream_list;
hidl_vec<camera3_stream_t*> streams;
@@ -394,18 +419,62 @@
streams.resize(stream_list.num_streams);
stream_list.streams = streams.data();
- mStreamMap.clear();
for (uint32_t i = 0; i < stream_list.num_streams; i++) {
- Camera3Stream stream;
- convertFromHidl(requestedConfiguration.streams[i], &stream);
- mStreamMap[stream.mId] = stream;
- streams[i] = &mStreamMap[stream.mId];
+ int id = requestedConfiguration.streams[i].id;
+
+ if (mStreamMap.count(id) == 0) {
+ Camera3Stream stream;
+ convertFromHidl(requestedConfiguration.streams[i], &stream);
+ mStreamMap[id] = stream;
+ mCirculatingBuffers.emplace(stream.mId, CirculatingBuffers{});
+ } else {
+ // width/height/format must not change, but usage/rotation might need to change
+ if (mStreamMap[id].stream_type !=
+ (int) requestedConfiguration.streams[i].streamType ||
+ mStreamMap[id].width != requestedConfiguration.streams[i].width ||
+ mStreamMap[id].height != requestedConfiguration.streams[i].height ||
+ mStreamMap[id].format != (int) requestedConfiguration.streams[i].format ||
+ mStreamMap[id].data_space != (android_dataspace_t)
+ requestedConfiguration.streams[i].dataSpace) {
+ ALOGE("%s: stream %d configuration changed!", __FUNCTION__, id);
+ _hidl_cb(Status::INTERNAL_ERROR, outStreams);
+ return Void();
+ }
+ mStreamMap[id].rotation = (int) requestedConfiguration.streams[i].rotation;
+ mStreamMap[id].usage = (uint32_t) requestedConfiguration.streams[i].usage;
+ }
+ streams[i] = &mStreamMap[id];
}
ATRACE_BEGIN("camera3->configure_streams");
status_t ret = mDevice->ops->configure_streams(mDevice, &stream_list);
ATRACE_END();
+ // delete unused streams, note we do this after adding new streams to ensure new stream
+ // will not have the same address as deleted stream, and HAL has a chance to reference
+ // the to be deleted stream in configure_streams call
+ for(auto it = mStreamMap.begin(); it != mStreamMap.end();) {
+ int id = it->first;
+ bool found = false;
+ for (const auto& stream : requestedConfiguration.streams) {
+ if (id == stream.id) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ // Unmap all buffers of deleted stream
+ for (auto& pair : mCirculatingBuffers.at(id)) {
+ sHandleImporter.freeBuffer(pair.second);
+ }
+ mCirculatingBuffers[id].clear();
+ mCirculatingBuffers.erase(id);
+ it = mStreamMap.erase(it);
+ } else {
+ ++it;
+ }
+ }
+
if (ret == -EINVAL) {
status = Status::ILLEGAL_ARGUMENT;
} else if (ret != OK) {
@@ -434,57 +503,55 @@
return Status::INTERNAL_ERROR;
}
- hidl_vec<buffer_handle_t> allBufs;
+ hidl_vec<buffer_handle_t*> allBufPtrs;
hidl_vec<int> allFences;
bool hasInputBuf = (request.inputBuffer.streamId != -1 &&
- request.inputBuffer.buffer.getNativeHandle() == nullptr);
+ request.inputBuffer.buffer.getNativeHandle() != nullptr);
size_t numOutputBufs = request.outputBuffers.size();
size_t numBufs = numOutputBufs + (hasInputBuf ? 1 : 0);
- status = importRequest(request, allBufs, allFences);
+ status = importRequest(request, allBufPtrs, allFences);
if (status != Status::OK) {
return status;
}
- if (hasInputBuf) {
- auto key = std::make_pair(request.inputBuffer.streamId, request.frameNumber);
- auto& bufCache = mInflightBuffers[key] = StreamBufferCache{};
- // The last parameter (&bufCache) must be in heap, or we will
- // send a pointer pointing to stack memory to HAL and later HAL will break
- // when trying to accessing it after this call returned.
- convertFromHidl(
- allBufs[numOutputBufs], request.inputBuffer.status,
- &mStreamMap[request.inputBuffer.streamId], allFences[numOutputBufs],
- &bufCache);
- halRequest.input_buffer = &(bufCache.mStreamBuffer);
- } else {
- halRequest.input_buffer = nullptr;
- }
-
- halRequest.num_output_buffers = numOutputBufs;
hidl_vec<camera3_stream_buffer_t> outHalBufs;
outHalBufs.resize(numOutputBufs);
- for (size_t i = 0; i < numOutputBufs; i++) {
- auto key = std::make_pair(request.outputBuffers[i].streamId, request.frameNumber);
- auto& bufCache = mInflightBuffers[key] = StreamBufferCache{};
- // The last parameter (&bufCache) must be in heap, or we will
- // send a pointer pointing to stack memory to HAL and later HAL will break
- // when trying to accessing it after this call returned.
- convertFromHidl(
- allBufs[i], request.outputBuffers[i].status,
- &mStreamMap[request.outputBuffers[i].streamId], allFences[i],
- &bufCache);
- outHalBufs[i] = bufCache.mStreamBuffer;
+ {
+ Mutex::Autolock _l(mInflightLock);
+ if (hasInputBuf) {
+ auto key = std::make_pair(request.inputBuffer.streamId, request.frameNumber);
+ auto& bufCache = mInflightBuffers[key] = camera3_stream_buffer_t{};
+ convertFromHidl(
+ allBufPtrs[numOutputBufs], request.inputBuffer.status,
+ &mStreamMap[request.inputBuffer.streamId], allFences[numOutputBufs],
+ &bufCache);
+ halRequest.input_buffer = &bufCache;
+ } else {
+ halRequest.input_buffer = nullptr;
+ }
+
+ halRequest.num_output_buffers = numOutputBufs;
+ for (size_t i = 0; i < numOutputBufs; i++) {
+ auto key = std::make_pair(request.outputBuffers[i].streamId, request.frameNumber);
+ auto& bufCache = mInflightBuffers[key] = camera3_stream_buffer_t{};
+ convertFromHidl(
+ allBufPtrs[i], request.outputBuffers[i].status,
+ &mStreamMap[request.outputBuffers[i].streamId], allFences[i],
+ &bufCache);
+ outHalBufs[i] = bufCache;
+ }
+ halRequest.output_buffers = outHalBufs.data();
}
- halRequest.output_buffers = outHalBufs.data();
ATRACE_ASYNC_BEGIN("frame capture", request.frameNumber);
ATRACE_BEGIN("camera3->process_capture_request");
status_t ret = mDevice->ops->process_capture_request(mDevice, &halRequest);
ATRACE_END();
if (ret != OK) {
+ Mutex::Autolock _l(mInflightLock);
ALOGE("%s: HAL process_capture_request call failed!", __FUNCTION__);
- cleanupInflightBufferFences(allBufs, numBufs, allFences, numBufs);
+ cleanupInflightFences(allFences, numBufs);
if (hasInputBuf) {
auto key = std::make_pair(request.inputBuffer.streamId, request.frameNumber);
mInflightBuffers.erase(key);
@@ -514,9 +581,26 @@
Return<void> CameraDeviceSession::close() {
Mutex::Autolock _l(mStateLock);
if (!mClosed) {
+ {
+ Mutex::Autolock _l(mInflightLock);
+ if (!mInflightBuffers.empty()) {
+ ALOGE("%s: trying to close while there are still %zu inflight buffers!",
+ __FUNCTION__, mInflightBuffers.size());
+ }
+ }
+
ATRACE_BEGIN("camera3->close");
mDevice->common.close(&mDevice->common);
ATRACE_END();
+
+ // free all imported buffers
+ for(auto& pair : mCirculatingBuffers) {
+ CirculatingBuffers& buffers = pair.second;
+ for (auto& p2 : buffers) {
+ sHandleImporter.freeBuffer(p2.second);
+ }
+ }
+
mClosed = true;
}
return Void();
@@ -536,25 +620,28 @@
size_t numOutputBufs = hal_result->num_output_buffers;
size_t numBufs = numOutputBufs + (hasInputBuf ? 1 : 0);
Status status = Status::OK;
- if (hasInputBuf) {
- int streamId = static_cast<Camera3Stream*>(hal_result->input_buffer->stream)->mId;
- // validate if buffer is inflight
- auto key = std::make_pair(streamId, frameNumber);
- if (d->mInflightBuffers.count(key) != 1) {
- ALOGE("%s: input buffer for stream %d frame %d is not inflight!",
- __FUNCTION__, streamId, frameNumber);
- return;
+ {
+ Mutex::Autolock _l(d->mInflightLock);
+ if (hasInputBuf) {
+ int streamId = static_cast<Camera3Stream*>(hal_result->input_buffer->stream)->mId;
+ // validate if buffer is inflight
+ auto key = std::make_pair(streamId, frameNumber);
+ if (d->mInflightBuffers.count(key) != 1) {
+ ALOGE("%s: input buffer for stream %d frame %d is not inflight!",
+ __FUNCTION__, streamId, frameNumber);
+ return;
+ }
}
- }
- for (size_t i = 0; i < numOutputBufs; i++) {
- int streamId = static_cast<Camera3Stream*>(hal_result->output_buffers[i].stream)->mId;
- // validate if buffer is inflight
- auto key = std::make_pair(streamId, frameNumber);
- if (d->mInflightBuffers.count(key) != 1) {
- ALOGE("%s: output buffer for stream %d frame %d is not inflight!",
- __FUNCTION__, streamId, frameNumber);
- return;
+ for (size_t i = 0; i < numOutputBufs; i++) {
+ int streamId = static_cast<Camera3Stream*>(hal_result->output_buffers[i].stream)->mId;
+ // validate if buffer is inflight
+ auto key = std::make_pair(streamId, frameNumber);
+ if (d->mInflightBuffers.count(key) != 1) {
+ ALOGE("%s: output buffer for stream %d frame %d is not inflight!",
+ __FUNCTION__, streamId, frameNumber);
+ return;
+ }
}
}
// We don't need to validate/import fences here since we will be passing them to camera service
@@ -576,6 +663,8 @@
releaseFences[numOutputBufs] = native_handle_create(/*numFds*/1, /*numInts*/0);
releaseFences[numOutputBufs]->data[0] = hal_result->input_buffer->release_fence;
result.inputBuffer.releaseFence = releaseFences[numOutputBufs];
+ } else {
+ releaseFences[numOutputBufs] = nullptr;
}
} else {
result.inputBuffer.streamId = -1;
@@ -592,33 +681,42 @@
releaseFences[i] = native_handle_create(/*numFds*/1, /*numInts*/0);
releaseFences[i]->data[0] = hal_result->output_buffers[i].release_fence;
result.outputBuffers[i].releaseFence = releaseFences[i];
+ } else {
+ releaseFences[i] = nullptr;
+ }
+ }
+
+ // Free inflight record/fences.
+ // Do this before call back to camera service because camera service might jump to
+ // configure_streams right after the processCaptureResult call so we need to finish
+ // updating inflight queues first
+ {
+ Mutex::Autolock _l(d->mInflightLock);
+ if (hasInputBuf) {
+ int streamId = static_cast<Camera3Stream*>(hal_result->input_buffer->stream)->mId;
+ auto key = std::make_pair(streamId, frameNumber);
+ sHandleImporter.closeFence(d->mInflightBuffers[key].acquire_fence);
+ d->mInflightBuffers.erase(key);
+ }
+
+ for (size_t i = 0; i < numOutputBufs; i++) {
+ int streamId = static_cast<Camera3Stream*>(hal_result->output_buffers[i].stream)->mId;
+ auto key = std::make_pair(streamId, frameNumber);
+ sHandleImporter.closeFence(d->mInflightBuffers[key].acquire_fence);
+ d->mInflightBuffers.erase(key);
+ }
+
+ if (d->mInflightBuffers.empty()) {
+ ALOGV("%s: inflight buffer queue is now empty!", __FUNCTION__);
}
}
d->mCallback->processCaptureResult(result);
- // Free cached buffer/fences.
- if (hasInputBuf) {
- int streamId = static_cast<Camera3Stream*>(hal_result->input_buffer->stream)->mId;
- auto key = std::make_pair(streamId, frameNumber);
- sHandleImporter.closeFence(d->mInflightBuffers[key].mStreamBuffer.acquire_fence);
- sHandleImporter.freeBuffer(d->mInflightBuffers[key].mBuffer);
- d->mInflightBuffers.erase(key);
- }
-
- for (size_t i = 0; i < numOutputBufs; i++) {
- int streamId = static_cast<Camera3Stream*>(hal_result->output_buffers[i].stream)->mId;
- auto key = std::make_pair(streamId, frameNumber);
- sHandleImporter.closeFence(d->mInflightBuffers[key].mStreamBuffer.acquire_fence);
- sHandleImporter.freeBuffer(d->mInflightBuffers[key].mBuffer);
- d->mInflightBuffers.erase(key);
- }
-
for (size_t i = 0; i < releaseFences.size(); i++) {
// We don't close the FD here as HAL needs to signal it later.
native_handle_delete(releaseFences[i]);
}
-
}
void CameraDeviceSession::sNotify(
@@ -628,7 +726,8 @@
const_cast<CameraDeviceSession*>(static_cast<const CameraDeviceSession*>(cb));
NotifyMsg hidlMsg;
convertToHidl(msg, &hidlMsg);
- if (hidlMsg.type == (MsgType) CAMERA3_MSG_ERROR) {
+ if (hidlMsg.type == (MsgType) CAMERA3_MSG_ERROR &&
+ hidlMsg.msg.error.errorStreamId != -1) {
if (d->mStreamMap.count(hidlMsg.msg.error.errorStreamId) != 1) {
ALOGE("%s: unknown stream ID %d reports an error!",
__FUNCTION__, hidlMsg.msg.error.errorStreamId);
diff --git a/camera/device/3.2/default/CameraDeviceSession.h b/camera/device/3.2/default/CameraDeviceSession.h
index 3e01b1a..498617e 100644
--- a/camera/device/3.2/default/CameraDeviceSession.h
+++ b/camera/device/3.2/default/CameraDeviceSession.h
@@ -17,6 +17,7 @@
#ifndef ANDROID_HARDWARE_CAMERA_DEVICE_V3_2_CAMERADEVICE3SESSION_H
#define ANDROID_HARDWARE_CAMERA_DEVICE_V3_2_CAMERADEVICE3SESSION_H
+#include <unordered_map>
#include "hardware/camera_common.h"
#include "hardware/camera3.h"
#include "utils/Mutex.h"
@@ -91,11 +92,54 @@
bool mDisconnected = false;
camera3_device_t* mDevice;
- const sp<ICameraDeviceCallback>& mCallback;
+ const sp<ICameraDeviceCallback> mCallback;
// Stream ID -> Camera3Stream cache
std::map<int, Camera3Stream> mStreamMap;
+
+ mutable Mutex mInflightLock; // protecting mInflightBuffers and mCirculatingBuffers
// (streamID, frameNumber) -> inflight buffer cache
- std::map<std::pair<int, uint32_t>, StreamBufferCache> mInflightBuffers;
+ std::map<std::pair<int, uint32_t>, camera3_stream_buffer_t> mInflightBuffers;
+
+ struct BufferHasher {
+ size_t operator()(const buffer_handle_t& buf) const {
+ if (buf == nullptr)
+ return 0;
+
+ size_t result = 1;
+ result = 31 * result + buf->numFds;
+ result = 31 * result + buf->numInts;
+ int length = buf->numFds + buf->numInts;
+ for (int i = 0; i < length; i++) {
+ result = 31 * result + buf->data[i];
+ }
+ return result;
+ }
+ };
+
+ struct BufferComparator {
+ bool operator()(const buffer_handle_t& buf1, const buffer_handle_t& buf2) const {
+ if (buf1->numFds == buf2->numFds && buf1->numInts == buf2->numInts) {
+ int length = buf1->numFds + buf1->numInts;
+ for (int i = 0; i < length; i++) {
+ if (buf1->data[i] != buf2->data[i]) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+ }
+ };
+
+ // buffers currently ciculating between HAL and camera service
+ // key: buffer_handle_t sent via HIDL interface
+ // value: imported buffer_handle_t
+ // Buffer will be imported during process_capture_request and will be freed
+ // when the its stream is deleted or camera device session is closed
+ typedef std::unordered_map<buffer_handle_t, buffer_handle_t,
+ BufferHasher, BufferComparator> CirculatingBuffers;
+ // Stream ID -> circulating buffers map
+ std::map<int, CirculatingBuffers> mCirculatingBuffers;
bool mInitFail;
bool initialize();
@@ -103,13 +147,12 @@
Status initStatus() const;
// Validate and import request's input buffer and acquire fence
- static Status importRequest(
+ Status importRequest(
const CaptureRequest& request,
- hidl_vec<buffer_handle_t>& allBufs,
+ hidl_vec<buffer_handle_t*>& allBufPtrs,
hidl_vec<int>& allFences);
- static void cleanupInflightBufferFences(
- hidl_vec<buffer_handle_t>& allBufs, size_t numBufs,
+ static void cleanupInflightFences(
hidl_vec<int>& allFences, size_t numFences);
/**
diff --git a/camera/device/3.2/default/convert.cpp b/camera/device/3.2/default/convert.cpp
index ae83e7d..35676df 100644
--- a/camera/device/3.2/default/convert.cpp
+++ b/camera/device/3.2/default/convert.cpp
@@ -50,6 +50,9 @@
// Note: existing data in dst will be gone. Caller still owns the memory of src
void convertToHidl(const camera_metadata_t *src, CameraMetadata* dst) {
+ if (src == nullptr) {
+ return;
+ }
size_t size = get_camera_metadata_size(src);
dst->setToExternal((uint8_t *) src, size);
return;
@@ -97,27 +100,29 @@
}
void convertFromHidl(
- buffer_handle_t bufIn, BufferStatus status, camera3_stream_t* stream, int acquireFence,
- StreamBufferCache* dst) {
- dst->mBuffer = bufIn;
- dst->mStreamBuffer.stream = stream;
- dst->mStreamBuffer.buffer = &dst->mBuffer;
- dst->mStreamBuffer.status = (int) status;
- dst->mStreamBuffer.acquire_fence = acquireFence;
- dst->mStreamBuffer.release_fence = -1; // meant for HAL to fill in
+ buffer_handle_t* bufPtr, BufferStatus status, camera3_stream_t* stream, int acquireFence,
+ camera3_stream_buffer_t* dst) {
+ dst->stream = stream;
+ dst->buffer = bufPtr;
+ dst->status = (int) status;
+ dst->acquire_fence = acquireFence;
+ dst->release_fence = -1; // meant for HAL to fill in
}
void convertToHidl(const camera3_notify_msg* src, NotifyMsg* dst) {
dst->type = (MsgType) src->type;
switch (src->type) {
case CAMERA3_MSG_ERROR:
- dst->msg.error.frameNumber = src->message.error.frame_number;
- // The camera3_stream_t* must be the same as what wrapper HAL passed to conventional
- // HAL, or the ID lookup will return garbage. Caller should validate the ID here is
- // indeed one of active stream IDs
- dst->msg.error.errorStreamId =
- static_cast<Camera3Stream*>(src->message.error.error_stream)->mId;
- dst->msg.error.errorCode = (ErrorCode) src->message.error.error_code;
+ {
+ // The camera3_stream_t* must be the same as what wrapper HAL passed to conventional
+ // HAL, or the ID lookup will return garbage. Caller should validate the ID here is
+ // indeed one of active stream IDs
+ Camera3Stream* stream = static_cast<Camera3Stream*>(
+ src->message.error.error_stream);
+ dst->msg.error.frameNumber = src->message.error.frame_number;
+ dst->msg.error.errorStreamId = (stream != nullptr) ? stream->mId : -1;
+ dst->msg.error.errorCode = (ErrorCode) src->message.error.error_code;
+ }
break;
case CAMERA3_MSG_SHUTTER:
dst->msg.shutter.frameNumber = src->message.shutter.frame_number;
diff --git a/camera/device/3.2/default/include/convert.h b/camera/device/3.2/default/include/convert.h
index f5ea564..96891f0 100644
--- a/camera/device/3.2/default/include/convert.h
+++ b/camera/device/3.2/default/include/convert.h
@@ -32,16 +32,6 @@
namespace V3_2 {
namespace implementation {
-// Cacheing the buffer/fence from camera service so HAL can reference the pointer after the
-// processCaptureRequest call has returned.
-// Remove the cache when:
-// 1. HAL API call failed, or
-// 2. HAL returns the buffer and the callback to camera service has returned
-struct StreamBufferCache {
- buffer_handle_t mBuffer;
- camera3_stream_buffer_t mStreamBuffer;
-};
-
// The camera3_stream_t sent to conventional HAL. Added mId fields to enable stream ID lookup
// fromt a downcasted camera3_stream
struct Camera3Stream : public camera3_stream {
@@ -55,14 +45,9 @@
void convertFromHidl(const Stream &src, Camera3Stream* dst);
void convertToHidl(const Camera3Stream* src, HalStream* dst);
-// dst->mStreamBuffer.buffer will be pointing to dst->mBuffer.
-// Most likely dst will be passed to HAL and HAL will try to access mStreamBuffer.buffer
-// after the API call returns. In that case caller must not use a local variable
-// within the scope of the API call to hold dst, because then dst->mStreamBuffer.buffer will be
-// invalid after the API call returns.
void convertFromHidl(
- buffer_handle_t, BufferStatus, camera3_stream_t*, int acquireFence, // inputs
- StreamBufferCache* dst);
+ buffer_handle_t*, BufferStatus, camera3_stream_t*, int acquireFence, // inputs
+ camera3_stream_buffer_t* dst);
void convertToHidl(const camera3_stream_configuration_t& src, HalStreamConfiguration* dst);
diff --git a/camera/provider/2.4/default/CameraProvider.cpp b/camera/provider/2.4/default/CameraProvider.cpp
index d9ac261..9617d8d 100644
--- a/camera/provider/2.4/default/CameraProvider.cpp
+++ b/camera/provider/2.4/default/CameraProvider.cpp
@@ -101,7 +101,7 @@
std::string cameraIdStr(camera_id);
TorchModeStatus status = (TorchModeStatus) new_status;
for (auto const& deviceNamePair : cp->mCameraDeviceNames) {
- if (cameraIdStr.compare(getLegacyCameraId(deviceNamePair.first)) == 0) {
+ if (cameraIdStr.compare(deviceNamePair.first) == 0) {
cp->mCallbacks->torchModeStatusChange(
deviceNamePair.second, status);
}
@@ -142,7 +142,7 @@
return -1;
}
if (sm[1].compare(kHAL3_2) == 0) {
- // maybe switched to 3.4 or define the hidl version enumlater
+ // maybe switched to 3.4 or define the hidl version enum later
return CAMERA_DEVICE_API_VERSION_3_2;
} else if (sm[1].compare(kHAL1_0) == 0) {
return CAMERA_DEVICE_API_VERSION_1_0;
@@ -229,8 +229,8 @@
// Setup vendor tags here so HAL can setup vendor keys in camera characteristics
VendorTagDescriptor::clearGlobalVendorTagDescriptor();
- setUpVendorTags();
- return false;
+ bool setupSucceed = setUpVendorTags();
+ return !setupSucceed; // return flag here is mInitFailed
}
bool CameraProvider::setUpVendorTags() {
@@ -311,7 +311,7 @@
}
}
hidl_vec<hidl_string> hidlDeviceNameList(deviceNameList);
- _hidl_cb (Status::OK, hidlDeviceNameList);
+ _hidl_cb(Status::OK, hidlDeviceNameList);
return Void();
}
@@ -321,12 +321,14 @@
return Void();
}
-Return<void> CameraProvider::getCameraDeviceInterface_V1_x(const hidl_string& /*cameraDeviceName*/, getCameraDeviceInterface_V1_x_cb /*_hidl_cb*/) {
+Return<void> CameraProvider::getCameraDeviceInterface_V1_x(
+ const hidl_string& /*cameraDeviceName*/, getCameraDeviceInterface_V1_x_cb /*_hidl_cb*/) {
// TODO implement after device 1.0 is implemented
return Void();
}
-Return<void> CameraProvider::getCameraDeviceInterface_V3_x(const hidl_string& cameraDeviceName, getCameraDeviceInterface_V3_x_cb _hidl_cb) {
+Return<void> CameraProvider::getCameraDeviceInterface_V3_x(
+ const hidl_string& cameraDeviceName, getCameraDeviceInterface_V3_x_cb _hidl_cb) {
std::smatch sm;
bool match = matchDeviceName(cameraDeviceName, sm);
if (!match) {
@@ -335,14 +337,18 @@
}
std::string cameraId = sm[2];
+ std::string deviceVersion = sm[1];
std::string deviceName(cameraDeviceName.c_str());
ssize_t index = mCameraDeviceNames.indexOf(std::make_pair(cameraId, deviceName));
if (index == NAME_NOT_FOUND) { // Either an illegal name or a device version mismatch
Status status = Status::OK;
ssize_t idx = mCameraIds.indexOf(cameraId);
if (idx == NAME_NOT_FOUND) {
+ ALOGE("%s: cannot find camera %s!", __FUNCTION__, cameraId.c_str());
status = Status::ILLEGAL_ARGUMENT;
} else { // invalid version
+ ALOGE("%s: camera device %s does not support version %s!",
+ __FUNCTION__, cameraId.c_str(), deviceVersion.c_str());
status = Status::OPERATION_NOT_SUPPORTED;
}
_hidl_cb(status, nullptr);
@@ -364,13 +370,13 @@
mModule, cameraId, mCameraDeviceNames);
if (device == nullptr) {
- ALOGE("%s: cannot allocate camera device!", __FUNCTION__);
+ ALOGE("%s: cannot allocate camera device for id %s", __FUNCTION__, cameraId.c_str());
_hidl_cb(Status::INTERNAL_ERROR, nullptr);
return Void();
}
if (device->isInitFailed()) {
- ALOGE("%s: camera device init failed!", __FUNCTION__);
+ ALOGE("%s: camera device %s init failed!", __FUNCTION__, cameraId.c_str());
device = nullptr;
_hidl_cb(Status::INTERNAL_ERROR, nullptr);
return Void();
diff --git a/camera/provider/2.4/default/CameraProvider.h b/camera/provider/2.4/default/CameraProvider.h
index 79899d9..8497ff3 100644
--- a/camera/provider/2.4/default/CameraProvider.h
+++ b/camera/provider/2.4/default/CameraProvider.h
@@ -61,8 +61,12 @@
Return<void> getVendorTags(getVendorTags_cb _hidl_cb) override;
Return<void> getCameraIdList(getCameraIdList_cb _hidl_cb) override;
Return<void> isSetTorchModeSupported(isSetTorchModeSupported_cb _hidl_cb) override;
- Return<void> getCameraDeviceInterface_V1_x(const hidl_string& cameraDeviceName, getCameraDeviceInterface_V1_x_cb _hidl_cb) override;
- Return<void> getCameraDeviceInterface_V3_x(const hidl_string& cameraDeviceName, getCameraDeviceInterface_V3_x_cb _hidl_cb) override;
+ Return<void> getCameraDeviceInterface_V1_x(
+ const hidl_string& cameraDeviceName,
+ getCameraDeviceInterface_V1_x_cb _hidl_cb) override;
+ Return<void> getCameraDeviceInterface_V3_x(
+ const hidl_string& cameraDeviceName,
+ getCameraDeviceInterface_V3_x_cb _hidl_cb) override;
private:
Mutex mCbLock;
diff --git a/camera/provider/2.4/vts/functional/camera_hidl_hal_test.cpp b/camera/provider/2.4/vts/functional/camera_hidl_hal_test.cpp
index b629dcc..0eb291c 100644
--- a/camera/provider/2.4/vts/functional/camera_hidl_hal_test.cpp
+++ b/camera/provider/2.4/vts/functional/camera_hidl_hal_test.cpp
@@ -78,8 +78,8 @@
public:
// get the test environment singleton
static CameraHidlEnvironment* Instance() {
- static CameraHidlEnvironment* instance = new CameraHidlEnvironment;
- return instance;
+ static CameraHidlEnvironment* instance = new CameraHidlEnvironment;
+ return instance;
}
virtual void SetUp() override;