Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b0d8cabe authored by Jan Sebechlebsky's avatar Jan Sebechlebsky
Browse files

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
parent 3b478c49

services/camera/virtualcamera/VirtualCameraRenderThread.cc

+9 −8
Original line number Diff line number Diff line
@@ -100,7 +100,11 @@ NotifyMsg createBufferErrorNotifyMsg(int frameNumber, int streamId) { 
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::enqueueTask( 


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 @@ void VirtualCameraRenderThread::processCaptureRequest( 
  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::processCaptureRequest( 


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());

services/camera/virtualcamera/VirtualCameraSession.cc

+11 −1
Original line number Diff line number Diff line
@@ -179,6 +179,14 @@ ndk::ScopedAStatus VirtualCameraSession::close() { 
    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::constructDefaultRequestSettings( 
ndk::ScopedAStatus VirtualCameraSession::flush() {

  ALOGV("%s", __func__);

  std::lock_guard<std::mutex> lock(mLock);

  if (mRenderThread != nullptr) {

    mRenderThread->flush();

  }

  return ndk::ScopedAStatus::ok();

}

services/camera/virtualcamera/VirtualCameraSession.h

+1 −1
Original line number Diff line number Diff line
@@ -56,7 +56,7 @@ class VirtualCameraSession 


  virtual ~VirtualCameraSession() override = default;



  ndk::ScopedAStatus close() override;

  ndk::ScopedAStatus close() override EXCLUDES(mLock);



  ndk::ScopedAStatus configureStreams(

      const ::aidl::android::hardware::camera::device::StreamConfiguration&

services/camera/virtualcamera/tests/VirtualCameraRenderThreadTest.cc

+7 −2
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ using ::aidl::android::hardware::camera::device::BufferRequest; 
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<StreamBuffer> IsStreamBufferWithStatus(const int streamId, 
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 {

services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc

+8 −0
Original line number Diff line number Diff line
@@ -175,6 +175,14 @@ TEST_F(VirtualCameraSessionTest, CloseTriggersClientTerminateCallback) { 
  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 = {