IMediaSource: Improve shared memory buffer transfer

Bug: 29125703
Change-Id: Icf1180dee65f6504e6c10dd4d5b28a8e441f67d1
diff --git a/include/media/IMediaSource.h b/include/media/IMediaSource.h
index 524e7aa..7a7599d 100644
--- a/include/media/IMediaSource.h
+++ b/include/media/IMediaSource.h
@@ -18,14 +18,17 @@
 
 #define IMEDIA_SOURCE_BASE_H_
 
+#include <map>
+
 #include <binder/IInterface.h>
+#include <binder/IMemory.h>
+#include <media/stagefright/MediaBuffer.h>
 #include <media/stagefright/MediaErrors.h>
 
 namespace android {
 
 struct MediaSource;
 class MetaData;
-class MediaBuffer;
 class MediaBufferGroup;
 
 class IMediaSource : public IInterface {
@@ -56,7 +59,7 @@
     // a) not request a seek
     // b) not be late, i.e. lateness_us = 0
     struct ReadOptions {
-        enum SeekMode {
+        enum SeekMode : int32_t {
             SEEK_PREVIOUS_SYNC,
             SEEK_NEXT_SYNC,
             SEEK_CLOSEST_SYNC,
@@ -72,6 +75,7 @@
         void clearSeekTo();
         bool getSeekTo(int64_t *time_us, SeekMode *mode) const;
 
+        // TODO: remove this if unused.
         void setLateBy(int64_t lateness_us);
         int64_t getLateBy() const;
 
@@ -79,6 +83,11 @@
         void clearNonBlocking();
         bool getNonBlocking() const;
 
+        // Used to clear all non-persistent options for multiple buffer reads.
+        void clearNonPersistent() {
+            clearSeekTo();
+        }
+
     private:
         enum Options {
             kSeekTo_Option      = 1,
@@ -98,21 +107,26 @@
     // A result of INFO_FORMAT_CHANGED indicates that the format of this
     // MediaSource has changed mid-stream, the client can continue reading
     // but should be prepared for buffers of the new configuration.
+    //
+    // TODO: consider removing read() in favor of readMultiple().
     virtual status_t read(
             MediaBuffer **buffer, const ReadOptions *options = NULL) = 0;
 
-    // Returns a vector of new buffers of data. The vector size could be
-    // <= |maxNumBuffers|. Used for buffers with small size
-    // since all buffer data are passed back by binder, not shared memory.
+    // Returns a vector of new buffers of data, where the new buffers are added
+    // to the end of the vector.
     // Call blocks until an error is encountered, or the end of the stream is
     // reached, or format change is hit, or |kMaxNumReadMultiple| buffers have
     // been read.
-    // End of stream is signalled by a result of ERROR_END_OF_STREAM.
+    // End of stream is signaled by a result of ERROR_END_OF_STREAM.
     // A result of INFO_FORMAT_CHANGED indicates that the format of this
     // MediaSource has changed mid-stream, the client can continue reading
     // but should be prepared for buffers of the new configuration.
+    //
+    // ReadOptions may be specified. Persistent options apply to all reads;
+    // non-persistent options (e.g. seek) apply only to the first read.
     virtual status_t readMultiple(
-            Vector<MediaBuffer *> *buffers, uint32_t maxNumBuffers = 1) = 0;
+            Vector<MediaBuffer *> *buffers, uint32_t maxNumBuffers = 1,
+            const ReadOptions *options = nullptr) = 0;
 
     // Returns true if |readMultiple| is supported, otherwise false.
     virtual bool supportReadMultiple() = 0;
@@ -148,20 +162,92 @@
     }
 
     virtual status_t readMultiple(
-            Vector<MediaBuffer *> * /* buffers */, uint32_t /* maxNumBuffers = 1 */) {
+            Vector<MediaBuffer *> * /* buffers */, uint32_t /* maxNumBuffers = 1 */,
+            const ReadOptions * /* options = nullptr */) {
         return ERROR_UNSUPPORTED;
     }
 
     virtual bool supportReadMultiple() {
         return false;
     }
+
+    static const size_t kBinderMediaBuffers = 4; // buffers managed by BnMediaSource
+
 protected:
     virtual ~BnMediaSource();
 
 private:
-    MediaBufferGroup *mGroup;
-};
+    uint32_t mBuffersSinceStop; // Buffer tracking variable
 
+    std::unique_ptr<MediaBufferGroup> mGroup;
+
+    // To prevent marshalling IMemory with each read transaction, we cache the IMemory pointer
+    // into a map.
+    //
+    // This is converted into an index, which is used to identify the associated memory
+    // on the receiving side.  We hold a reference to the IMemory here to ensure it doesn't
+    // change underneath us.
+
+    struct IndexCache {
+        IndexCache() : mIndex(0) { }
+
+        // Returns the index of the IMemory stored in cache or 0 if not found.
+        uint64_t lookup(const sp<IMemory> &mem) {
+            auto p = mMemoryToIndex.find(mem.get());
+            if (p == mMemoryToIndex.end()) {
+                return 0;
+            }
+            if (MediaBuffer::isDeadObject(p->second.first)) {
+                // this object's dead
+                ALOGW("Attempting to lookup a dead IMemory");
+                (void)mMemoryToIndex.erase(p);
+                return 0;
+            }
+            ALOGW_IF(p->second.first.get() != mem.get(), "Mismatched buffers without reset");
+            return p->second.second;
+        }
+
+        // Returns the index of the IMemory stored in the index cache.
+        uint64_t insert(const sp<IMemory> &mem) {
+            auto p = mMemoryToIndex.find(mem.get());
+            if (p == mMemoryToIndex.end()) {
+                if (mIndex == UINT64_MAX) {
+                    ALOGE("Index overflow");
+                    mIndex = 1; // skip overflow condition and hope for the best
+                } else {
+                    ++mIndex;
+                }
+                (void)mMemoryToIndex.emplace(// C++11 mem.get(), std::make_pair(mem, mIndex))
+                        std::piecewise_construct,
+                        std::forward_as_tuple(mem.get()), std::forward_as_tuple(mem, mIndex));
+                return mIndex;
+            }
+            ALOGW("IMemory already inserted into cache");
+            return p->second.second;
+        }
+
+        void reset() {
+            mMemoryToIndex.clear();
+            mIndex = 0;
+        }
+
+        void gc() {
+            for (auto it = mMemoryToIndex.begin(); it != mMemoryToIndex.end(); ) {
+                if (MediaBuffer::isDeadObject(it->second.first)) {
+                    it = mMemoryToIndex.erase(it);
+                } else {
+                    ++it;
+                }
+            }
+        }
+
+    private:
+        uint64_t mIndex;
+        // C++14 unordered_map erase on iterator is stable; C++11 has no guarantee.
+        // Could key on uintptr_t instead of IMemory *
+        std::map<IMemory *, std::pair<sp<IMemory>, uint64_t>> mMemoryToIndex;
+    } mIndexCache;
+};
 
 }  // namespace android
 
diff --git a/include/media/stagefright/MediaBuffer.h b/include/media/stagefright/MediaBuffer.h
index 18b80e3..55b1f58 100644
--- a/include/media/stagefright/MediaBuffer.h
+++ b/include/media/stagefright/MediaBuffer.h
@@ -18,6 +18,8 @@
 
 #define MEDIA_BUFFER_H_
 
+#include <atomic>
+#include <list>
 #include <media/stagefright/foundation/MediaBufferBase.h>
 
 #include <pthread.h>
@@ -60,6 +62,12 @@
 
     MediaBuffer(const sp<ABuffer> &buffer);
 
+    MediaBuffer(const sp<IMemory> &mem) :
+        MediaBuffer((uint8_t *)mem->pointer() + sizeof(SharedControl), mem->size()) {
+        // delegate and override mMemory
+        mMemory = mem;
+    }
+
     // Decrements the reference count and returns the buffer to its
     // associated MediaBufferGroup if the reference count drops to 0.
     virtual void release();
@@ -91,9 +99,44 @@
 
     int refcount() const;
 
+    bool isDeadObject() const {
+        return isDeadObject(mMemory);
+    }
+
+    static bool isDeadObject(const sp<IMemory> &memory) {
+        if (memory.get() == nullptr || memory->pointer() == nullptr) return false;
+        return reinterpret_cast<SharedControl *>(memory->pointer())->isDeadObject();
+    }
+
 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;
+    }
+
     virtual ~MediaBuffer();
 
