stagefright: explicitly release extractor in StagefrightMetadataRetriever

Extractor dtor on media.extractor side happens asynchronously with the
dtor of StagefrightMetadataRetriever. If the extractor calls back into
the DataSource after mediaserver side released the reference to it, the
binder transaction could get stuck.

Adding an explicity release() and call it before StagefrightMetadataRetriever
continues to destruct the DataSource. Only implement it for MPEG4Extractor
to avoid impacting other extractors at this stage.

Also adding some code to aggressively release the extractor and data
source in HeifDecoderImpl to free up memory.

bug: 65463215

Change-Id: I84c442a1e010dd37a976af5453a353e43f672e22

diff --git a/media/libheif/HeifDecoderImpl.cpp b/media/libheif/HeifDecoderImpl.cpp
index 115baff..4b131a7 100644 (file)
--- a/media/libheif/HeifDecoderImpl.cpp
+++ b/media/libheif/HeifDecoderImpl.cpp
@@ -401,6 +401,10 @@ bool HeifDecoderImpl::decode(HeifFrameInfo* frameInfo) {
                 videoFrame->getFlattenedIccData());
     }
     mFrameDecoded = true;
+
+    // Aggressive clear to avoid holding on to resources
+    mRetriever.clear();
+    mDataSource.clear();
     return true;
 }
 

diff --git a/media/libmedia/IMediaExtractor.cpp b/media/libmedia/IMediaExtractor.cpp
index 19b00f3..3c43a72 100644 (file)
--- a/media/libmedia/IMediaExtractor.cpp
+++ b/media/libmedia/IMediaExtractor.cpp
@@ -38,7 +38,8 @@ enum {
     SETMEDIACAS,
     SETUID,
     NAME,
-    GETMETRICS
+    GETMETRICS,
+    RELEASE,
 };
 
 class BpMediaExtractor : public BpInterface<IMediaExtractor> {
@@ -138,6 +139,13 @@ public:
         ALOGV("name NOT IMPLEMENTED");
         return NULL;
     }
+
+    virtual void release() {
+        ALOGV("release");
+        Parcel data, reply;
+        data.writeInterfaceToken(BpMediaExtractor::getInterfaceDescriptor());
+        remote()->transact(RELEASE, data, &reply);
+    }
 };
 
 IMPLEMENT_META_INTERFACE(MediaExtractor, "android.media.IMediaExtractor");
@@ -215,6 +223,12 @@ status_t BnMediaExtractor::onTransact(
             reply->writeInt32(setMediaCas(casToken));
             return OK;
         }
+        case RELEASE: {
+            ALOGV("release");
+            CHECK_INTERFACE(IMediaExtractor, data, reply);
+            release();
+            return OK;
+        }
         default:
             return BBinder::onTransact(code, data, reply, flags);
     }

diff --git a/media/libmedia/include/media/IMediaExtractor.h b/media/libmedia/include/media/IMediaExtractor.h
index 1e13b65..0ac7673 100644 (file)
--- a/media/libmedia/include/media/IMediaExtractor.h
+++ b/media/libmedia/include/media/IMediaExtractor.h
@@ -68,6 +68,8 @@ public:
     virtual void setUID(uid_t uid)  = 0;
 
     virtual const char * name() = 0;
+
+    virtual void release() = 0;
 };
 
 

diff --git a/media/libstagefright/MPEG4Extractor.cpp b/media/libstagefright/MPEG4Extractor.cpp
index 5ef0f56..eedb2f5 100644 (file)
--- a/media/libstagefright/MPEG4Extractor.cpp
+++ b/media/libstagefright/MPEG4Extractor.cpp
@@ -354,6 +354,10 @@ MPEG4Extractor::MPEG4Extractor(const sp<DataSource> &source)
 }
 
 MPEG4Extractor::~MPEG4Extractor() {
+    release();
+}
+
+void MPEG4Extractor::release() {
     Track *track = mFirstTrack;
     while (track) {
         Track *next = track->next;
@@ -375,6 +379,12 @@ MPEG4Extractor::~MPEG4Extractor() {
     for (size_t i = 0; i < mPssh.size(); i++) {
         delete [] mPssh[i].data;
     }
+    mPssh.clear();
+
+    if (mDataSource != NULL) {
+        mDataSource->close();
+        mDataSource.clear();
+    }
 }
 
 uint32_t MPEG4Extractor::flags() const {

diff --git a/media/libstagefright/StagefrightMetadataRetriever.cpp b/media/libstagefright/StagefrightMetadataRetriever.cpp
index a53897f..103da95 100644 (file)
--- a/media/libstagefright/StagefrightMetadataRetriever.cpp
+++ b/media/libstagefright/StagefrightMetadataRetriever.cpp
@@ -60,6 +60,12 @@ StagefrightMetadataRetriever::StagefrightMetadataRetriever()
 StagefrightMetadataRetriever::~StagefrightMetadataRetriever() {
     ALOGV("~StagefrightMetadataRetriever()");
     clearMetadata();
+    // Explicitly release extractor before continuing with the destructor,
+    // some extractors might need to callback to close off the DataSource
+    // and we need to make sure it's still there.
+    if (mExtractor != NULL) {
+        mExtractor->release();
+    }
     if (mSource != NULL) {
         mSource->close();
     }

diff --git a/media/libstagefright/include/MPEG4Extractor.h b/media/libstagefright/include/MPEG4Extractor.h
index 4a4c538..c9d7dde 100644 (file)
--- a/media/libstagefright/include/MPEG4Extractor.h
+++ b/media/libstagefright/include/MPEG4Extractor.h
@@ -62,6 +62,7 @@ public:
     virtual sp<MetaData> getMetaData();
     virtual uint32_t flags() const;
     virtual const char * name() { return "MPEG4Extractor"; }
+    virtual void release();
 
     // for DRM
     virtual char* getDrmTrackInfo(size_t trackID, int *len);

diff --git a/media/libstagefright/include/media/stagefright/MediaExtractor.h b/media/libstagefright/include/media/stagefright/MediaExtractor.h
index f12160b..6ec7eaf 100644 (file)
--- a/media/libstagefright/include/media/stagefright/MediaExtractor.h
+++ b/media/libstagefright/include/media/stagefright/MediaExtractor.h
@@ -72,6 +72,8 @@ public:
 
     virtual const char * name() { return "<unspecified>"; }
 
+    virtual void release() {}
+
 protected:
     MediaExtractor();
     virtual ~MediaExtractor();