Improve MediaBuffer robustness for remote clients (9bd3c9b0) · Commits · e / os / android_frameworks_av

include/media/stagefright/MediaBuffer.h

+38 −31

Original line number	Diff line number	Diff line
		@@ -68,11 +68,16 @@ public:
		mMemory = mem;
		}

		// Decrements the reference count and returns the buffer to its
		// associated MediaBufferGroup if the reference count drops to 0.
		// If MediaBufferGroup is set, decrement the local reference count;
		// if the local reference count drops to 0, return the buffer to the
		// associated MediaBufferGroup.
		//
		// If no MediaBufferGroup is set, the local reference count must be zero
		// when called, whereupon the MediaBuffer is deleted.
		virtual void release();

		// Increments the reference count.
		// Increments the local reference count.
		// Use only when MediaBufferGroup is set.
		virtual void add_ref();

		void *data() const;
		@@ -97,7 +102,28 @@ public:
		// MetaData.
		MediaBuffer *clone();

		int refcount() const;
		// sum of localRefcount() and remoteRefcount()
		int refcount() const {
		return localRefcount() + remoteRefcount();
		}

		int localRefcount() const {
		return mRefCount;
		}

		int remoteRefcount() const {
		if (mMemory.get() == nullptr \|\| mMemory->pointer() == nullptr) return 0;
		int32_t remoteRefcount =
		reinterpret_cast<SharedControl *>(mMemory->pointer())->getRemoteRefcount();
		// Sanity check so that remoteRefCount() is non-negative.
		return remoteRefcount >= 0 ? remoteRefcount : 0; // do not allow corrupted data.
		}

		// returns old value
		int addRemoteRefcount(int32_t value) {
		if (mMemory.get() == nullptr \|\| mMemory->pointer() == nullptr) return 0;
		return reinterpret_cast<SharedControl *>(mMemory->pointer())->addRemoteRefcount(value);
		}

		bool isDeadObject() const {
		return isDeadObject(mMemory);
		@@ -117,25 +143,6 @@ public:
		}

		protected:
		// MediaBuffer remote releases are handled through a
		// pending release count variable stored in a SharedControl block
		// at the start of the IMemory.

		// Returns old value of pending release count.
		inline int32_t addPendingRelease(int32_t value) {
		return getSharedControl()->addPendingRelease(value);
		}

		// Issues all pending releases (works in parallel).
		// Assumes there is a MediaBufferObserver.
		inline void resolvePendingRelease() {
		if (mMemory.get() == nullptr) return;
		while (addPendingRelease(-1) > 0) {
		release();
		}
		addPendingRelease(1);
		}

		// true if MediaBuffer is observed (part of a MediaBufferGroup).
		inline bool isObserved() const {
		return mObserver != nullptr;
		@@ -181,18 +188,18 @@ private:
		};

		// returns old value
		inline int32_t addPendingRelease(int32_t value) {
		inline int32_t addRemoteRefcount(int32_t value) {
		return std::atomic_fetch_add_explicit(
		&mPendingRelease, (int_least32_t)value, std::memory_order_seq_cst);
		&mRemoteRefcount, (int_least32_t)value, std::memory_order_seq_cst);
		}

		inline int32_t getPendingRelease() const {
		return std::atomic_load_explicit(&mPendingRelease, std::memory_order_seq_cst);
		inline int32_t getRemoteRefcount() const {
		return std::atomic_load_explicit(&mRemoteRefcount, std::memory_order_seq_cst);
		}

		inline void setPendingRelease(int32_t value) {
		inline void setRemoteRefcount(int32_t value) {
		std::atomic_store_explicit(
		&mPendingRelease, (int_least32_t)value, std::memory_order_seq_cst);
		&mRemoteRefcount, (int_least32_t)value, std::memory_order_seq_cst);
		}

		inline bool isDeadObject() const {
		@@ -209,13 +216,13 @@ private:
		std::atomic_store_explicit(
		&mFlags, (int_least32_t)0, std::memory_order_seq_cst);
		std::atomic_store_explicit(
		&mPendingRelease, (int_least32_t)0, std::memory_order_seq_cst);
		&mRemoteRefcount, (int_least32_t)0, std::memory_order_seq_cst);
		}

		private:
		// Caution: atomic_int_fast32_t is 64 bits on LP64.
		std::atomic_int_least32_t mFlags;
		std::atomic_int_least32_t mPendingRelease;
		std::atomic_int_least32_t mRemoteRefcount;
		int32_t unused[6]; // additional buffer space
		};