+    sp<IMemory> mMemory;
+
 private:
     friend class MediaBufferGroup;
     friend class OMXDecoder;
@@ -105,7 +148,6 @@
     void claim();
 
     MediaBufferObserver *mObserver;
-    MediaBuffer *mNextBuffer;
     int mRefCount;
 
     void *mData;
@@ -119,12 +161,57 @@
 
     MediaBuffer *mOriginal;
 
-    void setNextBuffer(MediaBuffer *buffer);
-    MediaBuffer *nextBuffer();
-
     MediaBuffer(const MediaBuffer &);
     MediaBuffer &operator=(const MediaBuffer &);
-    sp<IMemory> mMemory;
+
+    // SharedControl block at the start of IMemory.
+    struct SharedControl {
+        enum {
+            FLAG_DEAD_OBJECT = (1 << 0),
+        };
+
+        // returns old value
+        inline int32_t addPendingRelease(int32_t value) {
+            return std::atomic_fetch_add_explicit(
+                    &mPendingRelease, (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 void setPendingRelease(int32_t value) {
+            std::atomic_store_explicit(
+                    &mPendingRelease, (int_least32_t)value, std::memory_order_seq_cst);
+        }
+
+        inline bool isDeadObject() const {
+            return (std::atomic_load_explicit(
+                    &mFlags, std::memory_order_seq_cst) & FLAG_DEAD_OBJECT) != 0;
+        }
+
+        inline void setDeadObject() {
+            (void)std::atomic_fetch_or_explicit(
+                    &mFlags, (int_least32_t)FLAG_DEAD_OBJECT, std::memory_order_seq_cst);
+        }
+
+        inline void clear() {
+            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);
+        }
+
+    private:
+        // Caution: atomic_int_fast32_t is 64 bits on LP64.
+        std::atomic_int_least32_t mFlags;
+        std::atomic_int_least32_t mPendingRelease;
+        int32_t unused[6]; // additional buffer space
+    };
+
+    inline SharedControl *getSharedControl() const {
+         return reinterpret_cast<SharedControl *>(mMemory->pointer());
+     }
 };
 
 }  // namespace android
