From 13e8a83e7b6f95eaa0d87dfeec9c50318c4ba3b5 Mon Sep 17 00:00:00 2001 From: "Singh, Sapna1" Date: Wed, 15 Feb 2023 21:07:06 +0530 Subject: [PATCH 1/5] Modularize code and add a helper class for camera info validation Modularize all the socket communication code for handling all the incoming requests dynamically and add a helper class that facilitates validation of all the camera info metadata. Tracked-On: OAM-106780 Signed-off-by: Singh, Sapna1 --- Android.mk | 3 +- include/ClientCommunicator.h | 9 +- src/ClientCommunicator.cpp | 326 +++++++++++++++-------------------- src/VirtualCameraFactory.cpp | 2 +- 4 files changed, 147 insertions(+), 193 deletions(-) diff --git a/Android.mk b/Android.mk index 3aea96a..aa42054 100644 --- a/Android.mk +++ b/Android.mk @@ -18,7 +18,6 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) - ##################### Build camera-vhal ####################### LOCAL_MODULE := camera.$(TARGET_PRODUCT) @@ -37,6 +36,7 @@ camera_vhal_src := \ src/Exif.cpp \ src/Thumbnail.cpp \ src/ClientCommunicator.cpp \ + src/CapabilitiesHelper.cpp \ src/ConnectionsListener.cpp \ src/CameraSocketCommand.cpp \ src/onevpl-video-decode/MfxDecoder.cpp \ @@ -137,4 +137,3 @@ LOCAL_MODULE := camera.$(TARGET_PRODUCT).jpeg include $(BUILD_SHARED_LIBRARY) ###################################################### - diff --git a/include/ClientCommunicator.h b/include/ClientCommunicator.h index bc9418e..d43b004 100644 --- a/include/ClientCommunicator.h +++ b/include/ClientCommunicator.h @@ -35,6 +35,7 @@ #include "onevpl-video-decode/MfxDecoder.h" #include "CameraSocketCommand.h" #include "ConnectionsListener.h" +#include "CapabilitiesHelper.h" #include "VirtualBuffer.h" namespace android { @@ -57,7 +58,10 @@ class ClientCommunicator { bool clientThread(); bool threadLooper(); - void configureCapabilities(); + void sendCameraCapabilities(); + void handleCameraInfo(uint32_t header_size); + void sendAck(); + void handleIncomingFrames(uint32_t header_size); Mutex mMutex; static Mutex sMutex; //Synchronize across threads @@ -66,9 +70,11 @@ class ClientCommunicator { int mClientFd = -1; int mNumOfCamerasRequested; // Number of cameras requested to support by client. bool mIsConfigurationDone = false; + bool mValidClientCapInfo = false; std::shared_ptr mListener; std::shared_ptr mVideoDecoder; + CapabilitiesHelper mCapabilitiesHelper; // maximum size of a H264 packet in any aggregation packet is 65535 bytes. // Source: https://tools.ietf.org/html/rfc6184#page-13 @@ -83,6 +89,7 @@ class ClientCommunicator { bool validOrientation = false; bool validCameraFacing = false; }; + }; } // namespace android diff --git a/src/ClientCommunicator.cpp b/src/ClientCommunicator.cpp index a9be650..e61bbf6 100644 --- a/src/ClientCommunicator.cpp +++ b/src/ClientCommunicator.cpp @@ -94,25 +94,12 @@ status_t ClientCommunicator::sendCommandToClient(camera_packet_t *config_cmd_pac return true; } -void ClientCommunicator::configureCapabilities() { +void ClientCommunicator::sendCameraCapabilities() { ALOGVV("%s(%d) Enter", __FUNCTION__, mClientId); - - bool valid_client_cap_info = false; - int camera_id, expctd_cam_id; - struct ValidateClientCapability val_client_cap[MAX_NUMBER_OF_SUPPORTED_CAMERAS]; - size_t ack_packet_size = sizeof(camera_header_t) + sizeof(camera_ack_t); + Mutex::Autolock al(mMutex); size_t cap_packet_size = sizeof(camera_header_t) + sizeof(camera_capability_t); - ssize_t recv_size = 0; - mNumOfCamerasRequested = 0; - camera_ack_t ack_payload = ACK_CONFIG; - camera_capability_t capability = {}; - camera_packet_t *cap_packet = NULL; - camera_packet_t *ack_packet = NULL; - camera_header_t header = {}; - - Mutex::Autolock al(mMutex); cap_packet = (camera_packet_t *)malloc(cap_packet_size); if (cap_packet == NULL) { ALOGE("%s(%d): cap camera_packet_t allocation failed: %d ", __FUNCTION__, mClientId, __LINE__); @@ -129,37 +116,26 @@ void ClientCommunicator::configureCapabilities() { if (send(mClientFd, cap_packet, cap_packet_size, 0) < 0) { ALOGE("%s(%d): Failed to send camera capabilities, err: %s ", __FUNCTION__, mClientId, strerror(errno)); - goto out; - } - ALOGI("%s(%d): Sent CAPABILITY packet to client", __FUNCTION__, mClientId); - - if ((recv_size = recv(mClientFd, &header, sizeof(camera_header_t), MSG_WAITALL)) < 0) { - ALOGE("%s(%d): Failed to receive header, err: %s ", __FUNCTION__, mClientId, strerror(errno)); - goto out; - } - - if (header.type != CAMERA_INFO) { - ALOGE("%s(%d): invalid camera_packet_type: %s", __FUNCTION__, mClientId, - camera_type_to_str(header.type)); - goto out; - } + } else + ALOGI("%s(%d): Sent CAPABILITY packet to client", __FUNCTION__, mClientId); + free(cap_packet); + return; +} - if (header.size < sizeof(camera_info_t)) { - ALOGE("%s(%d): No camera device to support, header size received, size = %zu", - __FUNCTION__, mClientId, recv_size); - goto send_ack; - } else { - // Get the number of cameras requested to support from client. - mNumOfCamerasRequested = (header.size) / sizeof(camera_info_t); - } +void ClientCommunicator::handleCameraInfo(uint32_t header_size) { + ALOGVV("%s(%d) Enter", __FUNCTION__, mClientId); + Mutex::Autolock al(mMutex); + int camera_id, expctd_cam_id; + struct ValidateClientCapability val_client_cap[MAX_NUMBER_OF_SUPPORTED_CAMERAS]; + ssize_t recv_size = 0; camera_info_t camera_info[mNumOfCamerasRequested]; memset(camera_info, 0, sizeof(camera_info)); if ((recv_size = recv(mClientFd, camera_info, - header.size, MSG_WAITALL)) < 0) { + header_size, MSG_WAITALL)) < 0) { ALOGE("%s(%d): Failed to receive camera info, err: %s ", __FUNCTION__, mClientId, strerror(errno)); - goto out; + return; } ALOGI("%s(%d): Received CAMERA_INFO packet from client with recv_size: %zd ", __FUNCTION__, mClientId, @@ -174,8 +150,7 @@ void ClientCommunicator::configureCapabilities() { __FUNCTION__, mClientId); mNumOfCamerasRequested = MAX_NUMBER_OF_SUPPORTED_CAMERAS; } - - // validate capability info received from the client. + // validate capability info received from the client. for (int i = 0; i < mNumOfCamerasRequested; i++) { expctd_cam_id = i; if (expctd_cam_id == (int)camera_info[i].cameraId) @@ -186,66 +161,27 @@ void ClientCommunicator::configureCapabilities() { "expected Id %d", __FUNCTION__, mClientId, camera_info[i].cameraId, expctd_cam_id); - switch (camera_info[i].codec_type) { - case uint32_t(VideoCodecType::kH264): - case uint32_t(VideoCodecType::kH265): - val_client_cap[i].validCodecType = true; - break; - default: - val_client_cap[i].validCodecType = false; - break; - } - - switch (camera_info[i].resolution) { - case uint32_t(FrameResolution::k480p): - case uint32_t(FrameResolution::k720p): - case uint32_t(FrameResolution::k1080p): - val_client_cap[i].validResolution = true; - break; - default: - val_client_cap[i].validResolution = false; - break; - } - - switch (camera_info[i].sensorOrientation) { - case uint32_t(SensorOrientation::ORIENTATION_0): - case uint32_t(SensorOrientation::ORIENTATION_90): - case uint32_t(SensorOrientation::ORIENTATION_180): - case uint32_t(SensorOrientation::ORIENTATION_270): - val_client_cap[i].validOrientation = true; - break; - default: - val_client_cap[i].validOrientation = false; - break; - } + val_client_cap[i].validCodecType = mCapabilitiesHelper.IsCodecTypeValid(camera_info[i].codec_type); + val_client_cap[i].validResolution = mCapabilitiesHelper.IsResolutionValid(camera_info[i].resolution); + val_client_cap[i].validOrientation = mCapabilitiesHelper.IsSensorOrientationValid(camera_info[i].sensorOrientation); + val_client_cap[i].validCameraFacing = mCapabilitiesHelper.IsCameraFacingValid(camera_info[i].facing); - switch (camera_info[i].facing) { - case uint32_t(CameraFacing::BACK_FACING): - case uint32_t(CameraFacing::FRONT_FACING): - val_client_cap[i].validCameraFacing = true; - break; - default: - val_client_cap[i].validCameraFacing = false; - break; - } } - // Check whether recceived any invalid capability info or not. // ACK packet to client would be updated based on this verification. for (int i = 0; i < mNumOfCamerasRequested; i++) { if (!val_client_cap[i].validCodecType || !val_client_cap[i].validResolution || !val_client_cap[i].validOrientation || !val_client_cap[i].validCameraFacing) { - valid_client_cap_info = false; + mValidClientCapInfo = false; ALOGE("%s(%d): capability info received from client is not completely correct and expected", __FUNCTION__, mClientId); break; } else { ALOGVV("%s(%d): capability info received from client is correct and expected", __FUNCTION__, mClientId); - valid_client_cap_info = true; + mValidClientCapInfo = true; } } - // Updating metadata for each camera seperately with its capability info received. for (int i = 0; i < mNumOfCamerasRequested; i++) { camera_id = i; @@ -297,22 +233,24 @@ void ClientCommunicator::configureCapabilities() { // Wait till complete the metadata update for a camera. { Mutex::Autolock al(sMutex); - // Dynamic client configuration is not supported when camera - // session is active - if ((mCameraSessionState != CameraSessionState::kCameraOpened) - && (mCameraSessionState != CameraSessionState::kDecodingStarted)) { - gVirtualCameraFactory.createVirtualRemoteCamera(mVideoDecoder, mClientId, camera_info[i]); - } + gVirtualCameraFactory.createVirtualRemoteCamera(mVideoDecoder, mClientId, camera_info[i]); } } +} + -send_ack: +void ClientCommunicator::sendAck() { + ALOGVV("%s(%d) Enter", __FUNCTION__, mClientId); + Mutex::Autolock al(mMutex); + size_t ack_packet_size = sizeof(camera_header_t) + sizeof(camera_ack_t); + camera_ack_t ack_payload = ACK_CONFIG; + camera_packet_t *ack_packet = NULL; ack_packet = (camera_packet_t *)malloc(ack_packet_size); if (ack_packet == NULL) { ALOGE("%s(%d): ack camera_packet_t allocation failed: %d ", __FUNCTION__, mClientId, __LINE__); - goto out; + return; } - ack_payload = (valid_client_cap_info) ? ACK_CONFIG : NACK_CONFIG; + ack_payload = (mValidClientCapInfo) ? ACK_CONFIG : NACK_CONFIG; ack_packet->header.type = ACK; ack_packet->header.size = sizeof(camera_ack_t); @@ -321,16 +259,14 @@ void ClientCommunicator::configureCapabilities() { if (send(mClientFd, ack_packet, ack_packet_size, 0) < 0) { ALOGE("%s(%d): Failed to send camera capabilities, err: %s ", __FUNCTION__, mClientId, strerror(errno)); - goto out; + } else { + ALOGI("%s(%d): Sent ACK packet to client with ack_size: %zu ", __FUNCTION__, mClientId, + ack_packet_size); + ALOGI("%s(%d): Capability negotiation and metadata update for %d camera(s) completed successfully..", + __FUNCTION__, mClientId, mNumOfCamerasRequested); + mIsConfigurationDone = true; } - ALOGI("%s(%d): Sent ACK packet to client with ack_size: %zu ", __FUNCTION__, mClientId, - ack_packet_size); - ALOGI("%s(%d): Capability negotiation and metadata update for %d camera(s) completed successfully..", - __FUNCTION__, mClientId, mNumOfCamerasRequested); - mIsConfigurationDone = true; -out: free(ack_packet); - free(cap_packet); ALOGVV("%s(%d): Exit", __FUNCTION__, mClientId); } @@ -399,95 +335,43 @@ bool ClientCommunicator::clientThread() { MSG_WAITALL)) > 0) { ALOGVV("%s(%d): Received Header %zd bytes. Payload size: %u", __FUNCTION__, mClientId, size, header.size); - if (header.type == REQUEST_CAPABILITY && header.size == 0) { - ALOGI("%s(%d): Configure capability", __FUNCTION__, mClientId); - // Dynamic client configuration is not supported when camera - // session is active - if ((mCameraSessionState != CameraSessionState::kCameraOpened) - && (mCameraSessionState != CameraSessionState::kDecodingStarted)) { + switch (header.type) { + case REQUEST_CAPABILITY: + if(header.size != 0) + break; gVirtualCameraFactory.clearCameraInfo(mClientId); - } - configureCapabilities(); - continue; - } - if (!mIsConfigurationDone || header.type != CAMERA_DATA) { - ALOGE("%s(%d): invalid camera_packet_type: %s", __FUNCTION__, mClientId, - camera_type_to_str(header.type)); - continue; - } - - if (header.size > mSocketBuffer.size()) { - // maximum size of a H264 packet in any aggregation packet is 65535 - // bytes. Source: https://tools.ietf.org/html/rfc6184#page-13 - ALOGE( - "%s(%d) Fatal: Unusual encoded packet size detected: %u! Max is %zu, " - "...", - __func__, mClientId, header.size, mSocketBuffer.size()); - continue; - } - - // recv frame - if ((size = recv(mClientFd, mSocketBuffer.data(), header.size, - MSG_WAITALL)) > 0) { - if (size < header.size) { - ALOGW("%s(%d) : Incomplete data read %zd/%u bytes", __func__, mClientId, - size, header.size); - size_t bytes_read = size; - while (bytes_read < header.size) { - if ((size = recv(mClientFd, mSocketBuffer.data() + bytes_read, - header.size - bytes_read, MSG_WAITALL)) > 0) { - bytes_read += size; - ALOGI("%s(%d) : Read-%zd after Incomplete data, remaining-%lu", - __func__, mClientId, size, header.size - bytes_read); - } + sendCameraCapabilities(); + break; + case CAMERA_INFO: + mNumOfCamerasRequested = 0; + if (header.size < sizeof(camera_info_t)) { + ALOGE("%s(%d): No camera device to support, header size received, size = %zu", + __FUNCTION__, mClientId, size); + } else { + mNumOfCamerasRequested = (header.size) / sizeof(camera_info_t); + handleCameraInfo(header.size); } - size = header.size; - } - - ALOGV("%s : Received encoded frame from client", __func__); - mSocketBufferSize = header.size; - - ALOGVV("%s(%d): Camera session state: %s", __func__, mClientId, - kCameraSessionStateNames.at(mCameraSessionState).c_str()); - switch (mCameraSessionState) { - case CameraSessionState::kCameraOpened: - mCameraSessionState = CameraSessionState::kDecodingStarted; - ALOGVV("%s(%d): Decoding started now.", __func__, mClientId); - case CameraSessionState::kDecodingStarted: { - if (mCameraBuffer == NULL) break; - mCameraBuffer->clientRevCount++; - ALOGVV("%s(%d): Received Payload #%d %zd/%u bytes", __func__, mClientId, - mCameraBuffer->clientRevCount, size, header.size); - - mfxStatus ret = MFX_ERR_NONE; - // Start decoding received frames. - ret = mVideoDecoder->DecodeFrame(mSocketBuffer.data(), - mSocketBufferSize); - if (ret == MFX_ERR_NONE) { - ALOGV("%s(%d): Decoding success! Now need to get the output", - __func__, mClientId); - } else { - ALOGE("%s(%d): Decoding failed. ret = %d", __func__, - mClientId, ret); - } - - mSocketBuffer.fill(0); - break; + sendAck(); + break; + case CAMERA_DATA: + if (!mIsConfigurationDone) { + ALOGE("%s(%d): Invalid camera_packet_type: %s, Configuration not completed", __FUNCTION__, mClientId, + camera_type_to_str(header.type)); + } else if (header.size > mSocketBuffer.size()) { + // maximum size of a H264 packet in any aggregation packet is 65535 + // bytes. Source: https://tools.ietf.org/html/rfc6184#page-13 + ALOGE("%s(%d) Fatal: Unusual encoded packet size detected: %u! Max is %zu, " + "...",__func__, mClientId, header.size, mSocketBuffer.size()); + } else { + handleIncomingFrames(header.size); } - case CameraSessionState::kCameraClosed: - ALOGI("%s(%d): Closing and releasing the decoder", __func__, mClientId); - mCameraSessionState = CameraSessionState::kDecodingStopped; - break; - case CameraSessionState::kDecodingStopped: - ALOGVV("%s(%d): Decoder is already released, hence skip the client input", - __func__, mClientId); - mSocketBuffer.fill(0); - break; - default: - ALOGE("%s(%d): Invalid Camera session state!", __func__, mClientId); - break; - } - } + break; + default: + ALOGE("%s(%d): invalid camera_packet_type: %s", __FUNCTION__, mClientId, + camera_type_to_str(header.type)); + break; + } + continue; } } else { ALOGE( @@ -510,4 +394,68 @@ bool ClientCommunicator::clientThread() { return true; } +void ClientCommunicator::handleIncomingFrames(uint32_t header_size) { + ssize_t size = 0; + // recv frame + if ((size = recv(mClientFd, mSocketBuffer.data(), header_size, + MSG_WAITALL)) > 0) { + if (size < header_size) { + ALOGW("%s(%d) : Incomplete data read %zd/%u bytes", __func__, mClientId, + size, header_size); + size_t bytes_read = size; + while (bytes_read < header_size) { + if ((size = recv(mClientFd, mSocketBuffer.data() + bytes_read, + header_size - bytes_read, MSG_WAITALL)) > 0) { + bytes_read += size; + ALOGI("%s(%d) : Read-%zd after Incomplete data, remaining-%lu", + __func__, mClientId, size, header_size - bytes_read); + } + } + size = header_size; + } + + ALOGV("%s : Received encoded frame from client", __func__); + mSocketBufferSize = header_size; + + ALOGVV("%s(%d): Camera session state: %s", __func__, mClientId, + kCameraSessionStateNames.at(mCameraSessionState).c_str()); + switch (mCameraSessionState) { + case CameraSessionState::kCameraOpened: + mCameraSessionState = CameraSessionState::kDecodingStarted; + ALOGVV("%s(%d): Decoding started now.", __func__, mClientId); + case CameraSessionState::kDecodingStarted: { + if (mCameraBuffer == NULL) break; + mCameraBuffer->clientRevCount++; + ALOGVV("%s(%d): Received Payload #%d %zd/%u bytes", __func__, mClientId, + mCameraBuffer->clientRevCount, size, header.size); + + mfxStatus ret = MFX_ERR_NONE; + // Start decoding received frames. + ret = mVideoDecoder->DecodeFrame(mSocketBuffer.data(), mSocketBufferSize); + if (ret == MFX_ERR_NONE) { + ALOGV("%s(%d): Decoding success! Now need to get the output", + __func__, mClientId); + } else { + ALOGE("%s(%d): Decoding failed. ret = %d", __func__, + mClientId, ret); + } + + mSocketBuffer.fill(0); + break; + } + case CameraSessionState::kCameraClosed: + ALOGI("%s(%d): Closing and releasing the decoder", __func__, mClientId); + mCameraSessionState = CameraSessionState::kDecodingStopped; + break; + case CameraSessionState::kDecodingStopped: + ALOGVV("%s(%d): Decoder is already released, hence skip the client input", + __func__, mClientId); + mSocketBuffer.fill(0); + break; + default: + ALOGE("%s(%d): Invalid Camera session state!", __func__, mClientId); + break; + } + } +} } // namespace android diff --git a/src/VirtualCameraFactory.cpp b/src/VirtualCameraFactory.cpp index 11eefc4..185b5f9 100644 --- a/src/VirtualCameraFactory.cpp +++ b/src/VirtualCameraFactory.cpp @@ -125,8 +125,8 @@ VirtualCameraFactory::~VirtualCameraFactory() { for (auto it = mVirtualCameras.begin(); it != mVirtualCameras.end(); it++) { delete it->second; it->second = nullptr; - mVirtualCameras.erase(it); } + mVirtualCameras.clear(); if (mSocketListener) { mSocketListener->requestExit(); From c47f2c2a9e53b30f0098598156faf54f147cedea Mon Sep 17 00:00:00 2001 From: "Singh, Sapna1" Date: Thu, 23 Feb 2023 14:34:33 +0530 Subject: [PATCH 2/5] Add Gtest support to test Camera-vhal API's Introduce a property based on which instance id is taken for socket connection for Gtest purpose. Check for this property in camera-vhal and if this set then use the instance id that is set by property for Gtest else use the default one. Newly introduced property is ro.boot.container.testid. Add Unit TestCases for Camera-vhal in Gtest. Tracked-On: OAM-105831 Signed-off-by: Singh, Sapna1 sapna1.singh@intel.com --- Android.mk | 1 + Gtest/Android.mk | 36 ++++++++++ Gtest/CameraClient.cpp | 123 ++++++++++++++++++++++++++++++++++ Gtest/CameraClient.h | 48 ++++++++++++++ Gtest/CameraFixture.cpp | 124 +++++++++++++++++++++++++++++++++++ Gtest/CameraFixture.h | 89 +++++++++++++++++++++++++ Gtest/main.cpp | 30 +++++++++ src/VirtualCameraFactory.cpp | 6 +- 8 files changed, 456 insertions(+), 1 deletion(-) create mode 100644 Gtest/Android.mk create mode 100644 Gtest/CameraClient.cpp create mode 100644 Gtest/CameraClient.h create mode 100644 Gtest/CameraFixture.cpp create mode 100644 Gtest/CameraFixture.h create mode 100644 Gtest/main.cpp diff --git a/Android.mk b/Android.mk index aa42054..3383326 100644 --- a/Android.mk +++ b/Android.mk @@ -137,3 +137,4 @@ LOCAL_MODULE := camera.$(TARGET_PRODUCT).jpeg include $(BUILD_SHARED_LIBRARY) ###################################################### +include $(LOCAL_PATH)/Gtest/Android.mk diff --git a/Gtest/Android.mk b/Gtest/Android.mk new file mode 100644 index 0000000..2231772 --- /dev/null +++ b/Gtest/Android.mk @@ -0,0 +1,36 @@ +LOCAL_PATH:= $(call my-dir) +include $(CLEAR_VARS) + +LOCAL_LDLIBS += -landroid -llog +LOCAL_CFLAGS += -fexceptions +LOCAL_MODULE_TAGS:= optional + +LOCAL_SRC_FILES := main.cpp CameraFixture.cpp CameraClient.cpp +LOCAL_SHARED_LIBRARIES += liblog libcutils libutils camera.$(TARGET_PRODUCT) libvhal-client +LOCAL_MULTILIB := 64 +LOCAL_VENDOR_MODULE := true +LOCAL_STATIC_LIBRARIES += libgtest_main libgtest libgmock android.hardware.camera.common@1.0-helper android.hardware.graphics.mapper@2.0 + + +LOCAL_CPPFLAGS := $(LOG_FLAGS) $(WARNING_LEVEL) $(DEBUG_FLAGS) $(VERSION_FLAGS) + +LOCAL_C_INCLUDES += \ + $(LOCAL_PATH) \ + $(LOCAL_PATH)/../../libvhal-client \ + $(LOCAL_PATH)/../include \ + external/libjpeg-turbo \ + external/libexif \ + external/libyuv/files/include \ + frameworks/native/include/media/hardware \ + hardware/intel/libva \ + hardware/intel/onevpl/api/vpl \ + hardware/libhardware/modules/gralloc \ + $(LOCAL_PATH)/include \ + $(call include-path-for, camera) \ + external/gtest/include \ + external/gtest \ + bionic + +LOCAL_MODULE:= CameraFixtureTest + +include $(BUILD_EXECUTABLE) diff --git a/Gtest/CameraClient.cpp b/Gtest/CameraClient.cpp new file mode 100644 index 0000000..44b1ef0 --- /dev/null +++ b/Gtest/CameraClient.cpp @@ -0,0 +1,123 @@ +/* + * Copyright (C) 2023 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#define LOG_TAG "CameraClient" +#include "CameraClient.h" +#include + +using namespace std::chrono_literals; +using namespace vhal::client; +using namespace std; +int CameraClient::startDummyStreamer() +{ + is_running = true; + string socket_path("/ipc"); + UnixConnectionInfo conn_info = { socket_path, instance_id }; + try { + video_sink = make_shared(conn_info, + [&](const VideoSink::camera_config_cmd_t& ctrl_msg) { + switch (ctrl_msg.cmd) { + case VideoSink::camera_cmd_t::CMD_OPEN: { + ALOGI("%s: Received Open command from Camera VHal", __FUNCTION__); + break; + } + case VideoSink::camera_cmd_t::CMD_CLOSE: + ALOGI("%s: Received Close command from Camera VHal", __FUNCTION__); + exit(0); + default: + ALOGI("%s: Unknown Command received, exiting with failure", __FUNCTION__); + exit(1); + } + }); + + } catch (const std::exception& ex) { + ALOGI("%s: VideoSink creation error : %s", __FUNCTION__,ex.what()); + exit(1); + } + + ALOGI("%s: Waiting Camera Open callback..", __FUNCTION__); + + while (!video_sink->IsConnected()) + this_thread::sleep_for(100ms); + + // we need to be alive :) + while (is_running) { + this_thread::sleep_for(100ms); + } + return 0; +} + +bool CameraClient::IsConnected() { + return video_sink->IsConnected(); +} + +void CameraClient::RequestCameraCapability() { + if(IsConnected()) { + ALOGI("%s: Calling GetCameraCapabilty..", __FUNCTION__); + video_sink->GetCameraCapabilty(); + } +} + +void CameraClient::sendOneCameraConfig() { + int noOfCameras = 1; + std::vector camera_info(noOfCameras); + for (int i = 0; i < noOfCameras; i++) { + camera_info[i].cameraId = i; + camera_info[i].codec_type = VideoSink::VideoCodecType::kH264; + camera_info[i].resolution = VideoSink::FrameResolution::k1080p; + camera_info[i].sensorOrientation = VideoSink::SensorOrientation::ORIENTATION_0; + camera_info[i].facing = VideoSink::CameraFacing::BACK_FACING; + } + + ALOGI("%s: Calling SetCameraCapabilty..", __FUNCTION__); + video_sink->SetCameraCapabilty(camera_info); +} + +void CameraClient::sendTwoCameraConfig() { + int noOfCameras = 2; + std::vector camera_info(noOfCameras); + for (int i = 0; i < noOfCameras; i++) { + camera_info[i].cameraId = i; + camera_info[i].codec_type = VideoSink::VideoCodecType::kH264; + camera_info[i].resolution = (i==0) ? VideoSink::FrameResolution::k1080p : VideoSink::FrameResolution::k720p; + camera_info[i].sensorOrientation = VideoSink::SensorOrientation::ORIENTATION_0; + camera_info[i].facing = (i == 0) ? VideoSink::CameraFacing::BACK_FACING : VideoSink::CameraFacing::FRONT_FACING; + } + + ALOGI("%s: Calling SetCameraCapabilty..", __FUNCTION__); + video_sink->SetCameraCapabilty(camera_info); +} + +void CameraClient::sendMultipleCameraConfig() { + int noOfCameras = 4; + std::vector camera_info(noOfCameras); + for (int i = 0; i < noOfCameras; i++) { + camera_info[i].cameraId = i; + camera_info[i].codec_type = VideoSink::VideoCodecType::kH264; + camera_info[i].resolution = (i==0) ? VideoSink::FrameResolution::k1080p : VideoSink::FrameResolution::k720p; + camera_info[i].sensorOrientation = VideoSink::SensorOrientation::ORIENTATION_0; + camera_info[i].facing = (i == 0) ? VideoSink::CameraFacing::BACK_FACING : VideoSink::CameraFacing::FRONT_FACING; + } + + ALOGI("%s: Calling SetCameraCapabilty..", __FUNCTION__); + video_sink->SetCameraCapabilty(camera_info); +} + +void CameraClient::stopDummyStreamer() { + is_running = false; +} diff --git a/Gtest/CameraClient.h b/Gtest/CameraClient.h new file mode 100644 index 0000000..52ccb5d --- /dev/null +++ b/Gtest/CameraClient.h @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2023 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include "video_sink.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace vhal::client; +using namespace std; + +class CameraClient { +private: + bool is_running; +public : + int startDummyStreamer(); + void stopDummyStreamer(); + void RequestCameraCapability(); + void sendTwoCameraConfig(); + void sendOneCameraConfig(); + bool IsConnected(); + void sendMultipleCameraConfig(); + + int instance_id = 10000; + shared_ptr video_sink; +}; diff --git a/Gtest/CameraFixture.cpp b/Gtest/CameraFixture.cpp new file mode 100644 index 0000000..b12fcaa --- /dev/null +++ b/Gtest/CameraFixture.cpp @@ -0,0 +1,124 @@ +/* + * Copyright (C) 2023 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include +#include +#include "CameraFixture.h" +#include "CameraSocketCommand.h" + +using namespace std; +using namespace android; +using namespace std::chrono_literals; + +void CameraFixture::runStreamer(){ + mCameraClient.startDummyStreamer(); +} + +CameraFixture::CameraFixture() +{ + SetCallback(); +} + +void CameraFixture::SetUp() +{ + t1 = new thread(&CameraFixture::runStreamer, this); + this_thread::sleep_for(1500ms); +} + +void CameraFixture::TearDown() +{ + mCameraClient.stopDummyStreamer(); + t1->join(); + delete t1; + t1 = nullptr; +} + +CameraFixture::~CameraFixture() +{ + DestroyCallbacks(); +} + +TEST_F(CameraFixture, ClientWithOneCamera) +{ + ASSERT_EQ(NO_CAMERA_PRESENT, gVirtualCameraFactory.get_number_of_cameras()); + mCameraClient.RequestCameraCapability(); + mCameraClient.sendOneCameraConfig(); + this_thread::sleep_for(500ms); + ASSERT_EQ(ONE_CAMERA_CLIENT, gVirtualCameraFactory.get_number_of_cameras()); + gVirtualCameraFactory.clearCameraInfo(client_id); +} + +TEST_F(CameraFixture, ClientWithTwoCamera) +{ + mCameraClient.RequestCameraCapability(); + mCameraClient.sendTwoCameraConfig(); + this_thread::sleep_for(500ms); + ASSERT_EQ(TWO_CAMERA_CLIENT, gVirtualCameraFactory.get_number_of_cameras()); + gVirtualCameraFactory.clearCameraInfo(client_id); +} + +TEST_F(CameraFixture, ClientWithMultiCamera) +{ + mCameraClient.RequestCameraCapability(); + mCameraClient.sendMultipleCameraConfig(); + this_thread::sleep_for(500ms); + ASSERT_EQ(MULTI_CAMERA_CLIENT, gVirtualCameraFactory.get_number_of_cameras()); + gVirtualCameraFactory.clearCameraInfo(client_id); +} + +TEST_F(CameraFixture, CheckForCodecType) +{ + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsCodecTypeValid((uint32_t)android::socket::VideoCodecType::kH264)); + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsCodecTypeValid((uint32_t)android::socket::VideoCodecType::kH265)); + ASSERT_EQ(INVALID_TYPE, mCapabilitiesHelper.IsCodecTypeValid((uint32_t)INVALID_WAV)); +} + +TEST_F(CameraFixture, CheckForFacing) +{ + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsCameraFacingValid((uint32_t)android::socket::CameraFacing::BACK_FACING)); + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsCameraFacingValid((uint32_t)android::socket::CameraFacing::FRONT_FACING)); + ASSERT_EQ(INVALID_TYPE, mCapabilitiesHelper.IsCameraFacingValid((uint32_t)FRONT_FACING_SECOND)); +} + +TEST_F(CameraFixture, CheckForOrientation) +{ + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsSensorOrientationValid((uint32_t)android::socket::SensorOrientation::ORIENTATION_90)); + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsSensorOrientationValid((uint32_t)android::socket::SensorOrientation::ORIENTATION_270)); + ASSERT_EQ(INVALID_TYPE, mCapabilitiesHelper.IsSensorOrientationValid((uint32_t)INVALID_ORIENTATION_360)); +} + +TEST_F(CameraFixture, CheckForResolution) +{ + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsResolutionValid((uint32_t)android::socket::FrameResolution::k720p)); + ASSERT_EQ(VALID_TYPE, mCapabilitiesHelper.IsResolutionValid((uint32_t)android::socket::FrameResolution::k1080p)); + ASSERT_EQ(INVALID_TYPE, mCapabilitiesHelper.IsResolutionValid((uint32_t)INVALID_k2160p)); +} + +TEST_F(CameraFixture, MultipleCameraCapReq) +{ + mCameraClient.RequestCameraCapability(); + mCameraClient.RequestCameraCapability(); + mCameraClient.RequestCameraCapability(); + mCameraClient.sendMultipleCameraConfig(); + this_thread::sleep_for(500ms); + ASSERT_EQ(MULTI_CAMERA_CLIENT, gVirtualCameraFactory.get_number_of_cameras()); + gVirtualCameraFactory.clearCameraInfo(client_id); +} + diff --git a/Gtest/CameraFixture.h b/Gtest/CameraFixture.h new file mode 100644 index 0000000..67ede42 --- /dev/null +++ b/Gtest/CameraFixture.h @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2023 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#define ONE_CAMERA_CLIENT 1 +#define TWO_CAMERA_CLIENT 2 +#define MULTI_CAMERA_CLIENT 2 +#define NO_CAMERA_PRESENT 0 +#define VALID_TYPE true +#define INVALID_TYPE false +#define INVALID_WAV 5 +#define INVALID_ORIENTATION_360 360 +#define INVALID_k2160p 5 +#define FRONT_FACING_SECOND 2 + +#include +#include +#include +#include "gtest/gtest.h" +#include "CameraClient.h" +#include "CapabilitiesHelper.h" +#include "VirtualCameraFactory.h" + +using namespace android; +class CameraFixture : public ::testing::Test +{ +public: + int client_id = 0; + /*! Creates an instance of CameraFixture */ + CameraFixture(); + + /*! Destroys an instance of CameraFixture */ + ~CameraFixture(); + + virtual void SetUp(); + + virtual void TearDown(); + + std::thread* t1; + CameraClient mCameraClient; + CapabilitiesHelper mCapabilitiesHelper; + +private: + /*! Disabled Copy Constructor */ + CameraFixture(const CameraFixture&); + + /*! Disabled Assignment operator */ + CameraFixture& operator=(const CameraFixture&); + + void runStreamer(); + +protected: + camera_module_callbacks_t* callbacks; + static void test_camera_device_status_change( + const struct camera_module_callbacks*, int /*camera_id*/, + int /*new_status*/) {} + + static void test_torch_mode_status_change( + const struct camera_module_callbacks*, const char* /*camera_id*/, + int /*new_status*/) {} + + void SetCallback(){ + callbacks = + (camera_module_callbacks_t*)malloc(sizeof(camera_module_callbacks_t)); + callbacks->camera_device_status_change = test_camera_device_status_change; + callbacks->torch_mode_status_change = test_torch_mode_status_change; + gVirtualCameraFactory.set_callbacks(callbacks); + } + void DestroyCallbacks() { + delete callbacks; + callbacks = nullptr; + } + +}; + diff --git a/Gtest/main.cpp b/Gtest/main.cpp new file mode 100644 index 0000000..101f36e --- /dev/null +++ b/Gtest/main.cpp @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2023 Intel Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "CameraFixture.h" + +using namespace std; + +GTEST_API_ int main(int argc, char **argv) +{ + setenv("TERM", "xterm-color", true); + testing::InitGoogleTest(&argc, argv); + + return RUN_ALL_TESTS(); +} + diff --git a/src/VirtualCameraFactory.cpp b/src/VirtualCameraFactory.cpp index 185b5f9..0be9521 100644 --- a/src/VirtualCameraFactory.cpp +++ b/src/VirtualCameraFactory.cpp @@ -110,7 +110,11 @@ bool VirtualCameraFactory::createSocketListener() { ALOGV("%s: E", __FUNCTION__); char id[PROPERTY_VALUE_MAX] = {0}; - if (property_get("ro.boot.container.id", id, "") > 0) { + // This property is used by gtest, set this before running gtest + if (property_get("ro.boot.container.testid", id, "") > 0) { + mSocketListener = std::make_shared(id); + mSocketListener->run("ConnectionsListener"); + } else if (property_get("ro.boot.container.id", id, "") > 0) { mSocketListener = std::make_shared(id); mSocketListener->run("ConnectionsListener"); } else From 71a970ba023bdcf9e688d9895b125ed6aecb1a7b Mon Sep 17 00:00:00 2001 From: "Singh, Sapna1" Date: Fri, 10 Mar 2023 15:06:06 +0530 Subject: [PATCH 3/5] Fix crash seen in deleting VirtualCameraFactory object ConnectionsListener class was derived from Thread class which was causing issue while calling join for this thread from VirtualCameraFactory destructor. The Base class from which ConnectionsListener was derived was not the proper class. With current implementation, instead of deriving from thread class, a thread is introduced as a member of the class to do the same job and it is started during creation of ConnectionsListener object Tracked-On: OAM-106781 Signed-off-by: Singh, Sapna1 --- include/ConnectionsListener.h | 13 ++-- src/ConnectionsListener.cpp | 124 ++++++++++++++++++---------------- src/VirtualCameraFactory.cpp | 5 +- 3 files changed, 73 insertions(+), 69 deletions(-) diff --git a/include/ConnectionsListener.h b/include/ConnectionsListener.h index b329d1f..518940e 100644 --- a/include/ConnectionsListener.h +++ b/include/ConnectionsListener.h @@ -21,25 +21,23 @@ #define CONNECTIONS_LISTENER_H #include -#include +#include #include #include #define MAX_CONCURRENT_USER_NUM 8 - namespace android { -class ConnectionsListener : public Thread { +class ConnectionsListener { public: ConnectionsListener(std::string suffix); - virtual void requestExit(); - virtual status_t requestExitAndWait(); + void requestJoin(); + void requestExit(); int getClientFd(int clientId); void clearClientFd(int clientId); private: - virtual status_t readyToRun(); - virtual bool threadLoop() override; + bool socketListenerThreadProc(); Mutex mMutex; bool mRunning; int mSocketServerFd = -1; @@ -47,6 +45,7 @@ class ConnectionsListener : public Thread { uint32_t mNumConcurrentUsers = 0; std::vector> mClientFdPromises; std::vector mClientsConnected; + std::unique_ptr mSocketListenerThread; }; } // namespace android diff --git a/src/ConnectionsListener.cpp b/src/ConnectionsListener.cpp index 3221d03..50dd727 100644 --- a/src/ConnectionsListener.cpp +++ b/src/ConnectionsListener.cpp @@ -22,7 +22,11 @@ #include #include +#include +#include +#include #include +#include #include #include #include "CameraSocketCommand.h" @@ -30,8 +34,7 @@ namespace android { ConnectionsListener::ConnectionsListener(std::string suffix) - : Thread(/*canCallJava*/ false), - mRunning{true}, + : mRunning{true}, mSocketServerFd{-1} { std::string sock_path = "/ipc/camera-socket" + suffix; char *k8s_env_value = getenv("K8S_ENV"); @@ -56,11 +59,12 @@ ConnectionsListener::ConnectionsListener(std::string suffix) } mClientFdPromises.resize(num_clients); mClientsConnected.resize(num_clients, false); + mSocketListenerThread = std::unique_ptr(new std::thread(&ConnectionsListener::socketListenerThreadProc,this)); } -status_t ConnectionsListener::requestExitAndWait() { - ALOGE("%s: Not implemented. Use requestExit + join instead", __FUNCTION__); - return INVALID_OPERATION; +void ConnectionsListener::requestJoin () { + if(mSocketListenerThread->joinable()) + mSocketListenerThread->join(); } int ConnectionsListener::getClientFd(int clientId) { @@ -74,20 +78,10 @@ void ConnectionsListener::clearClientFd(int clientId) { void ConnectionsListener::requestExit() { Mutex::Autolock al(mMutex); - - ALOGV("%s: Requesting thread exit", __FUNCTION__); mRunning = false; - ALOGV("%s: Request exit complete.", __FUNCTION__); -} - -status_t ConnectionsListener::readyToRun() { - Mutex::Autolock al(mMutex); - - return OK; } - -bool ConnectionsListener::threadLoop() { +bool ConnectionsListener::socketListenerThreadProc() { mSocketServerFd = ::socket(AF_UNIX, SOCK_STREAM, 0); if (mSocketServerFd < 0) { ALOGE("%s:%d Fail to construct camera socket with error: %s", __FUNCTION__, __LINE__, @@ -95,6 +89,7 @@ bool ConnectionsListener::threadLoop() { return false; } + struct pollfd fd; struct sockaddr_un addr_un; memset(&addr_un, 0, sizeof(addr_un)); addr_un.sun_family = AF_UNIX; @@ -135,58 +130,69 @@ bool ConnectionsListener::threadLoop() { ALOGE("%s Failed to listen on %s", __FUNCTION__, mSocketPath.c_str()); return false; } + fd.fd = mSocketServerFd; + fd.events = POLLIN; while (mRunning) { - ALOGI(" %s: Wait for camera client to connect. . .", __FUNCTION__); - - socklen_t alen = sizeof(struct sockaddr_un); - - int new_client_fd = ::accept(mSocketServerFd, (struct sockaddr *)&addr_un, &alen); - ALOGI(" %s: Accepted client: [%d]", __FUNCTION__, new_client_fd); - if (new_client_fd < 0) { - ALOGE(" %s: Fail to accept client. Error: [%s]", __FUNCTION__, strerror(errno)); + uint32_t client_id = 0; + if (!mClientsConnected[client_id]) + ALOGI(" %s: Wait for camera client to connect. . .", __FUNCTION__); + ret = poll(&fd, 1, 3000); + if (ret <= 0) + { + ALOGV("%s: poll() failed or poll timeout", __FUNCTION__); continue; } - uint32_t client_id = 0; - if (mNumConcurrentUsers > 0) { - size_t packet_size = sizeof(android::socket::camera_header_t) + sizeof(client_id); - bool status = true; - android::socket::camera_packet_t * user_id_packet = (android::socket::camera_packet_t *)malloc(packet_size); - if (user_id_packet == NULL) { - ALOGE("%s: user_id_packet allocation failed: %d ", __FUNCTION__, __LINE__); + else if (fd.revents & POLLIN) { + socklen_t alen = sizeof(struct sockaddr_un); + + int new_client_fd = ::accept(mSocketServerFd, (struct sockaddr *)&addr_un, &alen); + if (new_client_fd < 0) { + ALOGE(" %s: Fail to accept client. Error: [%s]", __FUNCTION__, strerror(errno)); continue; + } else { + ALOGI(" %s: Accepted client: [%d]", __FUNCTION__, new_client_fd); } - if (recv(new_client_fd, (char *)user_id_packet, packet_size, MSG_WAITALL) < 0) { - ALOGE("%s: Failed to receive user_id header, err: %s ", __FUNCTION__, strerror(errno)); - status = false; - } - if (user_id_packet->header.type != android::socket::CAMERA_USER_ID) { - ALOGE("%s: Invalid packet type %d\n", __FUNCTION__, user_id_packet->header.type); - status = false; - } - if (user_id_packet->header.size != sizeof(client_id)) { - ALOGE("%s: Invalid packet size %u\n", __FUNCTION__, user_id_packet->header.size); - status = false; - } - if (!status) { + if (mNumConcurrentUsers > 0) { + size_t packet_size = sizeof(android::socket::camera_header_t) + sizeof(client_id); + bool status = true; + android::socket::camera_packet_t * user_id_packet = (android::socket::camera_packet_t *)malloc(packet_size); + if (user_id_packet == NULL) { + ALOGE("%s: user_id_packet allocation failed: %d ", __FUNCTION__, __LINE__); + continue; + } + if (recv(new_client_fd, (char *)user_id_packet, packet_size, MSG_WAITALL) < 0) { + ALOGE("%s: Failed to receive user_id header, err: %s ", __FUNCTION__, strerror(errno)); + status = false; + } + if (user_id_packet->header.type != android::socket::CAMERA_USER_ID) { + ALOGE("%s: Invalid packet type %d\n", __FUNCTION__, user_id_packet->header.type); + status = false; + } + if (user_id_packet->header.size != sizeof(client_id)) { + ALOGE("%s: Invalid packet size %u\n", __FUNCTION__, user_id_packet->header.size); + status = false; + } + if (!status) { + free(user_id_packet); + continue; + } + memcpy(&client_id, user_id_packet->payload, sizeof(client_id)); free(user_id_packet); - continue; + if (client_id < 0 || client_id >= mNumConcurrentUsers) { + ALOGE("%s: client_id = %u is not valid", __FUNCTION__, client_id); + continue; + } } - memcpy(&client_id, user_id_packet->payload, sizeof(client_id)); - free(user_id_packet); - if (client_id < 0 || client_id >= mNumConcurrentUsers) { - ALOGE("%s: client_id = %u is not valid", __FUNCTION__, client_id); - continue; + if (mClientsConnected[client_id]) { + ALOGE(" %s: IGNORING clientFd[%d] for already connected Client[%d]", __FUNCTION__, new_client_fd, client_id); + } else { + mClientFdPromises[client_id].set_value(new_client_fd); + ALOGI(" %s: Assigned clientFd[%d] to Client[%d]", __FUNCTION__, new_client_fd, client_id); + mClientsConnected[client_id] = true; } - } - if (mClientsConnected[client_id]) { - ALOGE(" %s: IGNORING clientFd[%d] for already connected Client[%d]", __FUNCTION__, new_client_fd, client_id); - } else { - mClientFdPromises[client_id].set_value(new_client_fd); - ALOGI(" %s: Assigned clientFd[%d] to Client[%d]", __FUNCTION__, new_client_fd, client_id); - mClientsConnected[client_id] = true; - } - } + } // end of poll function + } // end of while close(mSocketServerFd); mSocketServerFd = -1; return true; diff --git a/src/VirtualCameraFactory.cpp b/src/VirtualCameraFactory.cpp index 0be9521..29ef599 100644 --- a/src/VirtualCameraFactory.cpp +++ b/src/VirtualCameraFactory.cpp @@ -113,10 +113,8 @@ bool VirtualCameraFactory::createSocketListener() { // This property is used by gtest, set this before running gtest if (property_get("ro.boot.container.testid", id, "") > 0) { mSocketListener = std::make_shared(id); - mSocketListener->run("ConnectionsListener"); } else if (property_get("ro.boot.container.id", id, "") > 0) { mSocketListener = std::make_shared(id); - mSocketListener->run("ConnectionsListener"); } else ALOGE("%s: FATAL: container id is not set!!", __func__); @@ -134,8 +132,9 @@ VirtualCameraFactory::~VirtualCameraFactory() { if (mSocketListener) { mSocketListener->requestExit(); - mSocketListener->join(); + mSocketListener->requestJoin(); } + mClientThreads.clear(); } /****************************************************************************** From c670149e9fb00ac75f752fde694845798c63e5f4 Mon Sep 17 00:00:00 2001 From: "Singh, Sapna1" Date: Fri, 10 Mar 2023 16:26:36 +0530 Subject: [PATCH 4/5] Fix the clean up issue observed while running gtest for camera-vhal close the client thread after completing the execution of all testcases. An API is introduced to do cleanup and in that API, client thread exit is requested, socket server thread exit is requested and joins with the main thread after all the cleanup. Tracked-On: OAM-106783 Signed-off-by: Singh, Sapna1 --- Gtest/CameraFixture.cpp | 2 ++ include/ClientCommunicator.h | 1 + include/VirtualCameraFactory.h | 3 ++- src/ClientCommunicator.cpp | 9 ++++++--- src/VirtualCameraFactory.cpp | 15 +++++++++++++++ 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/Gtest/CameraFixture.cpp b/Gtest/CameraFixture.cpp index b12fcaa..2756eee 100644 --- a/Gtest/CameraFixture.cpp +++ b/Gtest/CameraFixture.cpp @@ -120,5 +120,7 @@ TEST_F(CameraFixture, MultipleCameraCapReq) this_thread::sleep_for(500ms); ASSERT_EQ(MULTI_CAMERA_CLIENT, gVirtualCameraFactory.get_number_of_cameras()); gVirtualCameraFactory.clearCameraInfo(client_id); + this_thread::sleep_for(200ms); + gVirtualCameraFactory.destroy(client_id); } diff --git a/include/ClientCommunicator.h b/include/ClientCommunicator.h index d43b004..1ccc7d8 100644 --- a/include/ClientCommunicator.h +++ b/include/ClientCommunicator.h @@ -49,6 +49,7 @@ class ClientCommunicator { ~ClientCommunicator(); int getClientId(); + void requestExit(); status_t sendCommandToClient(socket::camera_packet_t *config_cmd_packet, size_t config_cmd_packet_size); std::atomic mCameraSessionState; diff --git a/include/VirtualCameraFactory.h b/include/VirtualCameraFactory.h index 4661078..c35f966 100644 --- a/include/VirtualCameraFactory.h +++ b/include/VirtualCameraFactory.h @@ -177,7 +177,8 @@ class VirtualCameraFactory { bool createVirtualRemoteCamera(std::shared_ptr decoder, int clientId, android::socket::camera_info_t cameraInfo); - + // Does proper cleanup and exit + void destroy(int clientId); private: /**************************************************************************** * Private API diff --git a/src/ClientCommunicator.cpp b/src/ClientCommunicator.cpp index e61bbf6..e0c88f5 100644 --- a/src/ClientCommunicator.cpp +++ b/src/ClientCommunicator.cpp @@ -67,7 +67,6 @@ ClientCommunicator::ClientCommunicator(std::shared_ptr list } ClientCommunicator::~ClientCommunicator() { - mRunning = false; if (mThread.valid()) { mThread.wait(); } Mutex::Autolock al(mMutex); if (mClientFd > 0) { @@ -77,6 +76,10 @@ ClientCommunicator::~ClientCommunicator() { } } +void ClientCommunicator::requestExit() { + mRunning = false; +} + int ClientCommunicator::getClientId() { return mClientId; } @@ -273,7 +276,7 @@ void ClientCommunicator::sendAck() { bool ClientCommunicator::threadLooper() { while (mRunning) { if (!clientThread()) { - ALOGI("%s(%d) : clientThread returned flase, Exit", __FUNCTION__, mClientId); + ALOGI("%s(%d) : clientThread returned false, Exit", __FUNCTION__, mClientId); return false; } else { ALOGI("%s(%d) : Re-spawn clientThread", __FUNCTION__, mClientId); @@ -391,7 +394,7 @@ bool ClientCommunicator::clientThread() { close(mClientFd); mClientFd = -1; ALOGVV("%s(%d): Exit", __FUNCTION__, mClientId); - return true; + return mRunning; } void ClientCommunicator::handleIncomingFrames(uint32_t header_size) { diff --git a/src/VirtualCameraFactory.cpp b/src/VirtualCameraFactory.cpp index 29ef599..2b4c425 100644 --- a/src/VirtualCameraFactory.cpp +++ b/src/VirtualCameraFactory.cpp @@ -137,6 +137,21 @@ VirtualCameraFactory::~VirtualCameraFactory() { mClientThreads.clear(); } +void VirtualCameraFactory::destroy(int clientId) { + ALOGV("%s: Enter", __FUNCTION__); + for (auto it = mVirtualCameras.begin(); it != mVirtualCameras.end(); it++) { + delete it->second; + it->second = nullptr; + } + mVirtualCameras.clear(); + mClientThreads[clientId]->requestExit(); + if (mSocketListener) { + mSocketListener->requestExit(); + mSocketListener->requestJoin(); + } + mClientThreads.clear(); +} + /****************************************************************************** * Camera HAL API handlers. * From 56be12d7bb1ee5f5320581310fcb7051464531cd Mon Sep 17 00:00:00 2001 From: Sapna1-singh <109268418+Sapna1-singh@users.noreply.github.com> Date: Fri, 10 Mar 2023 17:34:23 +0530 Subject: [PATCH 5/5] Update ClientCommunicator.cpp --- src/ClientCommunicator.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ClientCommunicator.cpp b/src/ClientCommunicator.cpp index a27373a..387b4da 100644 --- a/src/ClientCommunicator.cpp +++ b/src/ClientCommunicator.cpp @@ -99,7 +99,7 @@ status_t ClientCommunicator::sendCommandToClient(camera_packet_t *config_cmd_pac void ClientCommunicator::sendCameraCapabilities() { ALOGVV("%s(%d) Enter", __FUNCTION__, mClientId); - mMutex.lock(); + Mutex::Autolock al(mMutex); size_t cap_packet_size = sizeof(camera_header_t) + sizeof(camera_capability_t); camera_capability_t capability = {}; camera_packet_t *cap_packet = NULL; @@ -127,7 +127,7 @@ void ClientCommunicator::sendCameraCapabilities() { void ClientCommunicator::handleCameraInfo(uint32_t header_size) { ALOGVV("%s(%d) Enter", __FUNCTION__, mClientId); - mMutex.lock(); + Mutex::Autolock al(mMutex); int camera_id, expctd_cam_id; struct ValidateClientCapability val_client_cap[MAX_NUMBER_OF_SUPPORTED_CAMERAS]; ssize_t recv_size = 0;