Fix few issues in virtual camera service:
* Fix incorrect flush implementation (do not try to fetch element from
empty queue).
* Gracefully handle flush call even if render thread is not running.
* Make sure thread is terminated when close is called on session.
* Remove metadata from CaptureResult send when processing flush
request.
* Set streamId to -1 in ErrorRequest NotifyMsg (required, otherwise
it NotifyMsg fails validation in framework).
Bug: 301023410
Test: atest virtual_camera_tests
Test: OpenCamera
Change-Id: I899a9eaed0b69eed93945391c2d0a25120136267
diff --git a/services/camera/virtualcamera/VirtualCameraRenderThread.cc b/services/camera/virtualcamera/VirtualCameraRenderThread.cc
index 5756797..8a2db1c 100644
--- a/services/camera/virtualcamera/VirtualCameraRenderThread.cc
+++ b/services/camera/virtualcamera/VirtualCameraRenderThread.cc
@@ -100,7 +100,11 @@
NotifyMsg createRequestErrorNotifyMsg(int frameNumber) {
NotifyMsg msg;
msg.set<NotifyMsg::Tag::error>(ErrorMsg{
- .frameNumber = frameNumber, .errorCode = ErrorCode::ERROR_REQUEST});
+ .frameNumber = frameNumber,
+ // errorStreamId needs to be set to -1 for ERROR_REQUEST
+ // (not tied to specific stream).
+ .errorStreamId = -1,
+ .errorCode = ErrorCode::ERROR_REQUEST});
return msg;
}
@@ -164,8 +168,9 @@
void VirtualCameraRenderThread::flush() {
std::lock_guard<std::mutex> lock(mLock);
- for (auto task = std::move(mQueue.front()); !mQueue.empty();
- mQueue.pop_front()) {
+ while (!mQueue.empty()) {
+ std::unique_ptr<ProcessCaptureRequestTask> task = std::move(mQueue.front());
+ mQueue.pop_front();
flushCaptureRequest(*task);
}
}
@@ -233,6 +238,7 @@
CaptureResult captureResult;
captureResult.fmqResultSize = 0;
captureResult.frameNumber = request.getFrameNumber();
+ // Partial result needs to be set to 1 when metadata are present.
captureResult.partialResult = 1;
captureResult.inputBuffer.streamId = -1;
captureResult.physicalCameraMetadata.resize(0);
@@ -308,15 +314,10 @@
void VirtualCameraRenderThread::flushCaptureRequest(
const ProcessCaptureRequestTask& request) {
- const std::chrono::nanoseconds timestamp =
- std::chrono::duration_cast<std::chrono::nanoseconds>(
- std::chrono::steady_clock::now().time_since_epoch());
-
CaptureResult captureResult;
captureResult.fmqResultSize = 0;
captureResult.frameNumber = request.getFrameNumber();
captureResult.inputBuffer.streamId = -1;
- captureResult.result = createCaptureResultMetadata(timestamp);
const std::vector<CaptureRequestBuffer>& buffers = request.getBuffers();
captureResult.outputBuffers.resize(buffers.size());
diff --git a/services/camera/virtualcamera/VirtualCameraSession.cc b/services/camera/virtualcamera/VirtualCameraSession.cc
index ff6235f..34d9932 100644
--- a/services/camera/virtualcamera/VirtualCameraSession.cc
+++ b/services/camera/virtualcamera/VirtualCameraSession.cc
@@ -179,6 +179,14 @@
mVirtualCameraClientCallback->onStreamClosed(/*streamId=*/0);
}
+ {
+ std::lock_guard<std::mutex> lock(mLock);
+ if (mRenderThread != nullptr) {
+ mRenderThread->stop();
+ mRenderThread = nullptr;
+ }
+ }
+
mSessionContext.closeAllStreams();
return ndk::ScopedAStatus::ok();
}
@@ -275,7 +283,9 @@
ndk::ScopedAStatus VirtualCameraSession::flush() {
ALOGV("%s", __func__);
std::lock_guard<std::mutex> lock(mLock);
- mRenderThread->flush();
+ if (mRenderThread != nullptr) {
+ mRenderThread->flush();
+ }
return ndk::ScopedAStatus::ok();
}
diff --git a/services/camera/virtualcamera/VirtualCameraSession.h b/services/camera/virtualcamera/VirtualCameraSession.h
index 6df5d58..50962e5 100644
--- a/services/camera/virtualcamera/VirtualCameraSession.h
+++ b/services/camera/virtualcamera/VirtualCameraSession.h
@@ -56,7 +56,7 @@
virtual ~VirtualCameraSession() override = default;
- ndk::ScopedAStatus close() override;
+ ndk::ScopedAStatus close() override EXCLUDES(mLock);
ndk::ScopedAStatus configureStreams(
const ::aidl::android::hardware::camera::device::StreamConfiguration&
diff --git a/services/camera/virtualcamera/tests/VirtualCameraRenderThreadTest.cc b/services/camera/virtualcamera/tests/VirtualCameraRenderThreadTest.cc
index 2e40d16..5f899b8 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraRenderThreadTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraRenderThreadTest.cc
@@ -46,6 +46,7 @@
using ::aidl::android::hardware::camera::device::BufferRequestStatus;
using ::aidl::android::hardware::camera::device::BufferStatus;
using ::aidl::android::hardware::camera::device::CaptureResult;
+using ::aidl::android::hardware::camera::device::ErrorCode;
using ::aidl::android::hardware::camera::device::ErrorMsg;
using ::aidl::android::hardware::camera::device::NotifyMsg;
using ::aidl::android::hardware::camera::device::StreamBuffer;
@@ -73,7 +74,11 @@
Matcher<NotifyMsg> IsRequestErrorNotifyMsg(const int frameId) {
return AllOf(Property(&NotifyMsg::getTag, Eq(NotifyMsg::error)),
Property(&NotifyMsg::get<NotifyMsg::error>,
- Field(&ErrorMsg::frameNumber, Eq(frameId))));
+ Field(&ErrorMsg::frameNumber, Eq(frameId))),
+ Property(&NotifyMsg::get<NotifyMsg::error>,
+ Field(&ErrorMsg::errorStreamId, Eq(-1))),
+ Property(&NotifyMsg::get<NotifyMsg::error>,
+ Field(&ErrorMsg::errorCode, Eq(ErrorCode::ERROR_REQUEST))));
}
class MockCameraDeviceCallback : public BnCameraDeviceCallback {
@@ -143,4 +148,4 @@
} // namespace
} // namespace virtualcamera
} // namespace companion
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc b/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
index 0bc5210..74505af 100644
--- a/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
+++ b/services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc
@@ -175,6 +175,14 @@
ASSERT_TRUE(mVirtualCameraSession->close().isOk());
}
+TEST_F(VirtualCameraSessionTest, FlushBeforeConfigure) {
+ // Flush request coming before the configure request finished
+ // (so potentially the thread is not yet running) should be
+ // gracefully handled.
+
+ EXPECT_TRUE(mVirtualCameraSession->flush().isOk());
+}
+
TEST_F(VirtualCameraSessionTest, onProcessCaptureRequestTriggersClientCallback) {
StreamConfiguration streamConfiguration;
streamConfiguration.streams = {