diff --git a/include/media/stagefright/MediaBufferGroup.h b/include/media/stagefright/MediaBufferGroup.h
index 7ca3fa1..d1f59f4 100644
--- a/include/media/stagefright/MediaBufferGroup.h
+++ b/include/media/stagefright/MediaBufferGroup.h
@@ -29,7 +29,7 @@
 
 class MediaBufferGroup : public MediaBufferObserver {
 public:
-    MediaBufferGroup();
+    MediaBufferGroup(size_t growthLimit = 0);
     ~MediaBufferGroup();
 
     void add_buffer(MediaBuffer *buffer);
@@ -45,6 +45,11 @@
     status_t acquire_buffer(
             MediaBuffer **buffer, bool nonBlocking = false, size_t requestedSize = 0);
 
+    size_t buffers() const { return mBuffers.size(); }
+
+    // freeBuffers is the number of free buffers allowed to remain.
+    void gc(size_t freeBuffers = 0);
+
 protected:
     virtual void signalBufferReturned(MediaBuffer *buffer);
 
@@ -53,8 +58,8 @@
 
     Mutex mLock;
     Condition mCondition;
-
-    MediaBuffer *mFirstBuffer, *mLastBuffer;
+    size_t mGrowthLimit;  // Do not automatically grow group larger than this.
+    std::list<MediaBuffer *> mBuffers;
 
     MediaBufferGroup(const MediaBufferGroup &);
     MediaBufferGroup &operator=(const MediaBufferGroup &);
diff --git a/media/libmedia/IMediaSource.cpp b/media/libmedia/IMediaSource.cpp
index d2b4291..2adce19 100644
--- a/media/libmedia/IMediaSource.cpp
+++ b/media/libmedia/IMediaSource.cpp
@@ -36,7 +36,7 @@
     STOP,
     PAUSE,
     GETFORMAT,
-    READ,
+    // READ, deprecated
     READMULTIPLE,
     RELEASE_BUFFER
 };
@@ -44,71 +44,30 @@
 enum {
     NULL_BUFFER,
     SHARED_BUFFER,
-    INLINE_BUFFER
+    INLINE_BUFFER,
+    SHARED_BUFFER_INDEX,
 };
 
-class RemoteMediaBufferReleaser : public BBinder {
-public:
-    RemoteMediaBufferReleaser(MediaBuffer *buf, sp<BnMediaSource> owner) {
-        mBuf = buf;
-        mOwner = owner;
-    }
-    ~RemoteMediaBufferReleaser() {
-        if (mBuf) {
-            ALOGW("RemoteMediaBufferReleaser dtor called while still holding buffer");
-            mBuf->release();
-        }
-    }
-    virtual status_t    onTransact( uint32_t code,
-                                    const Parcel& data,
-                                    Parcel* reply,
-                                    uint32_t flags = 0) {
-        if (code == RELEASE_BUFFER) {
-            mBuf->release();
-            mBuf = NULL;
-            return OK;
-        } else {
-            return BBinder::onTransact(code, data, reply, flags);
-        }
-    }
-private:
-    MediaBuffer *mBuf;
-    // Keep a ref to ensure MediaBuffer is released before the owner, i.e., BnMediaSource,
-    // because BnMediaSource needs to delete MediaBufferGroup in its dtor and
-    // MediaBufferGroup dtor requires all MediaBuffer's have 0 ref count.
-    sp<BnMediaSource> mOwner;
-};
-
-
 class RemoteMediaBufferWrapper : public MediaBuffer {
 public:
-    RemoteMediaBufferWrapper(sp<IMemory> mem, sp<IBinder> source);
+    RemoteMediaBufferWrapper(const sp<IMemory> &mem)
+        : MediaBuffer(mem) {
+        ALOGV("RemoteMediaBufferWrapper: creating %p", this);
+    }
+
 protected:
-    virtual ~RemoteMediaBufferWrapper();
-private:
-    sp<IMemory> mMemory;
-    sp<IBinder> mRemoteSource;
+    virtual ~RemoteMediaBufferWrapper() {
+        // Indicate to MediaBufferGroup to release.
+        int32_t old = addPendingRelease(1);
+        ALOGV("RemoteMediaBufferWrapper: releasing %p, old %d", this, old);
+        mMemory.clear(); // don't set the dead object flag.
+    }
 };
 
