Move dvr config data from display manager to display service
* The limitation of creating only one display manager client
is preventing config data access on multiple threads
Bug: 38269706
Test: DvrDisplayManagerTest::ConfigurationData
Change-Id: Ifc14cf0dd79248aea6367aca70c4e4f8359a25de
diff --git a/libs/vr/libdisplay/display_client.cpp b/libs/vr/libdisplay/display_client.cpp
index 5a0c3d0..6175da7 100644
--- a/libs/vr/libdisplay/display_client.cpp
+++ b/libs/vr/libdisplay/display_client.cpp
@@ -167,6 +167,19 @@
return InvokeRemoteMethod<DisplayProtocol::GetMetrics>();
}
+Status<std::string> DisplayClient::GetConfigurationData(
+ ConfigFileType config_type) {
+ auto status =
+ InvokeRemoteMethod<DisplayProtocol::GetConfigurationData>(config_type);
+ if (!status && status.error() != ENOENT) {
+ ALOGE(
+ "DisplayClient::GetConfigurationData: Unable to get"
+ "configuration data. Error: %s",
+ status.GetErrorMessage().c_str());
+ }
+ return status;
+}
+
Status<std::unique_ptr<Surface>> DisplayClient::CreateSurface(
const SurfaceAttributes& attributes) {
int error;
diff --git a/libs/vr/libdisplay/display_manager_client.cpp b/libs/vr/libdisplay/display_manager_client.cpp
index 37d6ff9..098b725 100644
--- a/libs/vr/libdisplay/display_manager_client.cpp
+++ b/libs/vr/libdisplay/display_manager_client.cpp
@@ -70,20 +70,6 @@
return status;
}
-pdx::Status<std::string> DisplayManagerClient::GetConfigurationData(
- ConfigFileType config_type) {
- auto status =
- InvokeRemoteMethod<DisplayManagerProtocol::GetConfigurationData>(
- config_type);
- if (!status && status.error() != ENOENT) {
- ALOGE(
- "DisplayManagerClient::GetConfigurationData: Unable to get "
- "configuration data. Error: %s",
- status.GetErrorMessage().c_str());
- }
- return status;
-}
-
pdx::Status<std::unique_ptr<ConsumerQueue>>
DisplayManagerClient::GetSurfaceQueue(int surface_id, int queue_id) {
auto status = InvokeRemoteMethod<DisplayManagerProtocol::GetSurfaceQueue>(
diff --git a/libs/vr/libdisplay/include/private/dvr/display_client.h b/libs/vr/libdisplay/include/private/dvr/display_client.h
index e5b3340..ce0bc80 100644
--- a/libs/vr/libdisplay/include/private/dvr/display_client.h
+++ b/libs/vr/libdisplay/include/private/dvr/display_client.h
@@ -68,6 +68,7 @@
class DisplayClient : public pdx::ClientBase<DisplayClient> {
public:
pdx::Status<Metrics> GetDisplayMetrics();
+ pdx::Status<std::string> GetConfigurationData(ConfigFileType config_type);
pdx::Status<std::unique_ptr<IonBuffer>> GetGlobalBuffer(
DvrGlobalBufferKey key);
pdx::Status<std::unique_ptr<Surface>> CreateSurface(
diff --git a/libs/vr/libdisplay/include/private/dvr/display_manager_client.h b/libs/vr/libdisplay/include/private/dvr/display_manager_client.h
index c5ec231..7281b76 100644
--- a/libs/vr/libdisplay/include/private/dvr/display_manager_client.h
+++ b/libs/vr/libdisplay/include/private/dvr/display_manager_client.h
@@ -27,7 +27,6 @@
pdx::Status<std::unique_ptr<ConsumerQueue>> GetSurfaceQueue(int surface_id,
int queue_id);
- pdx::Status<std::string> GetConfigurationData(ConfigFileType config_type);
using Client::event_fd;
pdx::Status<int> GetEventMask(int events) {
diff --git a/libs/vr/libdisplay/include/private/dvr/display_protocol.h b/libs/vr/libdisplay/include/private/dvr/display_protocol.h
index b857d48..d1787e6 100644
--- a/libs/vr/libdisplay/include/private/dvr/display_protocol.h
+++ b/libs/vr/libdisplay/include/private/dvr/display_protocol.h
@@ -186,6 +186,12 @@
PDX_SERIALIZABLE_MEMBERS(SurfaceInfo, surface_id, visible, z_order);
};
+enum class ConfigFileType : uint32_t {
+ kLensMetrics,
+ kDeviceMetrics,
+ kDeviceConfiguration
+};
+
struct DisplayProtocol {
// Service path.
static constexpr char kClientPath[] = "system/vr/display/client";
@@ -193,6 +199,7 @@
// Op codes.
enum {
kOpGetMetrics = 0,
+ kOpGetConfigurationData,
kOpGetGlobalBuffer,
kOpIsVrAppRunning,
kOpCreateSurface,
@@ -207,6 +214,8 @@
// Methods.
PDX_REMOTE_METHOD(GetMetrics, kOpGetMetrics, Metrics(Void));
+ PDX_REMOTE_METHOD(GetConfigurationData, kOpGetConfigurationData,
+ std::string(ConfigFileType config_type));
PDX_REMOTE_METHOD(GetGlobalBuffer, kOpGetGlobalBuffer,
LocalNativeBufferHandle(DvrGlobalBufferKey key));
PDX_REMOTE_METHOD(IsVrAppRunning, kOpIsVrAppRunning, bool(Void));
@@ -219,12 +228,6 @@
void(const SurfaceAttributes& attributes));
};
-enum class ConfigFileType : uint32_t {
- kLensMetrics,
- kDeviceMetrics,
- kDeviceConfiguration
-};
-
struct DisplayManagerProtocol {
// Service path.
static constexpr char kClientPath[] = "system/vr/display/manager";
@@ -234,7 +237,6 @@
kOpGetSurfaceState = 0,
kOpGetSurfaceQueue,
kOpSetupGlobalBuffer,
- kOpGetConfigurationData,
kOpDeleteGlobalBuffer,
};
@@ -250,8 +252,6 @@
PDX_REMOTE_METHOD(SetupGlobalBuffer, kOpSetupGlobalBuffer,
LocalNativeBufferHandle(DvrGlobalBufferKey key, size_t size,
uint64_t usage));
- PDX_REMOTE_METHOD(GetConfigurationData, kOpGetConfigurationData,
- std::string(ConfigFileType config_type));
PDX_REMOTE_METHOD(DeleteGlobalBuffer, kOpDeleteGlobalBuffer,
void(DvrGlobalBufferKey key));
};
diff --git a/libs/vr/libdvr/Android.bp b/libs/vr/libdvr/Android.bp
index 2b4ebbe..4650871 100644
--- a/libs/vr/libdvr/Android.bp
+++ b/libs/vr/libdvr/Android.bp
@@ -27,6 +27,7 @@
"dvr_api.cpp",
"dvr_buffer.cpp",
"dvr_buffer_queue.cpp",
+ "dvr_configuration_data.cpp",
"dvr_display_manager.cpp",
"dvr_hardware_composer_client.cpp",
"dvr_surface.cpp",
diff --git a/libs/vr/libdvr/dvr_api.cpp b/libs/vr/libdvr/dvr_api.cpp
index 2c95583..5f35dcf 100644
--- a/libs/vr/libdvr/dvr_api.cpp
+++ b/libs/vr/libdvr/dvr_api.cpp
@@ -6,6 +6,7 @@
// Headers from libdvr
#include <dvr/dvr_buffer.h>
#include <dvr/dvr_buffer_queue.h>
+#include <dvr/dvr_configuration_data.h>
#include <dvr/dvr_display_manager.h>
#include <dvr/dvr_surface.h>
#include <dvr/dvr_vsync.h>
diff --git a/libs/vr/libdvr/dvr_configuration_data.cpp b/libs/vr/libdvr/dvr_configuration_data.cpp
new file mode 100644
index 0000000..df0d54e
--- /dev/null
+++ b/libs/vr/libdvr/dvr_configuration_data.cpp
@@ -0,0 +1,40 @@
+#include "include/dvr/dvr_configuration_data.h"
+
+#include <private/dvr/display_client.h>
+
+using android::dvr::display::ConfigFileType;
+using android::dvr::display::DisplayClient;
+
+extern "C" {
+
+int dvrConfigurationDataGet(int config_type, uint8_t** data,
+ size_t* data_size) {
+ if (!data || !data_size) {
+ return -EINVAL;
+ }
+
+ auto client = DisplayClient::Create();
+ if (!client) {
+ ALOGE("dvrGetGlobalBuffer: Failed to create display client!");
+ return -ECOMM;
+ }
+
+ ConfigFileType config_file_type = static_cast<ConfigFileType>(config_type);
+ auto config_data_status =
+ client->GetConfigurationData(config_file_type);
+
+ if (!config_data_status) {
+ return -config_data_status.error();
+ }
+
+ *data_size = config_data_status.get().size();
+ *data = new uint8_t[*data_size];
+ std::copy_n(config_data_status.get().begin(), *data_size, *data);
+ return 0;
+}
+
+void dvrConfigurationDataDestroy(uint8_t* data) {
+ delete[] data;
+}
+
+} // extern "C"
diff --git a/libs/vr/libdvr/dvr_display_manager.cpp b/libs/vr/libdvr/dvr_display_manager.cpp
index ffe090d..97cd4ee 100644
--- a/libs/vr/libdvr/dvr_display_manager.cpp
+++ b/libs/vr/libdvr/dvr_display_manager.cpp
@@ -5,13 +5,13 @@
#include <private/android/AHardwareBufferHelpers.h>
#include <private/dvr/buffer_hub_client.h>
#include <private/dvr/buffer_hub_queue_client.h>
+#include <private/dvr/display_client.h>
#include <private/dvr/display_manager_client.h>
#include "dvr_internal.h"
using android::AHardwareBuffer_convertToGrallocUsageBits;
using android::dvr::BufferConsumer;
-using android::dvr::display::ConfigFileType;
using android::dvr::display::DisplayManagerClient;
using android::dvr::display::SurfaceAttributes;
using android::dvr::display::SurfaceAttribute;
@@ -150,30 +150,6 @@
return 0;
}
-int dvrConfigurationDataGet(DvrDisplayManager* client, int config_type,
- uint8_t** data, size_t* data_size) {
- if (!client || !data || !data_size) {
- return -EINVAL;
- }
-
- ConfigFileType config_file_type = static_cast<ConfigFileType>(config_type);
- auto config_data_status =
- client->client->GetConfigurationData(config_file_type);
-
- if (!config_data_status) {
- return -config_data_status.error();
- }
-
- *data_size = config_data_status.get().size();
- *data = new uint8_t[*data_size];
- std::copy_n(config_data_status.get().begin(), *data_size, *data);
- return 0;
-}
-
-void dvrConfigurationDataDestroy(DvrDisplayManager*, uint8_t* data) {
- delete[] data;
-}
-
int dvrDisplayManagerGetEventFd(DvrDisplayManager* client) {
if (!client)
return -EINVAL;
diff --git a/libs/vr/libdvr/include/dvr/dvr_api.h b/libs/vr/libdvr/include/dvr/dvr_api.h
index 06f89da..0eb393e 100644
--- a/libs/vr/libdvr/include/dvr/dvr_api.h
+++ b/libs/vr/libdvr/include/dvr/dvr_api.h
@@ -40,6 +40,16 @@
struct native_handle;
+// Device metrics data type enums.
+enum {
+ // Request the device lens metrics protobuf. This matches cardboard protos.
+ DVR_CONFIGURATION_DATA_LENS_METRICS = 0,
+ // Request the device metrics protobuf.
+ DVR_CONFIGURATION_DATA_DEVICE_METRICS = 1,
+ // Request the per device configuration data file.
+ DVR_CONFIGURATION_DATA_DEVICE_CONFIG = 2,
+};
+
// dvr_display_manager.h
typedef int (*DvrDisplayManagerCreatePtr)(DvrDisplayManager** client_out);
typedef void (*DvrDisplayManagerDestroyPtr)(DvrDisplayManager* client);
@@ -56,11 +66,9 @@
typedef int (*DvrDisplayManagerGetReadBufferQueuePtr)(
DvrDisplayManager* client, int surface_id, int queue_id,
DvrReadBufferQueue** queue_out);
-typedef int (*DvrConfigurationDataGetPtr)(DvrDisplayManager* client,
- int config_type, uint8_t** data,
+typedef int (*DvrConfigurationDataGetPtr)(int config_type, uint8_t** data,
size_t* data_size);
-typedef void (*DvrConfigurationDataDestroyPtr)(DvrDisplayManager* client,
- uint8_t* data);
+typedef void (*DvrConfigurationDataDestroyPtr)(uint8_t* data);
typedef int (*DvrSurfaceStateCreatePtr)(DvrSurfaceState** surface_state);
typedef void (*DvrSurfaceStateDestroyPtr)(DvrSurfaceState* surface_state);
typedef int (*DvrSurfaceStateGetSurfaceCountPtr)(DvrSurfaceState* surface_state,
diff --git a/libs/vr/libdvr/include/dvr/dvr_configuration_data.h b/libs/vr/libdvr/include/dvr/dvr_configuration_data.h
new file mode 100644
index 0000000..22a509f
--- /dev/null
+++ b/libs/vr/libdvr/include/dvr/dvr_configuration_data.h
@@ -0,0 +1,24 @@
+#ifndef DVR_CONFIGURATION_DATA_H_
+#define DVR_CONFIGURATION_DATA_H_
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <sys/cdefs.h>
+
+#include <dvr/dvr_display_types.h>
+#include <dvr/dvr_surface.h>
+
+__BEGIN_DECLS
+
+// Loads device configuration data of DVR_CONFIGURATION_DATA_*.
+// @return 0 on success. Otherwise returns a negative error value.
+int dvrConfigurationDataGet(int config_type,
+ uint8_t** data, size_t* data_size);
+
+// Destroy the configuration data returned from dvrGetConfigurationData.
+void dvrConfigurationDataDestroy(uint8_t* data);
+
+__END_DECLS
+
+#endif // DVR_CONFIGURATION_DATA_H_
diff --git a/libs/vr/libdvr/include/dvr/dvr_display_manager.h b/libs/vr/libdvr/include/dvr/dvr_display_manager.h
index eb1e711..0f80e20 100644
--- a/libs/vr/libdvr/include/dvr/dvr_display_manager.h
+++ b/libs/vr/libdvr/include/dvr/dvr_display_manager.h
@@ -40,21 +40,6 @@
int dvrDisplayManagerDeleteGlobalBuffer(DvrDisplayManager* client,
DvrGlobalBufferKey key);
-// Device metrics data type enums.
-enum {
- DVR_CONFIGURATION_DATA_LENS_METRICS = 0,
- DVR_CONFIGURATION_DATA_DEVICE_METRICS = 1,
- DVR_CONFIGURATION_DATA_DEVICE_CONFIG = 2,
-};
-
-// Loads device configuration data of DVR_CONFIGURATION_DATA_*.
-// @return 0 on success. Otherwise returns a negative error value.
-int dvrConfigurationDataGet(DvrDisplayManager* client, int config_type,
- uint8_t** data, size_t* data_size);
-
-// Destroy the configuration data returned from dvrGetConfigurationData.
-void dvrConfigurationDataDestroy(DvrDisplayManager* client, uint8_t* data);
-
// Returns an fd used to signal when surface updates occur. Note that depending
// on the underlying transport, only a subset of the real event bits may be
// supported. Use dvrDisplayManagerClientTranslateEpollEventMask to get the real
diff --git a/libs/vr/libdvr/tests/dvr_display_manager-test.cpp b/libs/vr/libdvr/tests/dvr_display_manager-test.cpp
index 6f11465..2249154 100644
--- a/libs/vr/libdvr/tests/dvr_display_manager-test.cpp
+++ b/libs/vr/libdvr/tests/dvr_display_manager-test.cpp
@@ -10,6 +10,7 @@
#include <thread>
#include <vector>
+#include <dvr/dvr_configuration_data.h>
#include <dvr/dvr_deleter.h>
#include <dvr/dvr_display_manager.h>
#include <dvr/dvr_surface.h>
@@ -204,8 +205,7 @@
Status<std::vector<uint8_t>> GetConfigData(int config_type) {
uint8_t* data = nullptr;
size_t data_size = 0;
- int error = dvrConfigurationDataGet(display_manager_.get(), config_type,
- &data, &data_size);
+ int error = dvrConfigurationDataGet(config_type, &data, &data_size);
if (error < 0) {
return ErrorStatus(-error);
}
@@ -214,7 +214,7 @@
return ErrorStatus(EINVAL);
}
std::vector<uint8_t> data_result(data, data + data_size);
- dvrConfigurationDataDestroy(display_manager_.get(), data);
+ dvrConfigurationDataDestroy(data);
std::string s(data, data + data_size);
return {std::move(data_result)};
}
@@ -593,6 +593,7 @@
}
TEST_F(DvrDisplayManagerTest, ConfigurationData) {
+ // TODO(hendrikw): Move this out of the display manager tests.
auto data1 = manager_->GetConfigData(-1);
ASSERT_STATUS_ERROR(data1);
diff --git a/libs/vr/libvrflinger/display_manager_service.cpp b/libs/vr/libvrflinger/display_manager_service.cpp
index c5b0d88..0e9a6ab 100644
--- a/libs/vr/libvrflinger/display_manager_service.cpp
+++ b/libs/vr/libvrflinger/display_manager_service.cpp
@@ -1,7 +1,5 @@
#include "display_manager_service.h"
-#include <android-base/file.h>
-#include <android-base/properties.h>
#include <pdx/channel_handle.h>
#include <pdx/default_transport/service_endpoint.h>
#include <private/android_filesystem_config.h>
@@ -21,14 +19,6 @@
using android::pdx::rpc::IfAnyOf;
using android::pdx::rpc::RemoteMethodError;
-namespace {
-
-const char kDvrLensMetricsProperty[] = "ro.dvr.lens_metrics";
-const char kDvrDeviceMetricsProperty[] = "ro.dvr.device_metrics";
-const char kDvrDeviceConfigProperty[] = "ro.dvr.device_configuration";
-
-} // namespace
-
namespace android {
namespace dvr {
@@ -93,11 +83,6 @@
*this, &DisplayManagerService::OnSetupGlobalBuffer, message);
return {};
- case DisplayManagerProtocol::GetConfigurationData::Opcode:
- DispatchRemoteMethod<DisplayManagerProtocol::GetConfigurationData>(
- *this, &DisplayManagerService::OnGetConfigurationData, message);
- return {};
-
case DisplayManagerProtocol::DeleteGlobalBuffer::Opcode:
DispatchRemoteMethod<DisplayManagerProtocol::DeleteGlobalBuffer>(
*this, &DisplayManagerService::OnDeleteGlobalBuffer, message);
@@ -179,35 +164,6 @@
return display_service_->DeleteGlobalBuffer(key);
}
-pdx::Status<std::string> DisplayManagerService::OnGetConfigurationData(
- pdx::Message& message, display::ConfigFileType config_type) {
- std::string property_name;
- switch (config_type) {
- case display::ConfigFileType::kLensMetrics:
- property_name = kDvrLensMetricsProperty;
- break;
- case display::ConfigFileType::kDeviceMetrics:
- property_name = kDvrDeviceMetricsProperty;
- break;
- case display::ConfigFileType::kDeviceConfiguration:
- property_name = kDvrDeviceConfigProperty;
- break;
- default:
- return ErrorStatus(EINVAL);
- }
- std::string file_path = base::GetProperty(property_name, "");
- if (file_path.empty()) {
- return ErrorStatus(ENOENT);
- }
-
- std::string data;
- if (!base::ReadFileToString(file_path, &data)) {
- return ErrorStatus(errno);
- }
-
- return std::move(data);
-}
-
void DisplayManagerService::OnDisplaySurfaceChange() {
if (display_manager_)
display_manager_->SetNotificationsPending(true);
diff --git a/libs/vr/libvrflinger/display_manager_service.h b/libs/vr/libvrflinger/display_manager_service.h
index 20c5507..c869ceb 100644
--- a/libs/vr/libvrflinger/display_manager_service.h
+++ b/libs/vr/libvrflinger/display_manager_service.h
@@ -61,8 +61,6 @@
uint64_t usage);
pdx::Status<void> OnDeleteGlobalBuffer(pdx::Message& message,
DvrGlobalBufferKey key);
- pdx::Status<std::string> OnGetConfigurationData(
- pdx::Message& message, display::ConfigFileType config_type);
// Called by the display service to indicate changes to display surfaces that
// the display manager should evaluate.
diff --git a/libs/vr/libvrflinger/display_service.cpp b/libs/vr/libvrflinger/display_service.cpp
index b180848..dc9807a 100644
--- a/libs/vr/libvrflinger/display_service.cpp
+++ b/libs/vr/libvrflinger/display_service.cpp
@@ -3,6 +3,8 @@
#include <unistd.h>
#include <vector>
+#include <android-base/file.h>
+#include <android-base/properties.h>
#include <dvr/dvr_display_types.h>
#include <pdx/default_transport/service_endpoint.h>
#include <pdx/rpc/remote_method.h>
@@ -18,6 +20,14 @@
using android::pdx::default_transport::Endpoint;
using android::pdx::rpc::DispatchRemoteMethod;
+namespace {
+
+const char kDvrLensMetricsProperty[] = "ro.dvr.lens_metrics";
+const char kDvrDeviceMetricsProperty[] = "ro.dvr.device_metrics";
+const char kDvrDeviceConfigProperty[] = "ro.dvr.device_configuration";
+
+} // namespace
+
namespace android {
namespace dvr {
@@ -60,6 +70,11 @@
*this, &DisplayService::OnGetMetrics, message);
return {};
+ case DisplayProtocol::GetConfigurationData::Opcode:
+ DispatchRemoteMethod<DisplayProtocol::GetConfigurationData>(
+ *this, &DisplayService::OnGetConfigurationData, message);
+ return {};
+
case DisplayProtocol::CreateSurface::Opcode:
DispatchRemoteMethod<DisplayProtocol::CreateSurface>(
*this, &DisplayService::OnCreateSurface, message);
@@ -102,6 +117,35 @@
{}}};
}
+pdx::Status<std::string> DisplayService::OnGetConfigurationData(
+ pdx::Message& /*message*/, display::ConfigFileType config_type) {
+ std::string property_name;
+ switch (config_type) {
+ case display::ConfigFileType::kLensMetrics:
+ property_name = kDvrLensMetricsProperty;
+ break;
+ case display::ConfigFileType::kDeviceMetrics:
+ property_name = kDvrDeviceMetricsProperty;
+ break;
+ case display::ConfigFileType::kDeviceConfiguration:
+ property_name = kDvrDeviceConfigProperty;
+ break;
+ default:
+ return ErrorStatus(EINVAL);
+ }
+ std::string file_path = base::GetProperty(property_name, "");
+ if (file_path.empty()) {
+ return ErrorStatus(ENOENT);
+ }
+
+ std::string data;
+ if (!base::ReadFileToString(file_path, &data)) {
+ return ErrorStatus(errno);
+ }
+
+ return std::move(data);
+}
+
// Creates a new DisplaySurface and associates it with this channel. This may
// only be done once per channel.
Status<display::SurfaceInfo> DisplayService::OnCreateSurface(
diff --git a/libs/vr/libvrflinger/display_service.h b/libs/vr/libvrflinger/display_service.h
index 8ba1728..cb21e9f 100644
--- a/libs/vr/libvrflinger/display_service.h
+++ b/libs/vr/libvrflinger/display_service.h
@@ -88,6 +88,8 @@
pdx::Status<BorrowedNativeBufferHandle> OnGetGlobalBuffer(
pdx::Message& message, DvrGlobalBufferKey key);
pdx::Status<display::Metrics> OnGetMetrics(pdx::Message& message);
+ pdx::Status<std::string> OnGetConfigurationData(
+ pdx::Message& message, display::ConfigFileType config_type);
pdx::Status<display::SurfaceInfo> OnCreateSurface(
pdx::Message& message, const display::SurfaceAttributes& attributes);