Merge "Move BufferHubMetadata off pdx::fileHandle" (7878ccc0) · Commits · e / os / android_frameworks_native-old

libs/ui/BufferHubBuffer.cpp

+7 −3

Original line number	Diff line number	Diff line
		@@ -36,10 +36,12 @@
		#include <private/dvr/bufferhub_rpc.h>
		#pragma clang diagnostic pop

		#include <ui/BufferHubBuffer.h>

		#include <poll.h>

		#include <android-base/unique_fd.h>
		#include <ui/BufferHubBuffer.h>

		using android::base::unique_fd;
		using android::dvr::BufferTraits;
		using android::dvr::DetachedBufferRPC;
		using android::dvr::NativeHandleWrapper;
		@@ -132,7 +134,9 @@ int BufferHubBuffer::ImportGraphicBuffer() {
		const int bufferId = bufferTraits.id();

		// Import the metadata.
		mMetadata = BufferHubMetadata::Import(bufferTraits.take_metadata_handle());
		LocalHandle metadataHandle = bufferTraits.take_metadata_handle();
		unique_fd metadataFd(metadataHandle.Release());
		mMetadata = BufferHubMetadata::Import(std::move(metadataFd));

		if (!mMetadata.IsValid()) {
		ALOGE("BufferHubBuffer::ImportGraphicBuffer: invalid metadata.");

libs/ui/BufferHubMetadata.cpp

+12 −12

Original line number	Diff line number	Diff line
		@@ -48,47 +48,47 @@ BufferHubMetadata BufferHubMetadata::Create(size_t userMetadataSize) {
		return {};
		}

		// Hand over the ownership of the fd to a pdx::LocalHandle immediately after the successful
		// return of ashmem_create_region. The ashmemHandle is going to own the fd and to prevent fd
		// Hand over the ownership of the fd to a unique_fd immediately after the successful
		// return of ashmem_create_region. The ashmemFd is going to own the fd and to prevent fd
		// leaks during error handling.
		pdx::LocalHandle ashmemHandle{fd};
		unique_fd ashmemFd{fd};

		if (ashmem_set_prot_region(ashmemHandle.Get(), kAshmemProt) != 0) {
		if (ashmem_set_prot_region(ashmemFd.get(), kAshmemProt) != 0) {
		ALOGE("BufferHubMetadata::Create: failed to set protect region.");
		return {};
		}

		return BufferHubMetadata::Import(std::move(ashmemHandle));
		return BufferHubMetadata::Import(std::move(ashmemFd));
		}

		/* static */
		BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmemHandle) {
		if (!ashmem_valid(ashmemHandle.Get())) {
		BufferHubMetadata BufferHubMetadata::Import(unique_fd ashmemFd) {
		if (!ashmem_valid(ashmemFd.get())) {
		ALOGE("BufferHubMetadata::Import: invalid ashmem fd.");
		return {};
		}

		size_t metadataSize = static_cast<size_t>(ashmem_get_size_region(ashmemHandle.Get()));
		size_t metadataSize = static_cast<size_t>(ashmem_get_size_region(ashmemFd.get()));
		size_t userMetadataSize = metadataSize - kMetadataHeaderSize;

		// Note that here the buffer state is mapped from shared memory as an atomic object. The
		// std::atomic's constructor will not be called so that the original value stored in the memory
		// region can be preserved.
		auto metadataHeader = static_cast<MetadataHeader*>(mmap(nullptr, metadataSize, kAshmemProt,
		MAP_SHARED, ashmemHandle.Get(),
		MAP_SHARED, ashmemFd.get(),
		/offset=/0));
		if (metadataHeader == nullptr) {
		ALOGE("BufferHubMetadata::Import: failed to map region.");
		return {};
		}

		return BufferHubMetadata(userMetadataSize, std::move(ashmemHandle), metadataHeader);
		return BufferHubMetadata(userMetadataSize, std::move(ashmemFd), metadataHeader);
		}

		BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle,
		BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd,
		MetadataHeader* metadataHeader)
		: mUserMetadataSize(userMetadataSize),
		mAshmemHandle(std::move(ashmemHandle)),
		mAshmemFd(std::move(ashmemFd)),
		mMetadataHeader(metadataHeader) {}

		BufferHubMetadata::~BufferHubMetadata() {

libs/ui/include/ui/BufferHubMetadata.h

+13 −11

Original line number	Diff line number	Diff line
		@@ -33,12 +33,17 @@
		#pragma clang diagnostic ignored "-Wundefined-func-template"
		#pragma clang diagnostic ignored "-Wunused-template"
		#pragma clang diagnostic ignored "-Wweak-vtables"
		#include <pdx/file_handle.h>
		#include <private/dvr/buffer_hub_defs.h>
		#pragma clang diagnostic pop

		#include <android-base/unique_fd.h>

		namespace android {

		namespace {
		using base::unique_fd;
		} // namespace

		class BufferHubMetadata {
		public:
		// Creates a new BufferHubMetadata backed by an ashmem region.
		@@ -50,11 +55,8 @@ public:

		// Imports an existing BufferHubMetadata from an ashmem FD.
		//
		// TODO(b/112338294): Refactor BufferHub to use Binder as its internal IPC backend instead of
		// UDS.
		//
		// @param ashmemHandle Ashmem file handle representing an ashmem region.
		static BufferHubMetadata Import(pdx::LocalHandle ashmemHandle);
		// @param ashmemFd Ashmem file descriptor representing an ashmem region.
		static BufferHubMetadata Import(unique_fd ashmemFd);

		BufferHubMetadata() = default;

		@@ -67,7 +69,7 @@ public:
		mUserMetadataSize = other.mUserMetadataSize;
		other.mUserMetadataSize = 0;

		mAshmemHandle = std::move(other.mAshmemHandle);
		mAshmemFd = std::move(other.mAshmemFd);

		// The old raw mMetadataHeader pointer must be cleared, otherwise the destructor will
		// automatically mummap() the shared memory.
		@@ -79,25 +81,25 @@ public:

		// Returns true if the metadata is valid, i.e. the metadata has a valid ashmem fd and the ashmem
		// has been mapped into virtual address space.
		bool IsValid() const { return mAshmemHandle.IsValid() && mMetadataHeader != nullptr; }
		bool IsValid() const { return mAshmemFd.get() != -1 && mMetadataHeader != nullptr; }

		size_t user_metadata_size() const { return mUserMetadataSize; }
		size_t metadata_size() const {
		return mUserMetadataSize + dvr::BufferHubDefs::kMetadataHeaderSize;
		}

		const pdx::LocalHandle& ashmem_handle() const { return mAshmemHandle; }
		const unique_fd& ashmem_fd() const { return mAshmemFd; }
		dvr::BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; }

		private:
		BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle,
		BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd,
		dvr::BufferHubDefs::MetadataHeader* metadataHeader);

		BufferHubMetadata(const BufferHubMetadata&) = delete;
		void operator=(const BufferHubMetadata&) = delete;

		size_t mUserMetadataSize = 0;
		pdx::LocalHandle mAshmemHandle;
		unique_fd mAshmemFd;
		dvr::BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr;
		};

libs/ui/tests/BufferHubMetadata_test.cpp

+10 −12

Original line number	Diff line number	Diff line
		@@ -43,11 +43,11 @@ TEST_F(BufferHubMetadataTest, Import_Success) {
		EXPECT_TRUE(m1.IsValid());
		EXPECT_NE(m1.metadata_header(), nullptr);

		pdx::LocalHandle h2 = m1.ashmem_handle().Duplicate();
		EXPECT_TRUE(h2.IsValid());
		unique_fd h2 = unique_fd(dup(m1.ashmem_fd().get()));
		EXPECT_NE(h2.get(), -1);

		BufferHubMetadata m2 = BufferHubMetadata::Import(std::move(h2));
		EXPECT_FALSE(h2.IsValid());
		EXPECT_EQ(h2.get(), -1);
		EXPECT_TRUE(m1.IsValid());
		BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header();
		EXPECT_NE(mh1, nullptr);
		@@ -71,31 +71,29 @@ TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) {
		BufferHubMetadata m1 = BufferHubMetadata::Create(sizeof(int));
		EXPECT_TRUE(m1.IsValid());
		EXPECT_NE(m1.metadata_header(), nullptr);
		EXPECT_TRUE(m1.ashmem_handle().IsValid());
		EXPECT_NE(m1.ashmem_fd().get(), -1);
		EXPECT_EQ(m1.user_metadata_size(), sizeof(int));

		BufferHubMetadata m2 = std::move(m1);

		// After the move, the metadata header (a raw pointer) should be reset in the
		// older buffer.
		// After the move, the metadata header (a raw pointer) should be reset in the older buffer.
		EXPECT_EQ(m1.metadata_header(), nullptr);
		EXPECT_NE(m2.metadata_header(), nullptr);

		EXPECT_FALSE(m1.ashmem_handle().IsValid());
		EXPECT_TRUE(m2.ashmem_handle().IsValid());
		EXPECT_EQ(m1.ashmem_fd().get(), -1);
		EXPECT_NE(m2.ashmem_fd().get(), -1);

		EXPECT_EQ(m1.user_metadata_size(), 0U);
		EXPECT_EQ(m2.user_metadata_size(), sizeof(int));

		BufferHubMetadata m3{std::move(m2)};

		// After the move, the metadata header (a raw pointer) should be reset in the
		// older buffer.
		// After the move, the metadata header (a raw pointer) should be reset in the older buffer.
		EXPECT_EQ(m2.metadata_header(), nullptr);
		EXPECT_NE(m3.metadata_header(), nullptr);

		EXPECT_FALSE(m2.ashmem_handle().IsValid());
		EXPECT_TRUE(m3.ashmem_handle().IsValid());
		EXPECT_EQ(m2.ashmem_fd().get(), -1);
		EXPECT_NE(m3.ashmem_fd().get(), -1);

		EXPECT_EQ(m2.user_metadata_size(), 0U);
		EXPECT_EQ(m3.user_metadata_size(), sizeof(int));

services/vr/bufferhubd/buffer_channel.cpp

+4 −1

Original line number	Diff line number	Diff line
		@@ -83,10 +83,13 @@ Status<BufferTraits<BorrowedHandle>> BufferChannel::OnImport(
		ATRACE_NAME("BufferChannel::OnImport");
		ALOGD_IF(TRACE, "BufferChannel::OnImport: buffer=%d.", buffer_id());

		BorrowedHandle ashmem_handle =
		BorrowedHandle(buffer_node_->metadata().ashmem_fd().get());

		// TODO(b/112057680) Move away from the GraphicBuffer-based IonBuffer.
		return BufferTraits<BorrowedHandle>{
		/buffer_handle=/buffer_node_->buffer_handle(),
		/metadata_handle=/buffer_node_->metadata().ashmem_handle().Borrow(),
		/metadata_handle=/ashmem_handle,
		/id=/buffer_id(),
		/client_state_mask=/client_state_mask_,
		/metadata_size=/buffer_node_->metadata().metadata_size(),