-RemoteMediaBufferWrapper::RemoteMediaBufferWrapper(sp<IMemory> mem, sp<IBinder> source)
-: MediaBuffer(mem->pointer(), mem->size()) {
-    mMemory = mem;
-    mRemoteSource = source;
-}
-
-RemoteMediaBufferWrapper::~RemoteMediaBufferWrapper() {
-    mMemory.clear();
-    // Explicitly ask the remote side to release the buffer. We could also just clear
-    // mRemoteSource, but that doesn't immediately release the reference on the remote side.
-    Parcel data, reply;
-    mRemoteSource->transact(RELEASE_BUFFER, data, &reply);
-    mRemoteSource.clear();
-}
-
 class BpMediaSource : public BpInterface<IMediaSource> {
 public:
     BpMediaSource(const sp<IBinder>& impl)
-        : BpInterface<IMediaSource>(impl)
+        : BpInterface<IMediaSource>(impl), mBuffersSinceStop(0)
     {
     }
 
@@ -135,7 +94,10 @@
         ALOGV("stop");
         Parcel data, reply;
         data.writeInterfaceToken(BpMediaSource::getInterfaceDescriptor());
-        return remote()->transact(STOP, data, &reply);
+        status_t status = remote()->transact(STOP, data, &reply);
+        mMemoryCache.reset();
+        mBuffersSinceStop = 0;
+        return status;
     }
 
     virtual sp<MetaData> getFormat() {
@@ -151,46 +113,16 @@
     }
 
     virtual status_t read(MediaBuffer **buffer, const ReadOptions *options) {
-        ALOGV("read");
-        Parcel data, reply;
-        data.writeInterfaceToken(BpMediaSource::getInterfaceDescriptor());
-        if (options) {
-            data.writeByteArray(sizeof(*options), (uint8_t*) options);
-        }
-        status_t ret = remote()->transact(READ, data, &reply);
-        if (ret != NO_ERROR) {
-            return ret;
-        }
-        // wrap the returned data in a MediaBuffer
-        ret = reply.readInt32();
-        int32_t buftype = reply.readInt32();
-        if (buftype == SHARED_BUFFER) {
-            sp<IBinder> remote = reply.readStrongBinder();
-            sp<IBinder> binder = reply.readStrongBinder();
-            sp<IMemory> mem = interface_cast<IMemory>(binder);
-            if (mem == NULL) {
-                ALOGE("received NULL IMemory for shared buffer");
-            }
-            size_t offset = reply.readInt32();
-            size_t length = reply.readInt32();
-            MediaBuffer *buf = new RemoteMediaBufferWrapper(mem, remote);
-            buf->set_range(offset, length);
-            buf->meta_data()->updateFromParcel(reply);
-            *buffer = buf;
-        } else if (buftype == NULL_BUFFER) {
-            ALOGV("got status %d and NULL buffer", ret);
-            *buffer = NULL;
-        } else {
-            int32_t len = reply.readInt32();
-            ALOGV("got status %d and len %d", ret, len);
-            *buffer = new MediaBuffer(len);
-            reply.read((*buffer)->data(), len);
-            (*buffer)->meta_data()->updateFromParcel(reply);
-        }
+        Vector<MediaBuffer *> buffers;
+        status_t ret = readMultiple(&buffers, 1 /* maxNumBuffers */, options);
+        *buffer = buffers.size() == 0 ? nullptr : buffers[0];
+        ALOGV("read status %d, bufferCount %u, sinceStop %u",
+                ret, *buffer != nullptr, mBuffersSinceStop);
         return ret;
     }
 
