Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9bbafdb5 authored by Andy Hung's avatar Andy Hung Committed by Android (Google) Code Review
Browse files

Merge "Improve MediaBuffer robustness for remote clients" into nyc-mr1-dev

parents 01b589c3 9bd3c9b0
Loading
Loading
Loading
Loading
+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
    };

+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:
+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);
+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);

+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) {