From 0806340688c937e7b78c2d89db3809274130df4e Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Wed, 13 May 2015 11:51:18 -0700 Subject: stagefright: prevent more crashes in ACodec Signal errors if OMX or native window reports an error Bug: 20439174 Change-Id: Iebeb16f5a29c6819d39568a184b921799a234686 --- media/libstagefright/ACodec.cpp | 463 ++++++++++++++++++++++++++-------------- 1 file changed, 299 insertions(+), 164 deletions(-) (limited to 'media/libstagefright') diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index c2727e3..a36134f 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -1054,8 +1054,9 @@ status_t ACodec::submitOutputMetaDataBuffer() { return OK; BufferInfo *info = dequeueBufferFromNativeWindow(); - if (info == NULL) + if (info == NULL) { return ERROR_IO; + } ALOGV("[%s] submitting output meta buffer ID %u for graphic buffer %p", mComponentName.c_str(), info->mBufferID, info->mGraphicBuffer.get()); @@ -1069,6 +1070,32 @@ status_t ACodec::submitOutputMetaDataBuffer() { return err; } +// static +const char *ACodec::_asString(BufferInfo::Status s) { + switch (s) { + case BufferInfo::OWNED_BY_US: return "OUR"; + case BufferInfo::OWNED_BY_COMPONENT: return "COMPONENT"; + case BufferInfo::OWNED_BY_UPSTREAM: return "UPSTREAM"; + case BufferInfo::OWNED_BY_DOWNSTREAM: return "DOWNSTREAM"; + case BufferInfo::OWNED_BY_NATIVE_WINDOW: return "SURFACE"; + case BufferInfo::UNRECOGNIZED: return "UNRECOGNIZED"; + default: return "?"; + } +} + +void ACodec::dumpBuffers(OMX_U32 portIndex) { + CHECK(portIndex == kPortIndexInput || portIndex == kPortIndexOutput); + ALOGI("[%s] %s port has %zu buffers:", mComponentName.c_str(), + portIndex == kPortIndexInput ? "input" : "output", mBuffers[portIndex].size()); + for (size_t i = 0; i < mBuffers[portIndex].size(); ++i) { + const BufferInfo &info = mBuffers[portIndex][i]; + ALOGI(" slot %2zu: #%8u %p/%p %s(%d) dequeued:%u", + i, info.mBufferID, info.mGraphicBuffer.get(), + info.mGraphicBuffer == NULL ? NULL : info.mGraphicBuffer->getNativeBuffer(), + _asString(info.mStatus), info.mStatus, info.mDequeuedAt); + } +} + status_t ACodec::cancelBufferToNativeWindow(BufferInfo *info) { CHECK_EQ((int)info->mStatus, (int)BufferInfo::OWNED_BY_US); @@ -1080,7 +1107,7 @@ status_t ACodec::cancelBufferToNativeWindow(BufferInfo *info) { ALOGW_IF(err != 0, "[%s] can not return buffer %u to native window", mComponentName.c_str(), info->mBufferID); - + // change ownership even if cancelBuffer fails info->mStatus = BufferInfo::OWNED_BY_NATIVE_WINDOW; return err; @@ -1156,34 +1183,32 @@ ACodec::BufferInfo *ACodec::dequeueBufferFromNativeWindow() { } } - if (oldest) { - CHECK(mStoreMetaDataInOutputBuffers); - - // discard buffer in LRU info and replace with new buffer - oldest->mGraphicBuffer = new GraphicBuffer(buf, false); - oldest->mStatus = BufferInfo::OWNED_BY_US; - - mOMX->updateGraphicBufferInMeta( - mNode, kPortIndexOutput, oldest->mGraphicBuffer, - oldest->mBufferID); + // it is impossible dequeue a buffer when there are no buffers with ANW + CHECK(oldest != NULL); + // it is impossible to dequeue an unknown buffer in non-meta mode, as the + // while loop above does not complete + CHECK(mStoreMetaDataInOutputBuffers); - VideoDecoderOutputMetaData *metaData = - reinterpret_cast( - oldest->mData->base()); - CHECK_EQ(metaData->eType, kMetadataBufferTypeGrallocSource); + // discard buffer in LRU info and replace with new buffer + oldest->mGraphicBuffer = new GraphicBuffer(buf, false); + oldest->mStatus = BufferInfo::OWNED_BY_US; - ALOGV("replaced oldest buffer #%u with age %u (%p/%p stored in %p)", - (unsigned)(oldest - &mBuffers[kPortIndexOutput][0]), - mDequeueCounter - oldest->mDequeuedAt, - metaData->pHandle, - oldest->mGraphicBuffer->handle, oldest->mData->base()); + mOMX->updateGraphicBufferInMeta( + mNode, kPortIndexOutput, oldest->mGraphicBuffer, + oldest->mBufferID); - return oldest; - } + VideoDecoderOutputMetaData *metaData = + reinterpret_cast( + oldest->mData->base()); + CHECK_EQ(metaData->eType, kMetadataBufferTypeGrallocSource); - TRESPASS(); + ALOGV("replaced oldest buffer #%u with age %u (%p/%p stored in %p)", + (unsigned)(oldest - &mBuffers[kPortIndexOutput][0]), + mDequeueCounter - oldest->mDequeuedAt, + metaData->pHandle, + oldest->mGraphicBuffer->handle, oldest->mData->base()); - return NULL; + return oldest; } status_t ACodec::freeBuffersOnPort(OMX_U32 portIndex) { @@ -1195,8 +1220,8 @@ status_t ACodec::freeBuffersOnPort(OMX_U32 portIndex) { } } + // clear mDealer even on an error mDealer[portIndex].clear(); - return err; } @@ -1222,16 +1247,25 @@ status_t ACodec::freeOutputBuffersNotOwnedByComponent() { status_t ACodec::freeBuffer(OMX_U32 portIndex, size_t i) { BufferInfo *info = &mBuffers[portIndex].editItemAt(i); + status_t err = OK; - CHECK(info->mStatus == BufferInfo::OWNED_BY_US - || info->mStatus == BufferInfo::OWNED_BY_NATIVE_WINDOW); + switch (info->mStatus) { + case BufferInfo::OWNED_BY_US: + if (portIndex == kPortIndexOutput && mNativeWindow != NULL) { + (void)cancelBufferToNativeWindow(info); + } + // fall through - if (portIndex == kPortIndexOutput && mNativeWindow != NULL - && info->mStatus == BufferInfo::OWNED_BY_US) { - cancelBufferToNativeWindow(info); + case BufferInfo::OWNED_BY_NATIVE_WINDOW: + err = mOMX->freeBuffer(mNode, portIndex, info->mBufferID); + break; + + default: + ALOGE("trying to free buffer not owned by us or ANW (%d)", info->mStatus); + err = FAILED_TRANSACTION; + break; } - status_t err = mOMX->freeBuffer(mNode, portIndex, info->mBufferID); // remove buffer even if mOMX->freeBuffer fails mBuffers[portIndex].removeAt(i); @@ -1239,8 +1273,7 @@ status_t ACodec::freeBuffer(OMX_U32 portIndex, size_t i) { } ACodec::BufferInfo *ACodec::findBufferByID( - uint32_t portIndex, IOMX::buffer_id bufferID, - ssize_t *index) { + uint32_t portIndex, IOMX::buffer_id bufferID, ssize_t *index) { for (size_t i = 0; i < mBuffers[portIndex].size(); ++i) { BufferInfo *info = &mBuffers[portIndex].editItemAt(i); @@ -3955,14 +3988,10 @@ status_t ACodec::getPortFormat(OMX_U32 portIndex, sp ¬ify) { notify->setInt32("channel-count", 1); if (params.eAMRBandMode >= OMX_AUDIO_AMRBandModeWB0) { - notify->setString( - "mime", MEDIA_MIMETYPE_AUDIO_AMR_WB); - + notify->setString("mime", MEDIA_MIMETYPE_AUDIO_AMR_WB); notify->setInt32("sample-rate", 16000); } else { - notify->setString( - "mime", MEDIA_MIMETYPE_AUDIO_AMR_NB); - + notify->setString("mime", MEDIA_MIMETYPE_AUDIO_AMR_NB); notify->setInt32("sample-rate", 8000); } break; @@ -4107,7 +4136,7 @@ status_t ACodec::getPortFormat(OMX_U32 portIndex, sp ¬ify) { case OMX_AUDIO_CodingGSMFR: { - OMX_AUDIO_PARAM_MP3TYPE params; + OMX_AUDIO_PARAM_PCMMODETYPE params; InitOMXParams(¶ms); params.nPortIndex = portIndex; @@ -4119,7 +4148,7 @@ status_t ACodec::getPortFormat(OMX_U32 portIndex, sp ¬ify) { notify->setString("mime", MEDIA_MIMETYPE_AUDIO_MSGSM); notify->setInt32("channel-count", params.nChannels); - notify->setInt32("sample-rate", params.nSampleRate); + notify->setInt32("sample-rate", params.nSamplingRate); break; } @@ -4429,10 +4458,13 @@ bool ACodec::BaseState::onOMXEmptyBufferDone(IOMX::buffer_id bufferID) { ALOGV("[%s] onOMXEmptyBufferDone %u", mCodec->mComponentName.c_str(), bufferID); - BufferInfo *info = - mCodec->findBufferByID(kPortIndexInput, bufferID); - - CHECK_EQ((int)info->mStatus, (int)BufferInfo::OWNED_BY_COMPONENT); + BufferInfo *info = mCodec->findBufferByID(kPortIndexInput, bufferID); + BufferInfo::Status status = BufferInfo::getSafeStatus(info); + if (status != BufferInfo::OWNED_BY_COMPONENT) { + ALOGE("Wrong ownership in EBD: %s(%d) buffer #%u", _asString(status), status, bufferID); + mCodec->dumpBuffers(kPortIndexInput); + return false; + } info->mStatus = BufferInfo::OWNED_BY_US; // We're in "store-metadata-in-buffers" mode, the underlying @@ -4516,7 +4548,13 @@ void ACodec::BaseState::onInputBufferFilled(const sp &msg) { } BufferInfo *info = mCodec->findBufferByID(kPortIndexInput, bufferID); - CHECK_EQ((int)info->mStatus, (int)BufferInfo::OWNED_BY_UPSTREAM); + BufferInfo::Status status = BufferInfo::getSafeStatus(info); + if (status != BufferInfo::OWNED_BY_UPSTREAM) { + ALOGE("Wrong ownership in IBF: %s(%d) buffer #%u", _asString(status), status, bufferID); + mCodec->dumpBuffers(kPortIndexInput); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return; + } info->mStatus = BufferInfo::OWNED_BY_US; @@ -4555,7 +4593,13 @@ void ACodec::BaseState::onInputBufferFilled(const sp &msg) { bufferID, buffer.get(), info->mData.get()); - CHECK_LE(buffer->size(), info->mData->capacity()); + if (buffer->size() > info->mData->capacity()) { + ALOGE("data size (%zu) is greated than buffer capacity (%zu)", + buffer->size(), // this is the data received + info->mData->capacity()); // this is out buffer size + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return; + } memcpy(info->mData->data(), buffer->data(), buffer->size()); } @@ -4594,31 +4638,31 @@ void ACodec::BaseState::onInputBufferFilled(const sp &msg) { mCodec->submitOutputMetaDataBuffer(); } } - - CHECK_EQ(mCodec->mOMX->emptyBuffer( - mCodec->mNode, - bufferID, - 0, - buffer->size(), - flags, - timeUs), - (status_t)OK); - + status_t err2 = mCodec->mOMX->emptyBuffer( + mCodec->mNode, + bufferID, + 0, + buffer->size(), + flags, + timeUs); + if (err2 != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err2)); + return; + } info->mStatus = BufferInfo::OWNED_BY_COMPONENT; - if (!eos) { + if (!eos && err == OK) { getMoreInputDataIfPossible(); } else { - ALOGV("[%s] Signalled EOS on the input port", - mCodec->mComponentName.c_str()); + ALOGV("[%s] Signalled EOS (%d) on the input port", + mCodec->mComponentName.c_str(), err); mCodec->mPortEOS[kPortIndexInput] = true; mCodec->mInputEOSResult = err; } } else if (!mCodec->mPortEOS[kPortIndexInput]) { - if (err != ERROR_END_OF_STREAM) { - ALOGV("[%s] Signalling EOS on the input port " - "due to error %d", + if (err != OK && err != ERROR_END_OF_STREAM) { + ALOGV("[%s] Signalling EOS on the input port due to error %d", mCodec->mComponentName.c_str(), err); } else { ALOGV("[%s] Signalling EOS on the input port", @@ -4628,15 +4672,17 @@ void ACodec::BaseState::onInputBufferFilled(const sp &msg) { ALOGV("[%s] calling emptyBuffer %u signalling EOS", mCodec->mComponentName.c_str(), bufferID); - CHECK_EQ(mCodec->mOMX->emptyBuffer( - mCodec->mNode, - bufferID, - 0, - 0, - OMX_BUFFERFLAG_EOS, - 0), - (status_t)OK); - + status_t err2 = mCodec->mOMX->emptyBuffer( + mCodec->mNode, + bufferID, + 0, + 0, + OMX_BUFFERFLAG_EOS, + 0); + if (err2 != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err2)); + return; + } info->mStatus = BufferInfo::OWNED_BY_COMPONENT; mCodec->mPortEOS[kPortIndexInput] = true; @@ -4692,6 +4738,7 @@ bool ACodec::BaseState::onOMXFillBufferDone( mCodec->mComponentName.c_str(), bufferID, timeUs, flags); ssize_t index; + status_t err= OK; #if TRACK_BUFFER_TIMING index = mCodec->mBufferStats.indexOfKey(timeUs); @@ -4710,8 +4757,13 @@ bool ACodec::BaseState::onOMXFillBufferDone( BufferInfo *info = mCodec->findBufferByID(kPortIndexOutput, bufferID, &index); - - CHECK_EQ((int)info->mStatus, (int)BufferInfo::OWNED_BY_COMPONENT); + BufferInfo::Status status = BufferInfo::getSafeStatus(info); + if (status != BufferInfo::OWNED_BY_COMPONENT) { + ALOGE("Wrong ownership in FBD: %s(%d) buffer #%u", _asString(status), status, bufferID); + mCodec->dumpBuffers(kPortIndexOutput); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return true; + } info->mDequeuedAt = ++mCodec->mDequeueCounter; info->mStatus = BufferInfo::OWNED_BY_US; @@ -4729,9 +4781,11 @@ bool ACodec::BaseState::onOMXFillBufferDone( ALOGV("[%s] calling fillBuffer %u", mCodec->mComponentName.c_str(), info->mBufferID); - CHECK_EQ(mCodec->mOMX->fillBuffer( - mCodec->mNode, info->mBufferID), - (status_t)OK); + err = mCodec->mOMX->fillBuffer(mCodec->mNode, info->mBufferID); + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); + return true; + } info->mStatus = BufferInfo::OWNED_BY_COMPONENT; break; @@ -4794,8 +4848,11 @@ bool ACodec::BaseState::onOMXFillBufferDone( } case FREE_BUFFERS: - CHECK_EQ((status_t)OK, - mCodec->freeBuffer(kPortIndexOutput, index)); + err = mCodec->freeBuffer(kPortIndexOutput, index); + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); + return true; + } break; default: @@ -4810,9 +4867,14 @@ void ACodec::BaseState::onOutputBufferDrained(const sp &msg) { IOMX::buffer_id bufferID; CHECK(msg->findInt32("buffer-id", (int32_t*)&bufferID)); ssize_t index; - BufferInfo *info = - mCodec->findBufferByID(kPortIndexOutput, bufferID, &index); - CHECK_EQ((int)info->mStatus, (int)BufferInfo::OWNED_BY_DOWNSTREAM); + BufferInfo *info = mCodec->findBufferByID(kPortIndexOutput, bufferID, &index); + BufferInfo::Status status = BufferInfo::getSafeStatus(info); + if (status != BufferInfo::OWNED_BY_DOWNSTREAM) { + ALOGE("Wrong ownership in OBD: %s(%d) buffer #%u", _asString(status), status, bufferID); + mCodec->dumpBuffers(kPortIndexOutput); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return; + } android_native_rect_t crop; if (msg->findRect("crop", &crop.left, &crop.top, &crop.right, &crop.bottom)) { @@ -4843,13 +4905,11 @@ void ACodec::BaseState::onOutputBufferDrained(const sp &msg) { status_t err; err = native_window_set_buffers_timestamp(mCodec->mNativeWindow.get(), timestampNs); - if (err != OK) { - ALOGW("failed to set buffer timestamp: %d", err); - } + ALOGW_IF(err != NO_ERROR, "failed to set buffer timestamp: %d", err); - if ((err = mCodec->mNativeWindow->queueBuffer( - mCodec->mNativeWindow.get(), - info->mGraphicBuffer.get(), -1)) == OK) { + err = mCodec->mNativeWindow->queueBuffer( + mCodec->mNativeWindow.get(), info->mGraphicBuffer.get(), -1); + if (err == OK) { info->mStatus = BufferInfo::OWNED_BY_NATIVE_WINDOW; } else { mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); @@ -4892,11 +4952,12 @@ void ACodec::BaseState::onOutputBufferDrained(const sp &msg) { if (info != NULL) { ALOGV("[%s] calling fillBuffer %u", mCodec->mComponentName.c_str(), info->mBufferID); - - CHECK_EQ(mCodec->mOMX->fillBuffer(mCodec->mNode, info->mBufferID), - (status_t)OK); - - info->mStatus = BufferInfo::OWNED_BY_COMPONENT; + status_t err = mCodec->mOMX->fillBuffer(mCodec->mNode, info->mBufferID); + if (err == OK) { + info->mStatus = BufferInfo::OWNED_BY_COMPONENT; + } else { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); + } } } break; @@ -4904,8 +4965,10 @@ void ACodec::BaseState::onOutputBufferDrained(const sp &msg) { case FREE_BUFFERS: { - CHECK_EQ((status_t)OK, - mCodec->freeBuffer(kPortIndexOutput, index)); + status_t err = mCodec->freeBuffer(kPortIndexOutput, index); + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); + } break; } @@ -5158,7 +5221,7 @@ void ACodec::LoadedState::stateEntered() { void ACodec::LoadedState::onShutdown(bool keepComponentAllocated) { if (!keepComponentAllocated) { - CHECK_EQ(mCodec->mOMX->freeNode(mCodec->mNode), (status_t)OK); + (void)mCodec->mOMX->freeNode(mCodec->mNode); mCodec->changeState(mCodec->mUninitializedState); } @@ -5239,11 +5302,13 @@ bool ACodec::LoadedState::onConfigureComponent( CHECK(mCodec->mNode != 0); + status_t err = OK; AString mime; - CHECK(msg->findString("mime", &mime)); - - status_t err = mCodec->configureCodec(mime.c_str(), msg); - + if (!msg->findString("mime", &mime)) { + err = BAD_VALUE; + } else { + err = mCodec->configureCodec(mime.c_str(), msg); + } if (err != OK) { ALOGE("[%s] configureCodec returning error %d", mCodec->mComponentName.c_str(), err); @@ -5416,11 +5481,12 @@ void ACodec::LoadedState::onUsePersistentInputSurface( void ACodec::LoadedState::onStart() { ALOGV("onStart"); - CHECK_EQ(mCodec->mOMX->sendCommand( - mCodec->mNode, OMX_CommandStateSet, OMX_StateIdle), - (status_t)OK); - - mCodec->changeState(mCodec->mLoadedToIdleState); + status_t err = mCodec->mOMX->sendCommand(mCodec->mNode, OMX_CommandStateSet, OMX_StateIdle); + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); + } else { + mCodec->changeState(mCodec->mLoadedToIdleState); + } } //////////////////////////////////////////////////////////////////////////////// @@ -5494,14 +5560,25 @@ bool ACodec::LoadedToIdleState::onOMXEvent( switch (event) { case OMX_EventCmdComplete: { - CHECK_EQ(data1, (OMX_U32)OMX_CommandStateSet); - CHECK_EQ(data2, (OMX_U32)OMX_StateIdle); + status_t err = OK; + if (data1 != (OMX_U32)OMX_CommandStateSet + || data2 != (OMX_U32)OMX_StateIdle) { + ALOGE("Unexpected command completion in LoadedToIdleState: %s(%u) %s(%u)", + asString((OMX_COMMANDTYPE)data1), data1, + asString((OMX_STATETYPE)data2), data2); + err = FAILED_TRANSACTION; + } - CHECK_EQ(mCodec->mOMX->sendCommand( - mCodec->mNode, OMX_CommandStateSet, OMX_StateExecuting), - (status_t)OK); + if (err == OK) { + err = mCodec->mOMX->sendCommand( + mCodec->mNode, OMX_CommandStateSet, OMX_StateExecuting); + } - mCodec->changeState(mCodec->mIdleToExecutingState); + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); + } else { + mCodec->changeState(mCodec->mIdleToExecutingState); + } return true; } @@ -5562,8 +5639,14 @@ bool ACodec::IdleToExecutingState::onOMXEvent( switch (event) { case OMX_EventCmdComplete: { - CHECK_EQ(data1, (OMX_U32)OMX_CommandStateSet); - CHECK_EQ(data2, (OMX_U32)OMX_StateExecuting); + if (data1 != (OMX_U32)OMX_CommandStateSet + || data2 != (OMX_U32)OMX_StateExecuting) { + ALOGE("Unexpected command completion in IdleToExecutingState: %s(%u) %s(%u)", + asString((OMX_COMMANDTYPE)data1), data1, + asString((OMX_STATETYPE)data2), data2); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return true; + } mCodec->mExecutingState->resume(); mCodec->changeState(mCodec->mExecutingState); @@ -5605,28 +5688,43 @@ void ACodec::ExecutingState::submitOutputMetaBuffers() { } void ACodec::ExecutingState::submitRegularOutputBuffers() { + bool failed = false; for (size_t i = 0; i < mCodec->mBuffers[kPortIndexOutput].size(); ++i) { BufferInfo *info = &mCodec->mBuffers[kPortIndexOutput].editItemAt(i); if (mCodec->mNativeWindow != NULL) { - CHECK(info->mStatus == BufferInfo::OWNED_BY_US - || info->mStatus == BufferInfo::OWNED_BY_NATIVE_WINDOW); + if (info->mStatus != BufferInfo::OWNED_BY_US + && info->mStatus != BufferInfo::OWNED_BY_NATIVE_WINDOW) { + ALOGE("buffers should be owned by us or the surface"); + failed = true; + break; + } if (info->mStatus == BufferInfo::OWNED_BY_NATIVE_WINDOW) { continue; } } else { - CHECK_EQ((int)info->mStatus, (int)BufferInfo::OWNED_BY_US); + if (info->mStatus != BufferInfo::OWNED_BY_US) { + ALOGE("buffers should be owned by us"); + failed = true; + break; + } } - ALOGV("[%s] calling fillBuffer %u", - mCodec->mComponentName.c_str(), info->mBufferID); + ALOGV("[%s] calling fillBuffer %u", mCodec->mComponentName.c_str(), info->mBufferID); - CHECK_EQ(mCodec->mOMX->fillBuffer(mCodec->mNode, info->mBufferID), - (status_t)OK); + status_t err = mCodec->mOMX->fillBuffer(mCodec->mNode, info->mBufferID); + if (err != OK) { + failed = true; + break; + } info->mStatus = BufferInfo::OWNED_BY_COMPONENT; } + + if (failed) { + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + } } void ACodec::ExecutingState::submitOutputBuffers() { @@ -5638,9 +5736,7 @@ void ACodec::ExecutingState::submitOutputBuffers() { void ACodec::ExecutingState::resume() { if (mActive) { - ALOGV("[%s] We're already active, no need to resume.", - mCodec->mComponentName.c_str()); - + ALOGV("[%s] We're already active, no need to resume.", mCodec->mComponentName.c_str()); return; } @@ -5683,11 +5779,16 @@ bool ACodec::ExecutingState::onMessageReceived(const sp &msg) { mActive = false; - CHECK_EQ(mCodec->mOMX->sendCommand( - mCodec->mNode, OMX_CommandStateSet, OMX_StateIdle), - (status_t)OK); - - mCodec->changeState(mCodec->mExecutingToIdleState); + status_t err = mCodec->mOMX->sendCommand( + mCodec->mNode, OMX_CommandStateSet, OMX_StateIdle); + if (err != OK) { + if (keepComponentAllocated) { + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + } + // TODO: do some recovery here. + } else { + mCodec->changeState(mCodec->mExecutingToIdleState); + } handled = true; break; @@ -5705,11 +5806,13 @@ bool ACodec::ExecutingState::onMessageReceived(const sp &msg) { mActive = false; - CHECK_EQ(mCodec->mOMX->sendCommand( - mCodec->mNode, OMX_CommandFlush, OMX_ALL), - (status_t)OK); + status_t err = mCodec->mOMX->sendCommand(mCodec->mNode, OMX_CommandFlush, OMX_ALL); + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + } else { + mCodec->changeState(mCodec->mFlushingState); + } - mCodec->changeState(mCodec->mFlushingState); handled = true; break; } @@ -5960,25 +6063,34 @@ bool ACodec::OutputPortSettingsChangedState::onOMXEvent( case OMX_EventCmdComplete: { if (data1 == (OMX_U32)OMX_CommandPortDisable) { - CHECK_EQ(data2, (OMX_U32)kPortIndexOutput); + if (data2 != (OMX_U32)kPortIndexOutput) { + ALOGW("ignoring EventCmdComplete CommandPortDisable for port %u", data2); + return false; + } - ALOGV("[%s] Output port now disabled.", - mCodec->mComponentName.c_str()); + ALOGV("[%s] Output port now disabled.", mCodec->mComponentName.c_str()); - CHECK(mCodec->mBuffers[kPortIndexOutput].isEmpty()); - mCodec->mDealer[kPortIndexOutput].clear(); + status_t err = OK; + if (!mCodec->mBuffers[kPortIndexOutput].isEmpty()) { + ALOGE("disabled port should be empty, but has %zu buffers", + mCodec->mBuffers[kPortIndexOutput].size()); + err = FAILED_TRANSACTION; + } else { + mCodec->mDealer[kPortIndexOutput].clear(); + } - CHECK_EQ(mCodec->mOMX->sendCommand( - mCodec->mNode, OMX_CommandPortEnable, kPortIndexOutput), - (status_t)OK); + if (err == OK) { + err = mCodec->mOMX->sendCommand( + mCodec->mNode, OMX_CommandPortEnable, kPortIndexOutput); + } - status_t err; - if ((err = mCodec->allocateBuffersOnPort( - kPortIndexOutput)) != OK) { - ALOGE("Failed to allocate output port buffers after " - "port reconfiguration (error 0x%08x)", - err); + if (err == OK) { + err = mCodec->allocateBuffersOnPort(kPortIndexOutput); + ALOGE_IF(err != OK, "Failed to allocate output port buffers after port " + "reconfiguration: (%d)", err); + } + if (err != OK) { mCodec->signalError(OMX_ErrorUndefined, makeNoSideEffectStatus(err)); // This is technically not correct, but appears to be @@ -6000,8 +6112,7 @@ bool ACodec::OutputPortSettingsChangedState::onOMXEvent( mCodec->mSentFormat = false; - ALOGV("[%s] Output port now reenabled.", - mCodec->mComponentName.c_str()); + ALOGV("[%s] Output port now reenabled.", mCodec->mComponentName.c_str()); if (mCodec->mExecutingState->active()) { mCodec->mExecutingState->submitOutputBuffers(); @@ -6035,7 +6146,7 @@ bool ACodec::ExecutingToIdleState::onMessageReceived(const sp &msg) { { // Don't send me a flush request if you previously wanted me // to shutdown. - ALOGE("Got flush request in IdleToLoadedState"); + ALOGW("Ignoring flush request in ExecutingToIdleState"); break; } @@ -6067,8 +6178,14 @@ bool ACodec::ExecutingToIdleState::onOMXEvent( switch (event) { case OMX_EventCmdComplete: { - CHECK_EQ(data1, (OMX_U32)OMX_CommandStateSet); - CHECK_EQ(data2, (OMX_U32)OMX_StateIdle); + if (data1 != (OMX_U32)OMX_CommandStateSet + || data2 != (OMX_U32)OMX_StateIdle) { + ALOGE("Unexpected command completion in ExecutingToIdleState: %s(%u) %s(%u)", + asString((OMX_COMMANDTYPE)data1), data1, + asString((OMX_STATETYPE)data2), data2); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return true; + } mComponentNowIdle = true; @@ -6091,12 +6208,15 @@ bool ACodec::ExecutingToIdleState::onOMXEvent( void ACodec::ExecutingToIdleState::changeStateIfWeOwnAllBuffers() { if (mComponentNowIdle && mCodec->allYourBuffersAreBelongToUs()) { - CHECK_EQ(mCodec->mOMX->sendCommand( - mCodec->mNode, OMX_CommandStateSet, OMX_StateLoaded), - (status_t)OK); - - CHECK_EQ(mCodec->freeBuffersOnPort(kPortIndexInput), (status_t)OK); - CHECK_EQ(mCodec->freeBuffersOnPort(kPortIndexOutput), (status_t)OK); + status_t err = mCodec->mOMX->sendCommand( + mCodec->mNode, OMX_CommandStateSet, OMX_StateLoaded); + if (err == OK) { + err = mCodec->freeBuffersOnPort(kPortIndexInput); + status_t err2 = mCodec->freeBuffersOnPort(kPortIndexOutput); + if (err == OK) { + err = err2; + } + } if ((mCodec->mFlags & kFlagPushBlankBuffersToNativeWindowOnShutdown) && mCodec->mNativeWindow != NULL) { @@ -6107,6 +6227,11 @@ void ACodec::ExecutingToIdleState::changeStateIfWeOwnAllBuffers() { pushBlankBuffersToNativeWindow(mCodec->mNativeWindow.get()); } + if (err != OK) { + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return; + } + mCodec->changeState(mCodec->mIdleToLoadedState); } } @@ -6147,7 +6272,7 @@ bool ACodec::IdleToLoadedState::onMessageReceived(const sp &msg) { { // Don't send me a flush request if you previously wanted me // to shutdown. - ALOGW("Ignoring flush request in ExecutingToIdleState"); + ALOGE("Got flush request in IdleToLoadedState"); break; } @@ -6168,8 +6293,14 @@ bool ACodec::IdleToLoadedState::onOMXEvent( switch (event) { case OMX_EventCmdComplete: { - CHECK_EQ(data1, (OMX_U32)OMX_CommandStateSet); - CHECK_EQ(data2, (OMX_U32)OMX_StateLoaded); + if (data1 != (OMX_U32)OMX_CommandStateSet + || data2 != (OMX_U32)OMX_StateLoaded) { + ALOGE("Unexpected command completion in IdleToLoadedState: %s(%u) %s(%u)", + asString((OMX_COMMANDTYPE)data1), data1, + asString((OMX_STATETYPE)data2), data2); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return true; + } mCodec->changeState(mCodec->mLoadedState); @@ -6226,7 +6357,12 @@ bool ACodec::FlushingState::onOMXEvent( switch (event) { case OMX_EventCmdComplete: { - CHECK_EQ(data1, (OMX_U32)OMX_CommandFlush); + if (data1 != (OMX_U32)OMX_CommandFlush) { + ALOGE("unexpected EventCmdComplete %s(%d) data2:%d in FlushingState", + asString((OMX_COMMANDTYPE)data1), data1, data2); + mCodec->signalError(OMX_ErrorUndefined, FAILED_TRANSACTION); + return true; + } if (data2 == kPortIndexInput || data2 == kPortIndexOutput) { if (mFlushComplete[data2]) { @@ -6236,8 +6372,7 @@ bool ACodec::FlushingState::onOMXEvent( } mFlushComplete[data2] = true; - if (mFlushComplete[kPortIndexInput] - && mFlushComplete[kPortIndexOutput]) { + if (mFlushComplete[kPortIndexInput] && mFlushComplete[kPortIndexOutput]) { changeStateIfWeOwnAllBuffers(); } } else if (data2 == OMX_ALL) { -- cgit v1.1