Merge "Load with memory_order_acquire"
diff --git a/libs/vr/libbufferhub/buffer_client_impl.cpp b/libs/vr/libbufferhub/buffer_client_impl.cpp
index 6bef98a..30cbb9f 100644
--- a/libs/vr/libbufferhub/buffer_client_impl.cpp
+++ b/libs/vr/libbufferhub/buffer_client_impl.cpp
@@ -10,6 +10,8 @@
: BpInterface<IBufferClient>(impl) {}
bool isValid() override;
+
+ status_t duplicate(uint64_t* outToken) override;
};
IMPLEMENT_META_INTERFACE(BufferClient, "android.dvr.IBufferClient");
@@ -17,6 +19,7 @@
// Transaction code
enum {
IS_VALID = IBinder::FIRST_CALL_TRANSACTION,
+ DUPLICATE,
};
bool BpBufferClient::isValid() {
@@ -38,13 +41,42 @@
}
}
+status_t BpBufferClient::duplicate(uint64_t* outToken) {
+ Parcel data, reply;
+ status_t ret =
+ data.writeInterfaceToken(IBufferClient::getInterfaceDescriptor());
+ if (ret != NO_ERROR) {
+ ALOGE("BpBufferClient::duplicate: failed to write into parcel; errno=%d",
+ ret);
+ return ret;
+ }
+
+ ret = remote()->transact(DUPLICATE, data, &reply);
+ if (ret == NO_ERROR) {
+ *outToken = reply.readUint64();
+ return NO_ERROR;
+ } else {
+ ALOGE("BpBufferClient::duplicate: failed to transact; errno=%d", ret);
+ return ret;
+ }
+}
+
status_t BnBufferClient::onTransact(uint32_t code, const Parcel& data,
Parcel* reply, uint32_t flags) {
switch (code) {
case IS_VALID: {
CHECK_INTERFACE(IBufferClient, data, reply);
return reply->writeBool(isValid());
- } break;
+ }
+ case DUPLICATE: {
+ CHECK_INTERFACE(IBufferClient, data, reply);
+ uint64_t token = 0;
+ status_t ret = duplicate(&token);
+ if (ret != NO_ERROR) {
+ return ret;
+ }
+ return reply->writeUint64(token);
+ }
default:
// Should not reach except binder defined transactions such as dumpsys
return BBinder::onTransact(code, data, reply, flags);
diff --git a/libs/vr/libbufferhub/include/private/dvr/IBufferClient.h b/libs/vr/libbufferhub/include/private/dvr/IBufferClient.h
index 03f2d95..31bf79d 100644
--- a/libs/vr/libbufferhub/include/private/dvr/IBufferClient.h
+++ b/libs/vr/libbufferhub/include/private/dvr/IBufferClient.h
@@ -14,6 +14,10 @@
// Checks if the buffer node is valid.
virtual bool isValid() = 0;
+
+ // Duplicates the client. Token_out will be set to a new token when succeed,
+ // and not changed when failed.
+ virtual status_t duplicate(uint64_t* outToken) = 0;
};
// BnInterface for IBufferClient. Should only be created in bufferhub service.
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 43fa262..c219afd 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -451,6 +451,8 @@
}
void TimeStats::flushPowerTimeLocked() {
+ if (!mEnabled.load()) return;
+
nsecs_t curTime = systemTime();
// elapsedTime is in milliseconds.
int64_t elapsedTime = (curTime - mPowerTime.prevTime) / 1000000;
@@ -500,10 +502,14 @@
ALOGV("GlobalPresentFenceTime[%" PRId64 "]",
mGlobalRecord.presentFences.front()->getSignalTime());
- const int32_t presentToPresentMs = msBetween(mGlobalRecord.prevPresentTime, curPresentTime);
- ALOGV("Global present2present[%d]", presentToPresentMs);
+ if (mGlobalRecord.prevPresentTime != 0) {
+ const int32_t presentToPresentMs =
+ msBetween(mGlobalRecord.prevPresentTime, curPresentTime);
+ ALOGV("Global present2present[%d] prev[%" PRId64 "] curr[%" PRId64 "]",
+ presentToPresentMs, mGlobalRecord.prevPresentTime, curPresentTime);
+ mTimeStats.presentToPresent.insert(presentToPresentMs);
+ }
- mTimeStats.presentToPresent.insert(presentToPresentMs);
mGlobalRecord.prevPresentTime = curPresentTime;
mGlobalRecord.presentFences.pop_front();
}
@@ -514,7 +520,15 @@
ATRACE_CALL();
std::lock_guard<std::mutex> lock(mMutex);
- if (presentFence == nullptr) {
+ if (presentFence == nullptr || !presentFence->isValid()) {
+ mGlobalRecord.prevPresentTime = 0;
+ return;
+ }
+
+ if (mPowerTime.powerMode != HWC_POWER_MODE_NORMAL) {
+ // Try flushing the last present fence on HWC_POWER_MODE_NORMAL.
+ flushAvailableGlobalRecordsToStatsLocked();
+ mGlobalRecord.presentFences.clear();
mGlobalRecord.prevPresentTime = 0;
return;
}
@@ -537,10 +551,10 @@
ATRACE_CALL();
std::lock_guard<std::mutex> lock(mMutex);
- ALOGD("Enabled");
mEnabled.store(true);
mTimeStats.statsStart = static_cast<int64_t>(std::time(0));
mPowerTime.prevTime = systemTime();
+ ALOGD("Enabled");
}
void TimeStats::disable() {
@@ -549,16 +563,17 @@
ATRACE_CALL();
std::lock_guard<std::mutex> lock(mMutex);
- ALOGD("Disabled");
+ flushPowerTimeLocked();
mEnabled.store(false);
mTimeStats.statsEnd = static_cast<int64_t>(std::time(0));
+ ALOGD("Disabled");
}
void TimeStats::clear() {
ATRACE_CALL();
std::lock_guard<std::mutex> lock(mMutex);
- ALOGD("Cleared");
+ mTimeStatsTracker.clear();
mTimeStats.stats.clear();
mTimeStats.statsStart = (mEnabled.load() ? static_cast<int64_t>(std::time(0)) : 0);
mTimeStats.statsEnd = 0;
@@ -568,6 +583,9 @@
mTimeStats.displayOnTime = 0;
mTimeStats.presentToPresent.hist.clear();
mPowerTime.prevTime = systemTime();
+ mGlobalRecord.prevPresentTime = 0;
+ mGlobalRecord.presentFences.clear();
+ ALOGD("Cleared");
}
bool TimeStats::isEnabled() {
diff --git a/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp b/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
index c701224..599ff5c 100644
--- a/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
+++ b/services/surfaceflinger/TimeStats/timestatsproto/TimeStatsHelper.cpp
@@ -39,7 +39,7 @@
if (delta < 0) return;
// std::lower_bound won't work on out of range values
if (delta > histogramConfig[HISTOGRAM_SIZE - 1]) {
- hist[histogramConfig[HISTOGRAM_SIZE - 1]]++;
+ hist[histogramConfig[HISTOGRAM_SIZE - 1]] += delta / histogramConfig[HISTOGRAM_SIZE - 1];
return;
}
auto iter = std::lower_bound(histogramConfig.begin(), histogramConfig.end(), delta);
diff --git a/services/vr/bufferhubd/Android.bp b/services/vr/bufferhubd/Android.bp
index c35ddd4..ea9bb1f 100644
--- a/services/vr/bufferhubd/Android.bp
+++ b/services/vr/bufferhubd/Android.bp
@@ -29,6 +29,7 @@
name: "libbufferhubd",
srcs: [
"buffer_channel.cpp",
+ "buffer_client.cpp",
"buffer_hub.cpp",
"buffer_hub_binder.cpp",
"buffer_node.cpp",
diff --git a/services/vr/bufferhubd/IBufferHub.cpp b/services/vr/bufferhubd/IBufferHub.cpp
index 2f39e41..022a9cc 100644
--- a/services/vr/bufferhubd/IBufferHub.cpp
+++ b/services/vr/bufferhubd/IBufferHub.cpp
@@ -13,6 +13,8 @@
uint32_t layer_count, uint32_t format,
uint64_t usage,
uint64_t user_metadata_size) override;
+
+ status_t importBuffer(uint64_t token, sp<IBufferClient>* outClient) override;
};
IMPLEMENT_META_INTERFACE(BufferHub, "android.dvr.IBufferHub");
@@ -20,6 +22,7 @@
// Transaction code
enum {
CREATE_BUFFER = IBinder::FIRST_CALL_TRANSACTION,
+ IMPORT_BUFFER,
};
sp<IBufferClient> BpBufferHub::createBuffer(uint32_t width, uint32_t height,
@@ -50,6 +53,27 @@
}
}
+status_t BpBufferHub::importBuffer(uint64_t token,
+ sp<IBufferClient>* outClient) {
+ Parcel data, reply;
+ status_t ret = NO_ERROR;
+ ret |= data.writeInterfaceToken(IBufferHub::getInterfaceDescriptor());
+ ret |= data.writeUint64(token);
+ if (ret != NO_ERROR) {
+ ALOGE("BpBufferHub::importBuffer: failed to write into parcel");
+ return ret;
+ }
+
+ ret = remote()->transact(IMPORT_BUFFER, data, &reply);
+ if (ret == NO_ERROR) {
+ *outClient = interface_cast<IBufferClient>(reply.readStrongBinder());
+ return NO_ERROR;
+ } else {
+ ALOGE("BpBufferHub::importBuffer: failed to transact; errno=%d", ret);
+ return ret;
+ }
+}
+
status_t BnBufferHub::onTransact(uint32_t code, const Parcel& data,
Parcel* reply, uint32_t flags) {
switch (code) {
@@ -64,7 +88,18 @@
sp<IBufferClient> ret = createBuffer(width, height, layer_count, format,
usage, user_metadata_size);
return reply->writeStrongBinder(IInterface::asBinder(ret));
- } break;
+ }
+ case IMPORT_BUFFER: {
+ CHECK_INTERFACE(IBufferHub, data, reply);
+ uint64_t token = data.readUint64();
+ sp<IBufferClient> client;
+ status_t ret = importBuffer(token, &client);
+ if (ret == NO_ERROR) {
+ return reply->writeStrongBinder(IInterface::asBinder(client));
+ } else {
+ return ret;
+ }
+ }
default:
// Should not reach except binder defined transactions such as dumpsys
return BBinder::onTransact(code, data, reply, flags);
diff --git a/services/vr/bufferhubd/buffer_client.cpp b/services/vr/bufferhubd/buffer_client.cpp
new file mode 100644
index 0000000..f14faf7
--- /dev/null
+++ b/services/vr/bufferhubd/buffer_client.cpp
@@ -0,0 +1,18 @@
+#include <private/dvr/buffer_client.h>
+#include <private/dvr/buffer_hub_binder.h>
+
+namespace android {
+namespace dvr {
+
+status_t BufferClient::duplicate(uint64_t* outToken) {
+ if (!buffer_node_) {
+ // Should never happen
+ ALOGE("BufferClient::duplicate: node is missing.");
+ return UNEXPECTED_NULL;
+ }
+ return service_->registerToken(std::weak_ptr<BufferNode>(buffer_node_),
+ outToken);
+}
+
+} // namespace dvr
+} // namespace android
\ No newline at end of file
diff --git a/services/vr/bufferhubd/buffer_hub_binder.cpp b/services/vr/bufferhubd/buffer_hub_binder.cpp
index f8a9758..afd34aa 100644
--- a/services/vr/bufferhubd/buffer_hub_binder.cpp
+++ b/services/vr/bufferhubd/buffer_hub_binder.cpp
@@ -65,18 +65,65 @@
return NO_ERROR;
}
+status_t BufferHubBinderService::registerToken(
+ const std::weak_ptr<BufferNode> node, uint64_t* outToken) {
+ do {
+ *outToken = token_engine_();
+ } while (token_map_.find(*outToken) != token_map_.end());
+
+ token_map_.emplace(*outToken, node);
+ return NO_ERROR;
+}
+
sp<IBufferClient> BufferHubBinderService::createBuffer(
uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format,
uint64_t usage, uint64_t user_metadata_size) {
std::shared_ptr<BufferNode> node = std::make_shared<BufferNode>(
width, height, layer_count, format, usage, user_metadata_size);
- sp<BufferClient> client = new BufferClient(node);
+ sp<BufferClient> client = new BufferClient(node, this);
// Add it to list for bookkeeping and dumpsys.
client_list_.push_back(client);
return client;
}
+status_t BufferHubBinderService::importBuffer(uint64_t token,
+ sp<IBufferClient>* outClient) {
+ auto iter = token_map_.find(token);
+
+ if (iter == token_map_.end()) { // Not found
+ ALOGE("BufferHubBinderService::importBuffer: token %" PRIu64
+ "does not exist.",
+ token);
+ return PERMISSION_DENIED;
+ }
+
+ if (iter->second.expired()) { // Gone
+ ALOGW(
+ "BufferHubBinderService::importBuffer: the original node of token "
+ "%" PRIu64 "has gone.",
+ token);
+ token_map_.erase(iter);
+ return DEAD_OBJECT;
+ }
+
+ // Promote the weak_ptr
+ std::shared_ptr<BufferNode> node(iter->second);
+ if (!node) {
+ ALOGE("BufferHubBinderService::importBuffer: promote weak_ptr failed.");
+ token_map_.erase(iter);
+ return DEAD_OBJECT;
+ }
+
+ sp<BufferClient> client = new BufferClient(node, this);
+ *outClient = client;
+
+ token_map_.erase(iter);
+ client_list_.push_back(client);
+
+ return NO_ERROR;
+}
+
} // namespace dvr
} // namespace android
diff --git a/services/vr/bufferhubd/include/private/dvr/IBufferHub.h b/services/vr/bufferhubd/include/private/dvr/IBufferHub.h
index bd5f9cf..502c6d6 100644
--- a/services/vr/bufferhubd/include/private/dvr/IBufferHub.h
+++ b/services/vr/bufferhubd/include/private/dvr/IBufferHub.h
@@ -17,6 +17,9 @@
uint32_t layer_count, uint32_t format,
uint64_t usage,
uint64_t user_metadata_size) = 0;
+
+ virtual status_t importBuffer(uint64_t token,
+ sp<IBufferClient>* outClient) = 0;
};
class BnBufferHub : public BnInterface<IBufferHub> {
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_client.h b/services/vr/bufferhubd/include/private/dvr/buffer_client.h
index 20d51ee..d79ec0a 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_client.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_client.h
@@ -7,19 +7,31 @@
namespace android {
namespace dvr {
+// Forward declaration to avoid circular dependency
+class BufferHubBinderService;
+
class BufferClient : public BnBufferClient {
public:
// Creates a server-side buffer client from an existing BufferNode. Note that
// this funciton takes ownership of the shared_ptr.
- explicit BufferClient(std::shared_ptr<BufferNode> node)
- : buffer_node_(std::move(node)){};
+ explicit BufferClient(std::shared_ptr<BufferNode> node,
+ BufferHubBinderService* service)
+ : service_(service), buffer_node_(std::move(node)){};
// Binder IPC functions
bool isValid() override {
return buffer_node_ ? buffer_node_->IsValid() : false;
};
+ status_t duplicate(uint64_t* outToken) override;
+
private:
+ // Hold a pointer to the service to bypass binder interface, as BufferClient
+ // and the service will be in the same process. Also, since service owns
+ // Client, if service dead the clients will be destroyed, so this pointer is
+ // guaranteed to be valid.
+ BufferHubBinderService* service_;
+
std::shared_ptr<BufferNode> buffer_node_;
};
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h b/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h
index 9064d87..cf6124b 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_hub_binder.h
@@ -1,12 +1,15 @@
#ifndef ANDROID_DVR_BUFFER_HUB_BINDER_H
#define ANDROID_DVR_BUFFER_HUB_BINDER_H
+#include <random>
+#include <unordered_map>
#include <vector>
#include <binder/BinderService.h>
#include <private/dvr/IBufferHub.h>
#include <private/dvr/buffer_client.h>
#include <private/dvr/buffer_hub.h>
+#include <private/dvr/buffer_node.h>
namespace android {
namespace dvr {
@@ -15,20 +18,33 @@
public BnBufferHub {
public:
static status_t start(const std::shared_ptr<BufferHubService>& pdx_service);
- // Dump bufferhub related information to given fd (usually stdout)
+ // Dumps bufferhub related information to given fd (usually stdout)
// usage: adb shell dumpsys bufferhubd
virtual status_t dump(int fd, const Vector<String16>& args) override;
+ // Marks a BufferNode to be duplicated.
+ // TODO(b/116681016): add importToken(int64_t)
+ status_t registerToken(const std::weak_ptr<BufferNode> node,
+ uint64_t* outToken);
+
// Binder IPC functions
sp<IBufferClient> createBuffer(uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format,
uint64_t usage,
uint64_t user_metadata_size) override;
+ status_t importBuffer(uint64_t token, sp<IBufferClient>* outClient) override;
+
private:
std::shared_ptr<BufferHubService> pdx_service_;
std::vector<sp<BufferClient>> client_list_;
+
+ // TODO(b/118180214): use a more secure implementation
+ std::mt19937_64 token_engine_;
+ // The mapping from token to a specific node. This is a many-to-one mapping.
+ // One node could be refered by 0 to multiple tokens.
+ std::unordered_map<uint64_t, std::weak_ptr<BufferNode>> token_map_;
};
} // namespace dvr
diff --git a/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp b/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp
index 7fa2226..7c00fa6 100644
--- a/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp
+++ b/services/vr/bufferhubd/tests/buffer_hub_binder_service-test.cpp
@@ -10,6 +10,7 @@
namespace {
+using testing::IsNull;
using testing::NotNull;
const int kWidth = 640;
@@ -38,6 +39,46 @@
EXPECT_TRUE(bufferClient->isValid());
}
+TEST_F(BufferHubBinderServiceTest, TestDuplicateAndImportBuffer) {
+ sp<IBufferClient> bufferClient = service->createBuffer(
+ kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize);
+ ASSERT_THAT(bufferClient, NotNull());
+ EXPECT_TRUE(bufferClient->isValid());
+
+ uint64_t token1 = 0ULL;
+ status_t ret = bufferClient->duplicate(&token1);
+ EXPECT_EQ(ret, NO_ERROR);
+
+ // Tokens should be unique even corresponding to the same buffer
+ uint64_t token2 = 0ULL;
+ ret = bufferClient->duplicate(&token2);
+ EXPECT_EQ(ret, NO_ERROR);
+ EXPECT_NE(token2, token1);
+
+ sp<IBufferClient> bufferClient1;
+ ret = service->importBuffer(token1, &bufferClient1);
+ EXPECT_EQ(ret, NO_ERROR);
+ ASSERT_THAT(bufferClient1, NotNull());
+ EXPECT_TRUE(bufferClient1->isValid());
+
+ // Consumes the token to keep the service clean
+ sp<IBufferClient> bufferClient2;
+ ret = service->importBuffer(token2, &bufferClient2);
+ EXPECT_EQ(ret, NO_ERROR);
+ ASSERT_THAT(bufferClient2, NotNull());
+ EXPECT_TRUE(bufferClient2->isValid());
+}
+
+TEST_F(BufferHubBinderServiceTest, TestImportUnexistingToken) {
+ // There is very little chance that this test fails if there is a token = 0
+ // in the service.
+ uint64_t unexistingToken = 0ULL;
+ sp<IBufferClient> bufferClient;
+ status_t ret = service->importBuffer(unexistingToken, &bufferClient);
+ EXPECT_EQ(ret, PERMISSION_DENIED);
+ EXPECT_THAT(bufferClient, IsNull());
+}
+
} // namespace
} // namespace dvr