-    virtual status_t readMultiple(Vector<MediaBuffer *> *buffers, uint32_t maxNumBuffers) {
+    virtual status_t readMultiple(
+            Vector<MediaBuffer *> *buffers, uint32_t maxNumBuffers, const ReadOptions *options) {
         ALOGV("readMultiple");
         if (buffers == NULL || !buffers->isEmpty()) {
             return BAD_VALUE;
@@ -198,26 +130,59 @@
         Parcel data, reply;
         data.writeInterfaceToken(BpMediaSource::getInterfaceDescriptor());
         data.writeUint32(maxNumBuffers);
+        if (options != nullptr) {
+            data.writeByteArray(sizeof(*options), (uint8_t*) options);
+        }
         status_t ret = remote()->transact(READMULTIPLE, data, &reply);
+        mMemoryCache.gc();
         if (ret != NO_ERROR) {
             return ret;
         }
         // wrap the returned data in a vector of MediaBuffers
-        int32_t bufCount = 0;
-        while (1) {
-            if (reply.readInt32() == 0) {
-                break;
+        int32_t buftype;
+        uint32_t bufferCount = 0;
+        while ((buftype = reply.readInt32()) != NULL_BUFFER) {
+            LOG_ALWAYS_FATAL_IF(bufferCount >= maxNumBuffers,
+                    "Received %u+ buffers and requested %u buffers",
+                    bufferCount + 1, maxNumBuffers);
+            MediaBuffer *buf;
+            if (buftype == SHARED_BUFFER || buftype == SHARED_BUFFER_INDEX) {
+                uint64_t index = reply.readUint64();
+                ALOGV("Received %s index %llu",
+                        buftype == SHARED_BUFFER ? "SHARED_BUFFER" : "SHARED_BUFFER_INDEX",
+                        (unsigned long long) index);
+                sp<IMemory> mem;
+                if (buftype == SHARED_BUFFER) {
+                    sp<IBinder> binder = reply.readStrongBinder();
+                    mem = interface_cast<IMemory>(binder);
+                    LOG_ALWAYS_FATAL_IF(mem.get() == nullptr,
+                            "Received NULL IMemory for shared buffer");
+                    mMemoryCache.insert(index, mem);
+                } else {
+                    mem = mMemoryCache.lookup(index);
+                    LOG_ALWAYS_FATAL_IF(mem.get() == nullptr,
+                            "Received invalid IMemory index for shared buffer: %llu",
+                            (unsigned long long)index);
+                }
+                size_t offset = reply.readInt32();
+                size_t length = reply.readInt32();
+                buf = new RemoteMediaBufferWrapper(mem);
+                buf->set_range(offset, length);
+                buf->meta_data()->updateFromParcel(reply);
+            } else { // INLINE_BUFFER
+                int32_t len = reply.readInt32();
+                ALOGV("INLINE_BUFFER status %d and len %d", ret, len);
+                buf = new MediaBuffer(len);
+                reply.read(buf->data(), len);
+                buf->meta_data()->updateFromParcel(reply);
             }
-            int32_t len = reply.readInt32();
-            ALOGV("got len %d", len);
-            MediaBuffer *buf = new MediaBuffer(len);
-            reply.read(buf->data(), len);
-            buf->meta_data()->updateFromParcel(reply);
             buffers->push_back(buf);
-            ++bufCount;
+            ++bufferCount;
+            ++mBuffersSinceStop;
         }
         ret = reply.readInt32();
-        ALOGV("got status %d, bufCount %d", ret, bufCount);
+        ALOGV("readMultiple status %d, bufferCount %u, sinceStop %u",
+                ret, bufferCount, mBuffersSinceStop);
         return ret;
     }
 
@@ -238,10 +203,51 @@
     }
 
 private:
+
+    uint32_t mBuffersSinceStop; // Buffer tracking variable
+
     // NuPlayer passes pointers-to-metadata around, so we use this to keep the metadata alive
     // XXX: could we use this for caching, or does metadata change on the fly?
     sp<MetaData> mMetaData;
 
+    // Cache all IMemory objects received from MediaExtractor.
+    // We gc IMemory objects that are no longer active (referenced by a MediaBuffer).
+
+    struct MemoryCache {
+        sp<IMemory> lookup(uint64_t index) {
+            auto p = mIndexToMemory.find(index);
+            if (p == mIndexToMemory.end()) {
+                ALOGE("cannot find index!");
+                return nullptr;
+            }
+            return p->second;
+        }
+
+        void insert(uint64_t index, const sp<IMemory> &mem) {
+            if (mIndexToMemory.find(index) != mIndexToMemory.end()) {
+                ALOGE("index %llu already present", (unsigned long long)index);
+                return;
+            }
+            (void)mIndexToMemory.emplace(index, mem);
+        }
+
+        void reset() {
+            mIndexToMemory.clear();
+        }
+
+        void gc() {
+            for (auto it = mIndexToMemory.begin(); it != mIndexToMemory.end(); ) {
+                if (MediaBuffer::isDeadObject(it->second)) {
+                    it = mIndexToMemory.erase(it);
+                } else {
+                    ++it;
+                }
+            }
+        }
+    private:
+        // C++14 unordered_map erase on iterator is stable; C++11 has no guarantee.
+        std::map<uint64_t, sp<IMemory>> mIndexToMemory;
+    } mMemoryCache;
 };
 
 IMPLEMENT_META_INTERFACE(MediaSource, "android.media.IMediaSource");
@@ -250,12 +256,11 @@
 #define LOG_TAG "BnMediaSource"
 
 BnMediaSource::BnMediaSource()
-    : mGroup(NULL) {
+    : mBuffersSinceStop(0)
+    , mGroup(new MediaBufferGroup(kBinderMediaBuffers /* growthLimit */)) {
 }
 
 BnMediaSource::~BnMediaSource() {
-    delete mGroup;
-    mGroup = NULL;
 }
 
 status_t BnMediaSource::onTransact(
@@ -278,7 +283,11 @@
         case STOP: {
             ALOGV("stop");
             CHECK_INTERFACE(IMediaSource, data, reply);
-            return stop();
+            status_t status = stop();
+            mGroup->gc();
+            mIndexCache.reset();
+            mBuffersSinceStop = 0;
+            return status;
         }
         case PAUSE: {
             ALOGV("pause");
@@ -295,116 +304,112 @@
             }
             return UNKNOWN_ERROR;
         }
-        case READ: {
-            ALOGV("read");
-            CHECK_INTERFACE(IMediaSource, data, reply);
-            status_t ret;
-            MediaBuffer *buf = NULL;
-            ReadOptions opts;
-            uint32_t len;
-            if (data.readUint32(&len) == NO_ERROR &&
-                    len == sizeof(opts) && data.read((void*)&opts, len) == NO_ERROR) {
-                ret = read(&buf, &opts);
-            } else {
-                ret = read(&buf, NULL);
-            }
-
-            reply->writeInt32(ret);
-            if (buf != NULL) {
-                size_t usedSize = buf->range_length();
-                // even if we're using shared memory, we might not want to use it, since for small
-                // sizes it's faster to copy data through the Binder transaction
-                // On the other hand, if the data size is large enough, it's better to use shared
-                // memory. When data is too large, binder can't handle it.
-                if (usedSize >= MediaBuffer::kSharedMemThreshold) {
-                    ALOGV("use shared memory: %zu", usedSize);
-
-                    MediaBuffer *transferBuf = buf;
-                    size_t offset = buf->range_offset();
-                    if (transferBuf->mMemory == NULL) {
-                        if (mGroup == NULL) {
-                            mGroup = new MediaBufferGroup;
-                            size_t allocateSize = usedSize;
-                            if (usedSize < SIZE_MAX / 3) {
-                                allocateSize = usedSize * 3 / 2;
-                            }
-                            mGroup->add_buffer(new MediaBuffer(allocateSize));
-                        }
-
-                        MediaBuffer *newBuf = NULL;
-                        ret = mGroup->acquire_buffer(
-                                &newBuf, false /* nonBlocking */, usedSize);
-                        if (ret != OK || newBuf == NULL || newBuf->mMemory == NULL) {
-                            ALOGW("failed to acquire shared memory, ret %d", ret);
-                            buf->release();
-                            if (newBuf != NULL) {
-                                newBuf->release();
-                            }
-                            reply->writeInt32(NULL_BUFFER);
-                            return NO_ERROR;
-                        }
-                        transferBuf = newBuf;
-                        memcpy(transferBuf->data(), (uint8_t*)buf->data() + buf->range_offset(),
-                                buf->range_length());
-                        offset = 0;
-                    }
-
-                    reply->writeInt32(SHARED_BUFFER);
-                    RemoteMediaBufferReleaser *wrapper =
-                        new RemoteMediaBufferReleaser(transferBuf, this);
-                    reply->writeStrongBinder(wrapper);
-                    reply->writeStrongBinder(IInterface::asBinder(transferBuf->mMemory));
-                    reply->writeInt32(offset);
-                    reply->writeInt32(usedSize);
-                    buf->meta_data()->writeToParcel(*reply);
-                    if (buf->mMemory == NULL) {
-                        buf->release();
-                    }
-                } else {
-                    // buffer is small: copy it
-                    if (buf->mMemory != NULL) {
-                        ALOGV("%zu shared mem available, but only %zu used", buf->mMemory->size(), buf->range_length());
-                    }
-                    reply->writeInt32(INLINE_BUFFER);
-                    reply->writeByteArray(buf->range_length(), (uint8_t*)buf->data() + buf->range_offset());
-                    buf->meta_data()->writeToParcel(*reply);
-                    buf->release();
-                }
-            } else {
-                ALOGV("ret %d, buf %p", ret, buf);
-                reply->writeInt32(NULL_BUFFER);
-            }
-            return NO_ERROR;
-        }
         case READMULTIPLE: {
-            ALOGV("readmultiple");
+            ALOGV("readMultiple");
             CHECK_INTERFACE(IMediaSource, data, reply);
+
+            // Get max number of buffers to read.
             uint32_t maxNumBuffers;
             data.readUint32(&maxNumBuffers);
-            status_t ret = NO_ERROR;
-            uint32_t bufferCount = 0;
             if (maxNumBuffers > kMaxNumReadMultiple) {
                 maxNumBuffers = kMaxNumReadMultiple;
             }
-            while (bufferCount < maxNumBuffers) {
-                if (reply->dataSize() >= MediaBuffer::kSharedMemThreshold) {
+
+            // Get read options, if any.
+            ReadOptions opts;
+            uint32_t len;
+            const bool useOptions =
+                    data.readUint32(&len) == NO_ERROR
+                    && len == sizeof(opts)
+                    && data.read((void *)&opts, len) == NO_ERROR;
+
+            mGroup->gc(kBinderMediaBuffers /* freeBuffers */);
+            mIndexCache.gc();
+            status_t ret = NO_ERROR;
+            uint32_t bufferCount = 0;
+            for (; bufferCount < maxNumBuffers; ++bufferCount, ++mBuffersSinceStop) {
+                MediaBuffer *buf = nullptr;
+                ret = read(&buf, useOptions ? &opts : nullptr);
+                opts.clearNonPersistent(); // Remove options that only apply to first buffer.
+                if (ret != NO_ERROR || buf == nullptr) {
                     break;
                 }
 
-                MediaBuffer *buf = NULL;
-                ret = read(&buf, NULL);
-                if (ret != NO_ERROR || buf == NULL) {
-                    break;
+                // Even if we're using shared memory, we might not want to use it, since for small
+                // sizes it's faster to copy data through the Binder transaction
+                // On the other hand, if the data size is large enough, it's better to use shared
+                // memory. When data is too large, binder can't handle it.
+                //
+                // TODO: reduce MediaBuffer::kSharedMemThreshold
+                MediaBuffer *transferBuf = nullptr;
+                const size_t length = buf->range_length();
+                size_t offset = buf->range_offset();
+                if (length >= MediaBuffer::kSharedMemThreshold) {
+                    if (buf->mMemory != nullptr) {
+                        ALOGV("Use shared memory: %zu", length);
+                        transferBuf = buf;
+                    } else {
+                        ALOGD("Large buffer %zu without IMemory!", length);
+                        ret = mGroup->acquire_buffer(
+                                &transferBuf, false /* nonBlocking */, length);
+                        if (ret != OK
+                                || transferBuf == nullptr
+                                || transferBuf->mMemory == nullptr) {
+                            ALOGW("Failed to acquire shared memory, size %zu, ret %d",
+                                    length, ret);
+                            if (transferBuf != nullptr) {
+                                transferBuf->release();
+                                transferBuf = nullptr;
+                            }
+                            // Current buffer transmit inline; no more additional buffers.
+                            maxNumBuffers = 0;
+                        } else {
+                            memcpy(transferBuf->data(), (uint8_t*)buf->data() + offset, length);
+                            offset = 0;
+                        }
+                    }
                 }
-                ++bufferCount;
-                reply->writeInt32(1);  // indicate one more MediaBuffer.
-                reply->writeByteArray(
-                        buf->range_length(), (uint8_t*)buf->data() + buf->range_offset());
-                buf->meta_data()->writeToParcel(*reply);
-                buf->release();
+                if (transferBuf != nullptr) { // Using shared buffers.
+                    if (!transferBuf->isObserved()) {
+                        // Transfer buffer must be part of a MediaBufferGroup.
+                        ALOGV("adding shared memory buffer %p to local group", transferBuf);
+                        mGroup->add_buffer(transferBuf);
+                        transferBuf->add_ref(); // We have already acquired buffer.
+                    }
+                    uint64_t index = mIndexCache.lookup(transferBuf->mMemory);
+                    if (index == 0) {
+                        index = mIndexCache.insert(transferBuf->mMemory);
+                        reply->writeInt32(SHARED_BUFFER);
+                        reply->writeUint64(index);
+                        reply->writeStrongBinder(IInterface::asBinder(transferBuf->mMemory));
+                        ALOGV("SHARED_BUFFER(%p) %llu",
+                                transferBuf, (unsigned long long)index);
+                    } else {
+                        reply->writeInt32(SHARED_BUFFER_INDEX);
+                        reply->writeUint64(index);
+                        ALOGV("SHARED_BUFFER_INDEX(%p) %llu",
+                                transferBuf, (unsigned long long)index);
+                    }
+                    reply->writeInt32(offset);
+                    reply->writeInt32(length);
+                    buf->meta_data()->writeToParcel(*reply);
+                    if (transferBuf != buf) {
+                        buf->release();
+                    }
+                } else {
+                    ALOGV_IF(buf->mMemory != nullptr,
+                            "INLINE(%p) %zu shared mem available, but only %zu used",
+                            buf, buf->mMemory->size(), length);
+                    reply->writeInt32(INLINE_BUFFER);
+                    reply->writeByteArray(length, (uint8_t*)buf->data() + offset);
+                    buf->meta_data()->writeToParcel(*reply);
+                    buf->release();
+                }
             }
-            reply->writeInt32(0);  // indicate no more MediaBuffer.
+            reply->writeInt32(NULL_BUFFER); // Indicate no more MediaBuffers.
             reply->writeInt32(ret);
+            ALOGV("readMultiple status %d, bufferCount %u, sinceStop %u",
+                    ret, bufferCount, mBuffersSinceStop);
             return NO_ERROR;
         }
         default:
diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp
index 1a0539d..efe82ba 100644
--- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp
+++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp
@@ -1444,7 +1444,7 @@
             }
         }
 
-        options.clearSeekTo();
+        options.clearNonPersistent();
 
         size_t id = 0;
         size_t count = mediaBuffers.size();
diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerStreamListener.h b/media/libmediaplayerservice/nuplayer/NuPlayerStreamListener.h
index 2de829b..0c6f652 100644
--- a/media/libmediaplayerservice/nuplayer/NuPlayerStreamListener.h
+++ b/media/libmediaplayerservice/nuplayer/NuPlayerStreamListener.h
@@ -24,7 +24,7 @@
 
 namespace android {
 
-struct MemoryDealer;
+class MemoryDealer;
 
 struct NuPlayer::NuPlayerStreamListener : public BnStreamListener {
     NuPlayerStreamListener(
diff --git a/media/libstagefright/foundation/MediaBuffer.cpp b/media/libstagefright/foundation/MediaBuffer.cpp
index fa8e241..15d557d 100644
--- a/media/libstagefright/foundation/MediaBuffer.cpp
+++ b/media/libstagefright/foundation/MediaBuffer.cpp
@@ -32,7 +32,6 @@
 
 MediaBuffer::MediaBuffer(void *data, size_t size)
     : mObserver(NULL),
-      mNextBuffer(NULL),
       mRefCount(0),
       mData(data),
       mSize(size),
@@ -45,7 +44,6 @@
 
 MediaBuffer::MediaBuffer(size_t size)
     : mObserver(NULL),
-      mNextBuffer(NULL),
       mRefCount(0),
       mData(NULL),
       mSize(size),
@@ -57,8 +55,10 @@
     if (size < kSharedMemThreshold) {
         mData = malloc(size);
     } else {
-        sp<MemoryDealer> memoryDealer = new MemoryDealer(size, "MediaBuffer");
-        mMemory = memoryDealer->allocate(size);
+        ALOGV("creating memoryDealer");
+        sp<MemoryDealer> memoryDealer =
+                new MemoryDealer(size + sizeof(SharedControl), "MediaBuffer");
+        mMemory = memoryDealer->allocate(size + sizeof(SharedControl));
         if (mMemory == NULL) {
             ALOGW("Failed to allocate shared memory, trying regular allocation!");
             mData = malloc(size);
@@ -66,7 +66,8 @@
                 ALOGE("Out of memory");
             }
         } else {
-            mData = mMemory->pointer();
+            getSharedControl()->clear();
+            mData = (uint8_t *)mMemory->pointer() + sizeof(SharedControl);
             ALOGV("Allocated shared mem buffer of size %zu @ %p", size, mData);
         }
     }
@@ -74,7 +75,6 @@
 
 MediaBuffer::MediaBuffer(const sp<GraphicBuffer>& graphicBuffer)
     : mObserver(NULL),
-      mNextBuffer(NULL),
       mRefCount(0),
       mData(NULL),
       mSize(1),
@@ -88,7 +88,6 @@
 
 MediaBuffer::MediaBuffer(const sp<ABuffer> &buffer)
     : mObserver(NULL),
-      mNextBuffer(NULL),
       mRefCount(0),
       mData(buffer->data()),
       mSize(buffer->size()),
@@ -102,6 +101,14 @@
 
 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);
+        }
         CHECK_EQ(mRefCount, 0);
         delete this;
         return;
@@ -183,6 +190,10 @@
         mOriginal->release();
         mOriginal = NULL;
     }
+
+   if (mMemory.get() != nullptr) {
+       getSharedControl()->setDeadObject();
+   }
 }
 
 void MediaBuffer::setObserver(MediaBufferObserver *observer) {
@@ -190,14 +201,6 @@
     mObserver = observer;
 }
 
-void MediaBuffer::setNextBuffer(MediaBuffer *buffer) {
-    mNextBuffer = buffer;
-}
-
-MediaBuffer *MediaBuffer::nextBuffer() {
-    return mNextBuffer;
-}
-
 int MediaBuffer::refcount() const {
     return mRefCount;
 }
diff --git a/media/libstagefright/foundation/MediaBufferGroup.cpp b/media/libstagefright/foundation/MediaBufferGroup.cpp
index 9022324..8be8e8b 100644
--- a/media/libstagefright/foundation/MediaBufferGroup.cpp
+++ b/media/libstagefright/foundation/MediaBufferGroup.cpp
@@ -23,20 +23,18 @@
 
 namespace android {
 
-MediaBufferGroup::MediaBufferGroup()
-    : mFirstBuffer(NULL),
-      mLastBuffer(NULL) {
+MediaBufferGroup::MediaBufferGroup(size_t growthLimit) :
+    mGrowthLimit(growthLimit) {
 }
 
 MediaBufferGroup::~MediaBufferGroup() {
-    MediaBuffer *next;
-    for (MediaBuffer *buffer = mFirstBuffer; buffer != NULL;
-         buffer = next) {
-        next = buffer->nextBuffer();
-
-        CHECK_EQ(buffer->refcount(), 0);
-
-        buffer->setObserver(NULL);
+    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.
+        buffer->setObserver(nullptr);
         buffer->release();
     }
 }
@@ -45,87 +43,92 @@
     Mutex::Autolock autoLock(mLock);
 
     buffer->setObserver(this);
+    mBuffers.emplace_back(buffer);
+    // optionally: mGrowthLimit = max(mGrowthLimit, mBuffers.size());
+}
 
-    if (mLastBuffer) {
-        mLastBuffer->setNextBuffer(buffer);
-    } else {
-        mFirstBuffer = buffer;
+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;
+        }
     }
-
-    mLastBuffer = buffer;
 }
 
 status_t MediaBufferGroup::acquire_buffer(
         MediaBuffer **out, bool nonBlocking, size_t requestedSize) {
     Mutex::Autolock autoLock(mLock);
-
     for (;;) {
-        MediaBuffer *freeBuffer = NULL;
-        MediaBuffer *freeBufferPrevious = NULL;
-        MediaBuffer *buffer = NULL;
-        MediaBuffer *bufferPrevious = NULL;
         size_t smallest = requestedSize;
-        for (buffer = mFirstBuffer;
-             buffer != NULL; buffer = buffer->nextBuffer()) {
-            if (buffer->refcount() == 0) {
-               if (buffer->size() >= requestedSize) {
-                   break;
-               } else if (buffer->size() < smallest) {
-                   freeBuffer = buffer;
-                   freeBufferPrevious = bufferPrevious;
-               }
+        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) {
+                    buffer = *it;
+                    break;
+                }
+                if (size < smallest) {
+                    smallest = size; // always free the smallest buf
+                    free = it;
+                }
             }
-            bufferPrevious = buffer;
         }
-
-        if (buffer == NULL && freeBuffer != NULL) {
-            ALOGV("allocate new buffer, requested size %zu vs available %zu",
-                    requestedSize, freeBuffer->size());
-            size_t allocateSize = requestedSize;
-            if (requestedSize < SIZE_MAX / 3) {
-                allocateSize = requestedSize * 3 / 2;
+        if (buffer == nullptr
+                && (free != mBuffers.end() || mBuffers.size() < mGrowthLimit)) {
+            // We alloc before we free so failure leaves group unchanged.
+            const size_t allocateSize = requestedSize < SIZE_MAX / 3 * 2 /* NB: ordering */ ?
+                    requestedSize * 3 / 2 : requestedSize;
+            buffer = new MediaBuffer(allocateSize);
+            if (buffer->data() == nullptr) {
+                ALOGE("Allocation failure for size %zu", allocateSize);
+                delete buffer; // Invalid alloc, prefer not to call release.
+                buffer = nullptr;
+            } else {
+                buffer->setObserver(this);
+                if (free != mBuffers.end()) {
+                    ALOGV("reallocate buffer, requested size %zu vs available %zu",
+                            requestedSize, (*free)->size());
+                    (*free)->setObserver(nullptr);
+                    (*free)->release();
+                    *free = buffer; // in-place replace
+                } else {
+                    ALOGV("allocate buffer, requested size %zu", requestedSize);
+                    mBuffers.emplace_back(buffer);
+                }
             }
-            MediaBuffer *newBuffer = new MediaBuffer(allocateSize);
-            newBuffer->setObserver(this);
-            if (freeBuffer == mFirstBuffer) {
-                mFirstBuffer = newBuffer;
-            }
-            if (freeBuffer == mLastBuffer) {
-                mLastBuffer = newBuffer;
-            }
-            newBuffer->setNextBuffer(freeBuffer->nextBuffer());
-            if (freeBufferPrevious != NULL) {
-                freeBufferPrevious->setNextBuffer(newBuffer);
-            }
-            freeBuffer->setObserver(NULL);
-            freeBuffer->release();
-
-            buffer = newBuffer;
         }
-
-        if (buffer != NULL) {
+        if (buffer != nullptr) {
             buffer->add_ref();
             buffer->reset();
-
             *out = buffer;
-            goto exit;
+            return OK;
         }
-
         if (nonBlocking) {
-            *out = NULL;
+            *out = nullptr;
             return WOULD_BLOCK;
         }
-
-        // All buffers are in use. Block until one of them is returned to us.
+        // All buffers are in use, block until one of them is returned.
         mCondition.wait(mLock);
     }
-
-exit:
-    return OK;
+    // Never gets here.
 }
 
 void MediaBufferGroup::signalBufferReturned(MediaBuffer *) {
-    Mutex::Autolock autoLock(mLock);
     mCondition.signal();
 }