From ba6218eae3dbcf3f962b3561b26374a214dbf5e2 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Tue, 5 Mar 2013 14:31:02 -0800 Subject: Correct MediaCodec + Surface behavior Assorted tweaks: - Allow signalEndOfInputStream() before ACodec is in Executing state (added message to two more states). - Return an error if signalEndOfInputStream() is called a second time on the same stream. - Require AndroidOpaque color format in createInputSurface(). - Disallow dequeueInputBuffer() after an input surface has been created (boolean flag in MediaCodec tracks it). - Discard input surface when encoder is re-configure()ed (drop OMXNodeInstance's ref when we go back to Loaded). Bug 7991062 Change-Id: Iff30f3036e14eb5a2f6536910dcf11aba33031ee --- media/libstagefright/ACodec.cpp | 39 +++++++++++++++--------- media/libstagefright/MediaCodec.cpp | 20 ++++++++---- media/libstagefright/omx/GraphicBufferSource.cpp | 25 +++++++++------ media/libstagefright/omx/GraphicBufferSource.h | 9 +++--- media/libstagefright/omx/OMXNodeInstance.cpp | 17 ++++++++--- 5 files changed, 69 insertions(+), 41 deletions(-) (limited to 'media/libstagefright') diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index 59fc45e..1a2eeb1 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -241,9 +241,6 @@ struct ACodec::ExecutingState : public ACodec::BaseState { // to fill with data. void resume(); - // Send EOS on input stream. - void onSignalEndOfInputStream(); - // Returns true iff input and output buffers are in play. bool active() const { return mActive; } @@ -3413,6 +3410,12 @@ bool ACodec::LoadedToIdleState::onMessageReceived(const sp &msg) { return true; } + case kWhatSignalEndOfInputStream: + { + mCodec->onSignalEndOfInputStream(); + return true; + } + default: return BaseState::onMessageReceived(msg); } @@ -3458,6 +3461,12 @@ bool ACodec::IdleToExecutingState::onMessageReceived(const sp &msg) { return true; } + case kWhatSignalEndOfInputStream: + { + mCodec->onSignalEndOfInputStream(); + return true; + } + default: return BaseState::onMessageReceived(msg); } @@ -3538,17 +3547,6 @@ void ACodec::ExecutingState::resume() { mActive = true; } -void ACodec::ExecutingState::onSignalEndOfInputStream() { - sp notify = mCodec->mNotify->dup(); - notify->setInt32("what", ACodec::kWhatSignaledInputEOS); - - status_t err = mCodec->mOMX->signalEndOfInputStream(mCodec->mNode); - if (err != OK) { - notify->setInt32("err", err); - } - notify->post(); -} - void ACodec::ExecutingState::stateEntered() { ALOGV("[%s] Now Executing", mCodec->mComponentName.c_str()); @@ -3640,7 +3638,7 @@ bool ACodec::ExecutingState::onMessageReceived(const sp &msg) { case ACodec::kWhatSignalEndOfInputStream: { - onSignalEndOfInputStream(); + mCodec->onSignalEndOfInputStream(); handled = true; break; } @@ -3678,6 +3676,17 @@ status_t ACodec::setParameters(const sp ¶ms) { return OK; } +void ACodec::onSignalEndOfInputStream() { + sp notify = mNotify->dup(); + notify->setInt32("what", ACodec::kWhatSignaledInputEOS); + + status_t err = mOMX->signalEndOfInputStream(mNode); + if (err != OK) { + notify->setInt32("err", err); + } + notify->post(); +} + bool ACodec::ExecutingState::onOMXEvent( OMX_EVENTTYPE event, OMX_U32 data1, OMX_U32 data2) { switch (event) { diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp index 79ea04c..0d89c0f 100644 --- a/media/libstagefright/MediaCodec.cpp +++ b/media/libstagefright/MediaCodec.cpp @@ -69,7 +69,8 @@ MediaCodec::MediaCodec(const sp &looper) mDequeueInputTimeoutGeneration(0), mDequeueInputReplyID(0), mDequeueOutputTimeoutGeneration(0), - mDequeueOutputReplyID(0) { + mDequeueOutputReplyID(0), + mHaveInputSurface(false) { } MediaCodec::~MediaCodec() { @@ -160,8 +161,6 @@ status_t MediaCodec::createInputSurface( sp* bufferProducer) { sp msg = new AMessage(kWhatCreateInputSurface, id()); - // TODO(fadden): require MediaFormat colorFormat == AndroidOpaque - sp response; status_t err = PostAndAwaitResponse(msg, &response); if (err == NO_ERROR) { @@ -256,8 +255,6 @@ status_t MediaCodec::queueSecureInputBuffer( } status_t MediaCodec::dequeueInputBuffer(size_t *index, int64_t timeoutUs) { - // TODO(fadden): fail if an input Surface has been configured - sp msg = new AMessage(kWhatDequeueInputBuffer, id()); msg->setInt64("timeoutUs", timeoutUs); @@ -604,6 +601,9 @@ void MediaCodec::onMessageReceived(const sp &msg) { CHECK_EQ(mState, CONFIGURING); setState(CONFIGURED); + // reset input surface flag + mHaveInputSurface = false; + (new AMessage)->postReply(mReplyID); break; } @@ -618,6 +618,7 @@ void MediaCodec::onMessageReceived(const sp &msg) { msg->findObject("input-surface", &obj); CHECK(obj != NULL); response->setObject("input-surface", obj); + mHaveInputSurface = true; } else { response->setInt32("err", err); } @@ -1029,10 +1030,17 @@ void MediaCodec::onMessageReceived(const sp &msg) { case kWhatDequeueInputBuffer: { - // TODO(fadden): make this fail if we're using an input Surface uint32_t replyID; CHECK(msg->senderAwaitsResponse(&replyID)); + if (mHaveInputSurface) { + ALOGE("dequeueInputBuffer can't be used with input surface"); + sp response = new AMessage; + response->setInt32("err", INVALID_OPERATION); + response->postReply(replyID); + break; + } + if (handleDequeueInputBuffer(replyID, true /* new request */)) { break; } diff --git a/media/libstagefright/omx/GraphicBufferSource.cpp b/media/libstagefright/omx/GraphicBufferSource.cpp index f207954..211e1d1 100644 --- a/media/libstagefright/omx/GraphicBufferSource.cpp +++ b/media/libstagefright/omx/GraphicBufferSource.cpp @@ -15,6 +15,7 @@ */ #define LOG_TAG "GraphicBufferSource" +//#define LOG_NDEBUG 0 #include #include @@ -110,15 +111,12 @@ void GraphicBufferSource::omxExecuting() { } } -void GraphicBufferSource::omxIdling(){ +void GraphicBufferSource::omxLoaded(){ Mutex::Autolock autoLock(mMutex); - ALOGV("--> idling"); - if (!mExecuting) { - // Transition from "loading" to "idling". Nothing to do. - return; - } + ALOGV("--> loaded"); + CHECK(mExecuting); - ALOGV("Dropped down to idle, avail=%d eos=%d eosSent=%d", + ALOGV("Dropped down to loaded, avail=%d eos=%d eosSent=%d", mNumFramesAvailable, mEndOfStream, mEndOfStreamSent); // Codec is no longer executing. Discard all codec-related state. @@ -282,10 +280,15 @@ status_t GraphicBufferSource::fillCodecBuffer_l() { return OK; } -void GraphicBufferSource::signalEndOfInputStream() { +status_t GraphicBufferSource::signalEndOfInputStream() { Mutex::Autolock autoLock(mMutex); - ALOGV("signalEndOfInputStream: exec=%d avail=%d", - mExecuting, mNumFramesAvailable); + ALOGV("signalEndOfInputStream: exec=%d avail=%d eos=%d", + mExecuting, mNumFramesAvailable, mEndOfStream); + + if (mEndOfStream) { + ALOGE("EOS was already signaled"); + return INVALID_OPERATION; + } // Set the end-of-stream flag. If no frames are pending from the // BufferQueue, and a codec buffer is available, and we're executing, @@ -300,6 +303,8 @@ void GraphicBufferSource::signalEndOfInputStream() { if (mExecuting && mNumFramesAvailable == 0) { submitEndOfInputStream_l(); } + + return OK; } status_t GraphicBufferSource::submitBuffer_l(sp& graphicBuffer, diff --git a/media/libstagefright/omx/GraphicBufferSource.h b/media/libstagefright/omx/GraphicBufferSource.h index 6d49f96..6a34bc5 100644 --- a/media/libstagefright/omx/GraphicBufferSource.h +++ b/media/libstagefright/omx/GraphicBufferSource.h @@ -67,10 +67,9 @@ public: // sitting in the BufferQueue, this will send them to the codec. void omxExecuting(); - // This is called when OMX transitions to OMX_StateIdle. If we were - // previously executing, this means we're about to be shut down. (We - // also enter Idle on the way up.) - void omxIdling(); + // This is called when OMX transitions to OMX_StateLoaded, indicating that + // we are shutting down. + void omxLoaded(); // A "codec buffer", i.e. a buffer that can be used to pass data into // the encoder, has been allocated. (This call does not call back into @@ -84,7 +83,7 @@ public: // This is called after the last input frame has been submitted. We // need to submit an empty buffer with the EOS flag set. If we don't // have a codec buffer ready, we just set the mEndOfStream flag. - void signalEndOfInputStream(); + status_t signalEndOfInputStream(); protected: // BufferQueue::ConsumerListener interface, called when a new frame of diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp index 6c2c33b..f3d8d14 100644 --- a/media/libstagefright/omx/OMXNodeInstance.cpp +++ b/media/libstagefright/omx/OMXNodeInstance.cpp @@ -584,6 +584,11 @@ status_t OMXNodeInstance::createInputSurface( mHandle, OMX_IndexParamPortDefinition, &def); CHECK(oerr == OMX_ErrorNone); + if (def.format.video.eColorFormat != OMX_COLOR_FormatAndroidOpaque) { + ALOGE("createInputSurface requires AndroidOpaque color format"); + return INVALID_OPERATION; + } + GraphicBufferSource* bufferSource = new GraphicBufferSource( this, def.format.video.nFrameWidth, def.format.video.nFrameHeight); if ((err = bufferSource->initCheck()) != OK) { @@ -602,11 +607,10 @@ status_t OMXNodeInstance::signalEndOfInputStream() { // flag set). Seems easier than doing the equivalent from here. sp bufferSource(getGraphicBufferSource()); if (bufferSource == NULL) { - ALOGW("signalEndOfInputStream should only be used with Surface input"); + ALOGW("signalEndOfInputStream can only be used with Surface input"); return INVALID_OPERATION; }; - bufferSource->signalEndOfInputStream(); - return OK; + return bufferSource->signalEndOfInputStream(); } status_t OMXNodeInstance::allocateBuffer( @@ -801,8 +805,11 @@ void OMXNodeInstance::onEvent( arg1 == OMX_CommandStateSet) { if (arg2 == OMX_StateExecuting) { bufferSource->omxExecuting(); - } else if (arg2 == OMX_StateIdle) { - bufferSource->omxIdling(); + } else if (arg2 == OMX_StateLoaded) { + // Must be shutting down -- won't have a GraphicBufferSource + // on the way up. + bufferSource->omxLoaded(); + setGraphicBufferSource(NULL); } } } -- cgit v1.1