From 114c1b6553e3d2dc5f2780a76d92bb382234220c Mon Sep 17 00:00:00 2001 From: Lajos Molnar Date: Thu, 27 Mar 2014 16:44:23 -0700 Subject: DO NOT MERGE: Revert "mediaplayer: keep more buffers with the BufferQueue" This reverts commit b635b0e66b257ab442e230bca96afd5105cf6829. Bug: 13655631 --- include/media/stagefright/ACodec.h | 1 - media/libstagefright/ACodec.cpp | 47 +++++++++++++++++--------------------- media/libstagefright/OMXCodec.cpp | 38 ++++++++---------------------- 3 files changed, 30 insertions(+), 56 deletions(-) diff --git a/include/media/stagefright/ACodec.h b/include/media/stagefright/ACodec.h index 46c62dc..bf3a998 100644 --- a/include/media/stagefright/ACodec.h +++ b/include/media/stagefright/ACodec.h @@ -203,7 +203,6 @@ private: unsigned mDequeueCounter; bool mStoreMetaDataInOutputBuffers; int32_t mMetaDataBuffersToSubmit; - size_t mNumUndequeuedBuffers; int64_t mRepeatFrameDelayUs; int64_t mMaxPtsGapUs; diff --git a/media/libstagefright/ACodec.cpp b/media/libstagefright/ACodec.cpp index 14d99cf..9276818 100644 --- a/media/libstagefright/ACodec.cpp +++ b/media/libstagefright/ACodec.cpp @@ -638,33 +638,18 @@ status_t ACodec::configureOutputBuffersFromNativeWindow( return err; } - // FIXME: assume that surface is controlled by app (native window - // returns the number for the case when surface is not controlled by app) - (*minUndequeuedBuffers)++; - - - // Use conservative allocation while also trying to reduce starvation - // - // 1. allocate at least nBufferCountMin + minUndequeuedBuffers - that is the - // minimum needed for the consumer to be able to work - // 2. try to allocate two (2) additional buffers to reduce starvation from - // the consumer - for (OMX_U32 extraBuffers = 2; /* condition inside loop */; extraBuffers--) { - OMX_U32 newBufferCount = - def.nBufferCountMin + *minUndequeuedBuffers + extraBuffers; + // XXX: Is this the right logic to use? It's not clear to me what the OMX + // buffer counts refer to - how do they account for the renderer holding on + // to buffers? + if (def.nBufferCountActual < def.nBufferCountMin + *minUndequeuedBuffers) { + OMX_U32 newBufferCount = def.nBufferCountMin + *minUndequeuedBuffers; def.nBufferCountActual = newBufferCount; err = mOMX->setParameter( mNode, OMX_IndexParamPortDefinition, &def, sizeof(def)); - if (err == OK) { - *minUndequeuedBuffers += extraBuffers; - break; - } - - ALOGW("[%s] setting nBufferCountActual to %lu failed: %d", - mComponentName.c_str(), newBufferCount, err); - /* exit condition */ - if (extraBuffers == 0) { + if (err != OK) { + ALOGE("[%s] setting nBufferCountActual to %lu failed: %d", + mComponentName.c_str(), newBufferCount, err); return err; } } @@ -689,7 +674,6 @@ status_t ACodec::allocateOutputBuffersFromNativeWindow() { &bufferCount, &bufferSize, &minUndequeuedBuffers); if (err != 0) return err; - mNumUndequeuedBuffers = minUndequeuedBuffers; ALOGV("[%s] Allocating %lu buffers from a native window of size %lu on " "output port", @@ -755,7 +739,6 @@ status_t ACodec::allocateOutputMetaDataBuffers() { &bufferCount, &bufferSize, &minUndequeuedBuffers); if (err != 0) return err; - mNumUndequeuedBuffers = minUndequeuedBuffers; ALOGV("[%s] Allocating %lu meta buffers on output port", mComponentName.c_str(), bufferCount); @@ -2446,7 +2429,19 @@ void ACodec::waitUntilAllPossibleNativeWindowBuffersAreReturnedToUs() { return; } - while (countBuffersOwnedByNativeWindow() > mNumUndequeuedBuffers + int minUndequeuedBufs = 0; + status_t err = mNativeWindow->query( + mNativeWindow.get(), NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS, + &minUndequeuedBufs); + + if (err != OK) { + ALOGE("[%s] NATIVE_WINDOW_MIN_UNDEQUEUED_BUFFERS query failed: %s (%d)", + mComponentName.c_str(), strerror(-err), -err); + + minUndequeuedBufs = 0; + } + + while (countBuffersOwnedByNativeWindow() > (size_t)minUndequeuedBufs && dequeueBufferFromNativeWindow() != NULL) { // these buffers will be submitted as regular buffers; account for this if (mStoreMetaDataInOutputBuffers && mMetaDataBuffersToSubmit > 0) { diff --git a/media/libstagefright/OMXCodec.cpp b/media/libstagefright/OMXCodec.cpp index 79a9665..43736ad 100644 --- a/media/libstagefright/OMXCodec.cpp +++ b/media/libstagefright/OMXCodec.cpp @@ -92,7 +92,6 @@ static sp InstantiateSoftwareEncoder( #define CODEC_LOGI(x, ...) ALOGI("[%s] "x, mComponentName, ##__VA_ARGS__) #define CODEC_LOGV(x, ...) ALOGV("[%s] "x, mComponentName, ##__VA_ARGS__) -#define CODEC_LOGW(x, ...) ALOGW("[%s] "x, mComponentName, ##__VA_ARGS__) #define CODEC_LOGE(x, ...) ALOGE("[%s] "x, mComponentName, ##__VA_ARGS__) struct OMXCodecObserver : public BnOMXObserver { @@ -1778,40 +1777,21 @@ status_t OMXCodec::allocateOutputBuffersFromNativeWindow() { strerror(-err), -err); return err; } - // FIXME: assume that surface is controlled by app (native window - // returns the number for the case when surface is not controlled by app) - minUndequeuedBufs++; - - // Use conservative allocation while also trying to reduce starvation - // - // 1. allocate at least nBufferCountMin + minUndequeuedBuffers - that is the - // minimum needed for the consumer to be able to work - // 2. try to allocate two (2) additional buffers to reduce starvation from - // the consumer - CODEC_LOGI("OMX-buffers: min=%u actual=%u undeq=%d", - def.nBufferCountMin, def.nBufferCountActual, minUndequeuedBufs); - - for (OMX_U32 extraBuffers = 2; /* condition inside loop */; extraBuffers--) { - OMX_U32 newBufferCount = - def.nBufferCountMin + minUndequeuedBufs + extraBuffers; + + // XXX: Is this the right logic to use? It's not clear to me what the OMX + // buffer counts refer to - how do they account for the renderer holding on + // to buffers? + if (def.nBufferCountActual < def.nBufferCountMin + minUndequeuedBufs) { + OMX_U32 newBufferCount = def.nBufferCountMin + minUndequeuedBufs; def.nBufferCountActual = newBufferCount; err = mOMX->setParameter( mNode, OMX_IndexParamPortDefinition, &def, sizeof(def)); - - if (err == OK) { - minUndequeuedBufs += extraBuffers; - break; - } - - CODEC_LOGW("setting nBufferCountActual to %lu failed: %d", - newBufferCount, err); - /* exit condition */ - if (extraBuffers == 0) { + if (err != OK) { + CODEC_LOGE("setting nBufferCountActual to %lu failed: %d", + newBufferCount, err); return err; } } - CODEC_LOGI("OMX-buffers: min=%u actual=%u undeq=%d", - def.nBufferCountMin, def.nBufferCountActual, minUndequeuedBufs); err = native_window_set_buffer_count( mNativeWindow.get(), def.nBufferCountActual); -- cgit v1.1