From 6607b39baa05ee85a0857c3f95ff9224517b2abc Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Wed, 19 Oct 2011 21:14:13 -0700 Subject: [PATCH] Stagefright: idle OMX after ANW errors This change fixes an issue in Stagefright where the state of an OMXCodec object can get out of sync with the state of the OMX component. In particular, if one of the ANativeWindow functions failed and put the OMXCodec into the ERROR state, this would cause Stagefright to skip doing the Executing -> Idle transition. Without this transition the freeBuffersOnPort call would never be made, and the MediaBuffers would end up being leaked (which would also leak the Gralloc buffers they reference). Bug: 5333695 Change-Id: I85ea0cf92d18e7ef6d35c7d1e2a7b4e2c9745d34 --- include/media/IOMX.h | 3 +++ media/libmedia/IOMX.cpp | 26 +++++++++++++++++++ media/libstagefright/OMXCodec.cpp | 21 ++++++++++++++- media/libstagefright/include/OMX.h | 3 +++ .../libstagefright/include/OMXNodeInstance.h | 2 ++ media/libstagefright/omx/OMX.cpp | 6 +++++ media/libstagefright/omx/OMXNodeInstance.cpp | 8 ++++++ 7 files changed, 68 insertions(+), 1 deletion(-) diff --git a/include/media/IOMX.h b/include/media/IOMX.h index 02ad7030def55..c4cc947a8902b 100644 --- a/include/media/IOMX.h +++ b/include/media/IOMX.h @@ -78,6 +78,9 @@ public: node_id node, OMX_INDEXTYPE index, const void *params, size_t size) = 0; + virtual status_t getState( + node_id node, OMX_STATETYPE* state) = 0; + virtual status_t storeMetaDataInBuffers( node_id node, OMX_U32 port_index, OMX_BOOL enable) = 0; diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index d3aab08408222..7d2fbced3e5e3 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -38,6 +38,7 @@ enum { SET_PARAMETER, GET_CONFIG, SET_CONFIG, + GET_STATE, ENABLE_GRAPHIC_BUFFERS, USE_BUFFER, USE_GRAPHIC_BUFFER, @@ -198,6 +199,17 @@ public: return reply.readInt32(); } + virtual status_t getState( + node_id node, OMX_STATETYPE* state) { + Parcel data, reply; + data.writeInterfaceToken(IOMX::getInterfaceDescriptor()); + data.writeIntPtr((intptr_t)node); + remote()->transact(GET_STATE, data, &reply); + + *state = static_cast(reply.readInt32()); + return reply.readInt32(); + } + virtual status_t enableGraphicBuffers( node_id node, OMX_U32 port_index, OMX_BOOL enable) { Parcel data, reply; @@ -524,6 +536,20 @@ status_t BnOMX::onTransact( return NO_ERROR; } + case GET_STATE: + { + CHECK_INTERFACE(IOMX, data, reply); + + node_id node = (void*)data.readIntPtr(); + OMX_STATETYPE state = OMX_StateInvalid; + + status_t err = getState(node, &state); + reply->writeInt32(state); + reply->writeInt32(err); + + return NO_ERROR; + } + case ENABLE_GRAPHIC_BUFFERS: { CHECK_INTERFACE(IOMX, data, reply); diff --git a/media/libstagefright/OMXCodec.cpp b/media/libstagefright/OMXCodec.cpp index 7c34257a383d1..112877180c465 100755 --- a/media/libstagefright/OMXCodec.cpp +++ b/media/libstagefright/OMXCodec.cpp @@ -3600,11 +3600,24 @@ status_t OMXCodec::stop() { mAsyncCompletion.wait(mLock); } + bool isError = false; switch (mState) { case LOADED: - case ERROR: break; + case ERROR: + { + OMX_STATETYPE state = OMX_StateInvalid; + status_t err = mOMX->getState(mNode, &state); + CHECK_EQ(err, (status_t)OK); + + if (state != OMX_StateExecuting) { + break; + } + // else fall through to the idling code + isError = true; + } + case EXECUTING: { setState(EXECUTING_TO_IDLE); @@ -3639,6 +3652,12 @@ status_t OMXCodec::stop() { mAsyncCompletion.wait(mLock); } + if (isError) { + // We were in the ERROR state coming in, so restore that now + // that we've idled the OMX component. + setState(ERROR); + } + break; } diff --git a/media/libstagefright/include/OMX.h b/media/libstagefright/include/OMX.h index d54b1c18393ba..53e764fd504b9 100644 --- a/media/libstagefright/include/OMX.h +++ b/media/libstagefright/include/OMX.h @@ -59,6 +59,9 @@ public: node_id node, OMX_INDEXTYPE index, const void *params, size_t size); + virtual status_t getState( + node_id node, OMX_STATETYPE* state); + virtual status_t enableGraphicBuffers( node_id node, OMX_U32 port_index, OMX_BOOL enable); diff --git a/media/libstagefright/include/OMXNodeInstance.h b/media/libstagefright/include/OMXNodeInstance.h index 1ccf50dc53cd2..47ca5797f3d35 100644 --- a/media/libstagefright/include/OMXNodeInstance.h +++ b/media/libstagefright/include/OMXNodeInstance.h @@ -49,6 +49,8 @@ struct OMXNodeInstance { status_t getConfig(OMX_INDEXTYPE index, void *params, size_t size); status_t setConfig(OMX_INDEXTYPE index, const void *params, size_t size); + status_t getState(OMX_STATETYPE* state); + status_t enableGraphicBuffers(OMX_U32 portIndex, OMX_BOOL enable); status_t getGraphicBufferUsage(OMX_U32 portIndex, OMX_U32* usage); diff --git a/media/libstagefright/omx/OMX.cpp b/media/libstagefright/omx/OMX.cpp index 33d3f309e5e7a..3715fe92e84f9 100644 --- a/media/libstagefright/omx/OMX.cpp +++ b/media/libstagefright/omx/OMX.cpp @@ -303,6 +303,12 @@ status_t OMX::setConfig( index, params, size); } +status_t OMX::getState( + node_id node, OMX_STATETYPE* state) { + return findInstance(node)->getState( + state); +} + status_t OMX::enableGraphicBuffers( node_id node, OMX_U32 port_index, OMX_BOOL enable) { return findInstance(node)->enableGraphicBuffers(port_index, enable); diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index b612f899100c5..0ff398a72f397 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -266,6 +266,14 @@ status_t OMXNodeInstance::setConfig( return StatusFromOMXError(err); } +status_t OMXNodeInstance::getState(OMX_STATETYPE* state) { + Mutex::Autolock autoLock(mLock); + + OMX_ERRORTYPE err = OMX_GetState(mHandle, state); + + return StatusFromOMXError(err); +} + status_t OMXNodeInstance::enableGraphicBuffers( OMX_U32 portIndex, OMX_BOOL enable) { Mutex::Autolock autoLock(mLock);