include/media/stagefright/MediaBufferGroup.h

+1 −4

Original line number	Diff line number	Diff line
		@@ -53,10 +53,7 @@ public:

		size_t buffers() const { return mBuffers.size(); }

		// freeBuffers is the number of free buffers allowed to remain.
		void gc(size_t freeBuffers = 0);

		protected:
		// If buffer is nullptr, have acquire_buffer() check for remote release.
		virtual void signalBufferReturned(MediaBuffer *buffer);

		private:

media/libmedia/IMediaSource.cpp

+12 −10

Original line number	Diff line number	Diff line
		@@ -58,9 +58,9 @@ public:

		protected:
		virtual ~RemoteMediaBufferWrapper() {
		// Indicate to MediaBufferGroup to release.
		int32_t old = addPendingRelease(1);
		ALOGV("RemoteMediaBufferWrapper: releasing %p, old %d", this, old);
		// Release our interest in the MediaBuffer's shared memory.
		int32_t old = addRemoteRefcount(-1);
		ALOGV("RemoteMediaBufferWrapper: releasing %p, refcount %d", this, old - 1);
		mMemory.clear(); // don't set the dead object flag.
		}
		};
		@@ -296,8 +296,8 @@ status_t BnMediaSource::onTransact(
		case STOP: {
		ALOGV("stop");
		CHECK_INTERFACE(IMediaSource, data, reply);
		mGroup->signalBufferReturned(nullptr);
		status_t status = stop();
		mGroup->gc();
		mIndexCache.reset();
		mBuffersSinceStop = 0;
		return status;
		@@ -305,6 +305,7 @@ status_t BnMediaSource::onTransact(
		case PAUSE: {
		ALOGV("pause");
		CHECK_INTERFACE(IMediaSource, data, reply);
		mGroup->signalBufferReturned(nullptr);
		return pause();
		}
		case GETFORMAT: {
		@@ -336,7 +337,7 @@ status_t BnMediaSource::onTransact(
		&& len == sizeof(opts)
		&& data.read((void *)&opts, len) == NO_ERROR;

		mGroup->gc(kBinderMediaBuffers /* freeBuffers */);
		mGroup->signalBufferReturned(nullptr);
		mIndexCache.gc();
		size_t inlineTransferSize = 0;
		status_t ret = NO_ERROR;
		@@ -411,11 +412,12 @@ status_t BnMediaSource::onTransact(
		reply->writeInt32(offset);
		reply->writeInt32(length);
		buf->meta_data()->writeToParcel(*reply);
		if (transferBuf != buf) {
		buf->release();
		} else if (!supportNonblockingRead()) {
		if (transferBuf == buf) {
		buf->addRemoteRefcount(1);
		if (!supportNonblockingRead()) {
		maxNumBuffers = 0; // stop readMultiple with one shared buffer.
		}
		}
		} else {
		ALOGV_IF(buf->mMemory != nullptr,
		"INLINE(%p) %zu shared mem available, but only %zu used",
		@@ -423,12 +425,12 @@ status_t BnMediaSource::onTransact(
		reply->writeInt32(INLINE_BUFFER);
		reply->writeByteArray(length, (uint8_t*)buf->data() + offset);
		buf->meta_data()->writeToParcel(*reply);
		buf->release();
		inlineTransferSize += length;
		if (inlineTransferSize > kInlineMaxTransfer) {
		maxNumBuffers = 0; // stop readMultiple if inline transfer is too large.
		}
		}
		buf->release();
		}
		reply->writeInt32(NULL_BUFFER); // Indicate no more MediaBuffers.
		reply->writeInt32(ret);

media/libstagefright/foundation/MediaBuffer.cpp

+1 −12

Original line number	Diff line number	Diff line
		@@ -105,14 +105,7 @@ MediaBuffer::MediaBuffer(const sp<ABuffer> &buffer)

		void MediaBuffer::release() {
		if (mObserver == NULL) {
		if (mMemory.get() != nullptr) {
		// See if there is a pending release and there are no observers.
		// Ideally this never happens.
		while (addPendingRelease(-1) > 0) {
		__sync_fetch_and_sub(&mRefCount, 1);
		}
		addPendingRelease(1);
		}
		// Legacy contract for MediaBuffer without a MediaBufferGroup.
		CHECK_EQ(mRefCount, 0);
		delete this;
		return;
		@@ -205,10 +198,6 @@ void MediaBuffer::setObserver(MediaBufferObserver *observer) {
		mObserver = observer;
		}

		int MediaBuffer::refcount() const {
		return mRefCount;
		}

		MediaBuffer *MediaBuffer::clone() {
		CHECK(mGraphicBuffer == NULL);

media/libstagefright/foundation/MediaBufferGroup.cpp

+19 −28

Original line number	Diff line number	Diff line
		@@ -51,7 +51,7 @@ MediaBufferGroup::MediaBufferGroup(size_t buffers, size_t buffer_size, size_t gr

		for (size_t i = 0; i < buffers; ++i) {
		sp<IMemory> mem = memoryDealer->allocate(augmented_size);
		if (mem.get() == nullptr) {
		if (mem.get() == nullptr \|\| mem->pointer() == nullptr) {
		ALOGW("Only allocated %zu shared buffers of size %zu", i, buffer_size);
		break;
		}
		@@ -76,11 +76,24 @@ MediaBufferGroup::MediaBufferGroup(size_t buffers, size_t buffer_size, size_t gr

		MediaBufferGroup::~MediaBufferGroup() {
		for (MediaBuffer *buffer : mBuffers) {
		buffer->resolvePendingRelease();
		// If we don't release it, perhaps noone will release it.
		LOG_ALWAYS_FATAL_IF(buffer->refcount() != 0,
		"buffer refcount %p = %d != 0", buffer, buffer->refcount());
		// actually delete it.
		if (buffer->refcount() != 0) {
		const int localRefcount = buffer->localRefcount();
		const int remoteRefcount = buffer->remoteRefcount();

		// Fatal if we have a local refcount.
		LOG_ALWAYS_FATAL_IF(localRefcount != 0,
		"buffer(%p) localRefcount %d != 0, remoteRefcount %d",
		buffer, localRefcount, remoteRefcount);

		// Log an error if we have a remaining remote refcount,
		// as the remote process may have died or may have inappropriate behavior.
		// The shared memory associated with the MediaBuffer will
		// automatically be reclaimed when there are no remaining fds
		// associated with it.
		ALOGE("buffer(%p) has residual remoteRefcount %d",
		buffer, remoteRefcount);
		}
		// gracefully delete.
		buffer->setObserver(nullptr);
		buffer->release();
		}
		@@ -94,32 +107,11 @@ void MediaBufferGroup::add_buffer(MediaBuffer *buffer) {
		// optionally: mGrowthLimit = max(mGrowthLimit, mBuffers.size());
		}

		void MediaBufferGroup::gc(size_t freeBuffers) {
		Mutex::Autolock autoLock(mLock);

		size_t freeCount = 0;
		for (auto it = mBuffers.begin(); it != mBuffers.end(); ) {
		(*it)->resolvePendingRelease();
		if ((*it)->isDeadObject()) {
		// The MediaBuffer has been deleted, why is it in the MediaBufferGroup?
		LOG_ALWAYS_FATAL("buffer(%p) has dead object with refcount %d",
		(it), (it)->refcount());
		} else if ((*it)->refcount() == 0 && ++freeCount > freeBuffers) {
		(*it)->setObserver(nullptr);
		(*it)->release();
		it = mBuffers.erase(it);
		} else {
		++it;
		}
		}
		}

		bool MediaBufferGroup::has_buffers() {
		if (mBuffers.size() < mGrowthLimit) {
		return true; // We can add more buffers internally.
		}
		for (MediaBuffer *buffer : mBuffers) {
		buffer->resolvePendingRelease();
		if (buffer->refcount() == 0) {
		return true;
		}
		@@ -135,7 +127,6 @@ status_t MediaBufferGroup::acquire_buffer(
		MediaBuffer *buffer = nullptr;
		auto free = mBuffers.end();
		for (auto it = mBuffers.begin(); it != mBuffers.end(); ++it) {
		(*it)->resolvePendingRelease();
		if ((*it)->refcount() == 0) {
		const size_t size = (*it)->size();
		if (size >= requestedSize) {