From 202bce11a7f66f27e6dbb6d154ddc123aa62513d Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 3 Dec 2014 11:47:36 -0800 Subject: Fix NuPlayer assertion on failure to create AudioTrack Under heavy media load or monkey/stress testing, more than 32 AudioTracks may be created or memory resources may be scarce. Remove the assertion on failure to create AudioTrack and signal MEDIA_ERROR. Bug: 17319843 Change-Id: I5d4e200b5f50d800046851a33e035cdc6ff10075 --- media/libmediaplayerservice/nuplayer/NuPlayer.cpp | 39 ++++++++------------ media/libmediaplayerservice/nuplayer/NuPlayer.h | 2 +- .../nuplayer/NuPlayerDecoder.cpp | 27 +++++--------- .../nuplayer/NuPlayerDecoder.h | 3 -- .../nuplayer/NuPlayerDecoderBase.cpp | 20 +++++++++- .../nuplayer/NuPlayerDecoderBase.h | 6 ++- .../nuplayer/NuPlayerDecoderPassThrough.cpp | 21 +++++------ .../nuplayer/NuPlayerDecoderPassThrough.h | 2 - .../nuplayer/NuPlayerRenderer.cpp | 43 ++++++++++++++-------- .../nuplayer/NuPlayerRenderer.h | 7 ++-- 10 files changed, 88 insertions(+), 82 deletions(-) (limited to 'media') diff --git a/media/libmediaplayerservice/nuplayer/NuPlayer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayer.cpp index c69f74b..d433a4d 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayer.cpp @@ -629,15 +629,16 @@ void NuPlayer::onMessageReceived(const sp &msg) { sp audioMeta = mSource->getFormatMeta(true /* audio */); sp videoFormat = mSource->getFormat(false /* audio */); audio_stream_type_t streamType = mAudioSink->getAudioStreamType(); - bool canOffload = canOffloadStream(audioMeta, (videoFormat != NULL), - true /* is_streaming */, streamType); + const bool hasVideo = (videoFormat != NULL); + const bool canOffload = canOffloadStream( + audioMeta, hasVideo, true /* is_streaming */, streamType); if (canOffload) { if (!mOffloadAudio) { mRenderer->signalEnableOffloadAudio(); } // open audio sink early under offload mode. sp format = mSource->getFormat(true /*audio*/); - openAudioSink(format, true /*offloadOnly*/); + tryOpenAudioSinkForOffload(format, hasVideo); } instantiateDecoder(true, &mAudioDecoder); } @@ -774,6 +775,8 @@ void NuPlayer::onMessageReceived(const sp &msg) { // Decoder errors can be due to Source (e.g. from streaming), // or from decoding corrupted bitstreams, or from other decoder // MediaCodec operations (e.g. from an ongoing reset or seek). + // They may also be due to openAudioSink failure at + // decoder start or after a format change. // // We try to gracefully shut down the affected decoder if possible, // rather than trying to force the shutdown with something @@ -1146,28 +1149,16 @@ void NuPlayer::postScanSources() { mScanSourcesPending = true; } -void NuPlayer::openAudioSink(const sp &format, bool offloadOnly) { - uint32_t flags; - int64_t durationUs; - bool hasVideo = (mVideoDecoder != NULL); - // FIXME: we should handle the case where the video decoder - // is created after we receive the format change indication. - // Current code will just make that we select deep buffer - // with video which should not be a problem as it should - // not prevent from keeping A/V sync. - if (!hasVideo && - mSource->getDuration(&durationUs) == OK && - durationUs - > AUDIO_SINK_MIN_DEEP_BUFFER_DURATION_US) { - flags = AUDIO_OUTPUT_FLAG_DEEP_BUFFER; - } else { - flags = AUDIO_OUTPUT_FLAG_NONE; - } - - mOffloadAudio = mRenderer->openAudioSink( - format, offloadOnly, hasVideo, flags); +void NuPlayer::tryOpenAudioSinkForOffload(const sp &format, bool hasVideo) { + // Note: This is called early in NuPlayer to determine whether offloading + // is possible; otherwise the decoders call the renderer openAudioSink directly. - if (mOffloadAudio) { + status_t err = mRenderer->openAudioSink( + format, true /* offloadOnly */, hasVideo, AUDIO_OUTPUT_FLAG_NONE, &mOffloadAudio); + if (err != OK) { + // Any failure we turn off mOffloadAudio. + mOffloadAudio = false; + } else if (mOffloadAudio) { sp audioMeta = mSource->getFormatMeta(true /* audio */); sendMetaDataToHal(mAudioSink, audioMeta); diff --git a/media/libmediaplayerservice/nuplayer/NuPlayer.h b/media/libmediaplayerservice/nuplayer/NuPlayer.h index 6be38a4..1569816 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayer.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayer.h @@ -188,7 +188,7 @@ private: mFlushComplete[1][1] = false; } - void openAudioSink(const sp &format, bool offloadOnly); + void tryOpenAudioSinkForOffload(const sp &format, bool hasVideo); void closeAudioSink(); status_t instantiateDecoder(bool audio, sp *decoder); diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp index 012d33e..6ad28b5 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.cpp @@ -44,7 +44,7 @@ NuPlayer::Decoder::Decoder( const sp &renderer, const sp &nativeWindow, const sp &ccDecoder) - : mNotify(notify), + : DecoderBase(notify), mNativeWindow(nativeWindow), mSource(source), mRenderer(renderer), @@ -56,7 +56,6 @@ NuPlayer::Decoder::Decoder( mIsVideoAVC(false), mIsSecure(false), mFormatChangePending(false), - mBufferGeneration(0), mPaused(true), mResumePending(false), mComponentName("decoder") { @@ -336,20 +335,6 @@ void NuPlayer::Decoder::doRequestBuffers() { } } -void NuPlayer::Decoder::handleError(int32_t err) -{ - // We cannot immediately release the codec due to buffers still outstanding - // in the renderer. We signal to the player the error so it can shutdown/release the - // decoder after flushing and increment the generation to discard unnecessary messages. - - ++mBufferGeneration; - - sp notify = mNotify->dup(); - notify->setInt32("what", kWhatError); - notify->setInt32("err", err); - notify->post(); -} - bool NuPlayer::Decoder::handleAnInputBuffer() { if (mFormatChangePending) { return false; @@ -462,8 +447,14 @@ bool NuPlayer::Decoder::handleAnOutputBuffer() { flags = AUDIO_OUTPUT_FLAG_NONE; } - mRenderer->openAudioSink( - format, false /* offloadOnly */, hasVideo, flags); + res = mRenderer->openAudioSink( + format, false /* offloadOnly */, hasVideo, flags, NULL /* isOffloaded */); + if (res != OK) { + ALOGE("Failed to open AudioSink on format change for %s (err=%d)", + mComponentName.c_str(), res); + handleError(res); + return false; + } } return true; } else if (res == INFO_DISCONTINUITY) { diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.h b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.h index 2c08f0d..1bfa94f 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoder.h @@ -53,7 +53,6 @@ private: kWhatRenderBuffer = 'rndr', }; - sp mNotify; sp mNativeWindow; sp mSource; @@ -83,12 +82,10 @@ private: bool mIsSecure; bool mFormatChangePending; - int32_t mBufferGeneration; bool mPaused; bool mResumePending; AString mComponentName; - void handleError(int32_t err); bool handleAnInputBuffer(); bool handleAnOutputBuffer(); diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.cpp index 4164350..d56fc4d 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.cpp @@ -28,8 +28,10 @@ namespace android { -NuPlayer::DecoderBase::DecoderBase() - : mRequestInputBuffersPending(false) { +NuPlayer::DecoderBase::DecoderBase(const sp ¬ify) + : mNotify(notify), + mBufferGeneration(0), + mRequestInputBuffersPending(false) { // Every decoder has its own looper because MediaCodec operations // are blocking, but NuPlayer needs asynchronous operations. mDecoderLooper = new ALooper; @@ -180,5 +182,19 @@ void NuPlayer::DecoderBase::onMessageReceived(const sp &msg) { } } +void NuPlayer::DecoderBase::handleError(int32_t err) +{ + // We cannot immediately release the codec due to buffers still outstanding + // in the renderer. We signal to the player the error so it can shutdown/release the + // decoder after flushing and increment the generation to discard unnecessary messages. + + ++mBufferGeneration; + + sp notify = mNotify->dup(); + notify->setInt32("what", kWhatError); + notify->setInt32("err", err); + notify->post(); +} + } // namespace android diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.h b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.h index 5feb6a1..6732ff4 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderBase.h @@ -29,7 +29,7 @@ struct MediaCodec; struct MediaBuffer; struct NuPlayer::DecoderBase : public AHandler { - DecoderBase(); + DecoderBase(const sp ¬ify); void configure(const sp &format); void init(); @@ -71,6 +71,10 @@ protected: void onRequestInputBuffers(); void scheduleRequestBuffers(); virtual void doRequestBuffers() = 0; + virtual void handleError(int32_t err); + + sp mNotify; + int32_t mBufferGeneration; private: enum { diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp index 79b7a3c..9f7f09a 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.cpp @@ -43,12 +43,11 @@ NuPlayer::DecoderPassThrough::DecoderPassThrough( const sp ¬ify, const sp &source, const sp &renderer) - : mNotify(notify), + : DecoderBase(notify), mSource(source), mRenderer(renderer), mSkipRenderingUntilMediaTimeUs(-1ll), mPaused(false), - mBufferGeneration(0), mReachedEOS(true), mPendingAudioErr(OK), mPendingBuffersToDrain(0), @@ -75,17 +74,15 @@ void NuPlayer::DecoderPassThrough::onConfigure(const sp &format) { onRequestInputBuffers(); - uint32_t flags; - int64_t durationUs; - if (mSource->getDuration(&durationUs) == OK && - durationUs > AUDIO_SINK_MIN_DEEP_BUFFER_DURATION_US) { - flags = AUDIO_OUTPUT_FLAG_DEEP_BUFFER; - } else { - flags = AUDIO_OUTPUT_FLAG_NONE; + // The audio sink is already opened before the PassThrough decoder is created. + // Opening again might be relevant if decoder is instantiated after shutdown and + // format is different. + status_t err = mRenderer->openAudioSink( + format, true /* offloadOnly */, false /* hasVideo */, + AUDIO_OUTPUT_FLAG_NONE /* flags */, NULL /* isOffloaded */); + if (err != OK) { + handleError(err); } - - mRenderer->openAudioSink( - format, true /* offloadOnly */, false /* hasVideo */, flags); } void NuPlayer::DecoderPassThrough::onSetRenderer( diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.h b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.h index 37b28c8..a6e1faf 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerDecoderPassThrough.h @@ -52,13 +52,11 @@ private: kWhatBufferConsumed = 'bufC', }; - sp mNotify; sp mSource; sp mRenderer; int64_t mSkipRenderingUntilMediaTimeUs; bool mPaused; - int32_t mBufferGeneration; bool mReachedEOS; // Used by feedDecoderInputData to aggregate small buffers into diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp index f7d30a8..a1e1aec 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.cpp @@ -250,11 +250,12 @@ void NuPlayer::Renderer::setPauseStartedTimeRealUs(int64_t realUs) { mPauseStartedTimeRealUs = realUs; } -bool NuPlayer::Renderer::openAudioSink( +status_t NuPlayer::Renderer::openAudioSink( const sp &format, bool offloadOnly, bool hasVideo, - uint32_t flags) { + uint32_t flags, + bool *isOffloaded) { sp msg = new AMessage(kWhatOpenAudioSink, id()); msg->setMessage("format", format); msg->setInt32("offload-only", offloadOnly); @@ -264,9 +265,15 @@ bool NuPlayer::Renderer::openAudioSink( sp response; msg->postAndAwaitResponse(&response); - int32_t offload; - CHECK(response->findInt32("offload", &offload)); - return (offload != 0); + int32_t err; + if (!response->findInt32("err", &err)) { + err = INVALID_OPERATION; + } else if (err == OK && isOffloaded != NULL) { + int32_t offload; + CHECK(response->findInt32("offload", &offload)); + *isOffloaded = (offload != 0); + } + return err; } void NuPlayer::Renderer::closeAudioSink() { @@ -292,10 +299,11 @@ void NuPlayer::Renderer::onMessageReceived(const sp &msg) { uint32_t flags; CHECK(msg->findInt32("flags", (int32_t *)&flags)); - bool offload = onOpenAudioSink(format, offloadOnly, hasVideo, flags); + status_t err = onOpenAudioSink(format, offloadOnly, hasVideo, flags); sp response = new AMessage; - response->setInt32("offload", offload); + response->setInt32("err", err); + response->setInt32("offload", offloadingAudio()); uint32_t replyID; CHECK(msg->senderAwaitsResponse(&replyID)); @@ -651,8 +659,9 @@ bool NuPlayer::Renderer::onDrainAudioQueue() { ssize_t written = mAudioSink->write(entry->mBuffer->data() + entry->mOffset, copy); if (written < 0) { - // An error in AudioSink write is fatal here. - LOG_ALWAYS_FATAL("AudioSink write error(%zd) when writing %zu bytes", written, copy); + // An error in AudioSink write. Perhaps the AudioSink was not properly opened. + ALOGE("AudioSink write error(%zd) when writing %zu bytes", written, copy); + break; } entry->mOffset += written; @@ -1321,7 +1330,7 @@ void NuPlayer::Renderer::cancelAudioOffloadPauseTimeout() { } } -bool NuPlayer::Renderer::onOpenAudioSink( +status_t NuPlayer::Renderer::onOpenAudioSink( const sp &format, bool offloadOnly, bool hasVideo, @@ -1383,7 +1392,7 @@ bool NuPlayer::Renderer::onOpenAudioSink( if (memcmp(&mCurrentOffloadInfo, &offloadInfo, sizeof(offloadInfo)) == 0) { ALOGV("openAudioSink: no change in offload mode"); // no change from previous configuration, everything ok. - return offloadingAudio(); + return OK; } ALOGV("openAudioSink: try to open AudioSink in offload mode"); uint32_t offloadFlags = flags; @@ -1429,7 +1438,7 @@ bool NuPlayer::Renderer::onOpenAudioSink( audioSinkChanged = true; mAudioSink->close(); mCurrentOffloadInfo = AUDIO_INFO_INITIALIZER; - CHECK_EQ(mAudioSink->open( + status_t err = mAudioSink->open( sampleRate, numChannels, (audio_channel_mask_t)channelMask, @@ -1437,8 +1446,11 @@ bool NuPlayer::Renderer::onOpenAudioSink( 8 /* bufferCount */, NULL, NULL, - (audio_output_flags_t)pcmFlags), - (status_t)OK); + (audio_output_flags_t)pcmFlags); + if (err != OK) { + ALOGW("openAudioSink: non offloaded open failed status: %d", err); + return err; + } mAudioSink->start(); } if (audioSinkChanged) { @@ -1447,8 +1459,7 @@ bool NuPlayer::Renderer::onOpenAudioSink( if (offloadingAudio()) { mAudioOffloadTornDown = false; } - - return offloadingAudio(); + return OK; } void NuPlayer::Renderer::onCloseAudioSink() { diff --git a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h index 14ae924..406c64c 100644 --- a/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h +++ b/media/libmediaplayerservice/nuplayer/NuPlayerRenderer.h @@ -73,11 +73,12 @@ struct NuPlayer::Renderer : public AHandler { int64_t getVideoLateByUs(); void setPauseStartedTimeRealUs(int64_t realUs); - bool openAudioSink( + status_t openAudioSink( const sp &format, bool offloadOnly, bool hasVideo, - uint32_t flags); + uint32_t flags, + bool *isOffloaded); void closeAudioSink(); enum { @@ -209,7 +210,7 @@ private: void onResume(); void onSetVideoFrameRate(float fps); void onAudioOffloadTearDown(AudioOffloadTearDownReason reason); - bool onOpenAudioSink( + status_t onOpenAudioSink( const sp &format, bool offloadOnly, bool hasVideo, -- cgit v1.1