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
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 @@
sp<MetaData> audioMeta = mSource->getFormatMeta(true /* audio */);
sp<AMessage> 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<AMessage> format = mSource->getFormat(true /*audio*/);
- openAudioSink(format, true /*offloadOnly*/);
+ tryOpenAudioSinkForOffload(format, hasVideo);
}
instantiateDecoder(true, &mAudioDecoder);
}
@@ -774,6 +775,8 @@
// 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 @@
mScanSourcesPending = true;
}
-void NuPlayer::openAudioSink(const sp<AMessage> &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;
- }
+void NuPlayer::tryOpenAudioSinkForOffload(const sp<AMessage> &format, bool hasVideo) {
+ // Note: This is called early in NuPlayer to determine whether offloading
+ // is possible; otherwise the decoders call the renderer openAudioSink directly.
- mOffloadAudio = mRenderer->openAudioSink(
- format, offloadOnly, hasVideo, flags);
-
- 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<MetaData> 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 @@
mFlushComplete[1][1] = false;
}
- void openAudioSink(const sp<AMessage> &format, bool offloadOnly);
+ void tryOpenAudioSinkForOffload(const sp<AMessage> &format, bool hasVideo);
void closeAudioSink();
status_t instantiateDecoder(bool audio, sp<DecoderBase> *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 @@
const sp<Renderer> &renderer,
const sp<NativeWindowWrapper> &nativeWindow,
const sp<CCDecoder> &ccDecoder)
- : mNotify(notify),
+ : DecoderBase(notify),
mNativeWindow(nativeWindow),
mSource(source),
mRenderer(renderer),
@@ -56,7 +56,6 @@
mIsVideoAVC(false),
mIsSecure(false),
mFormatChangePending(false),
- mBufferGeneration(0),
mPaused(true),
mResumePending(false),
mComponentName("decoder") {
@@ -336,20 +335,6 @@
}
}
-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<AMessage> 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 @@
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 @@
kWhatRenderBuffer = 'rndr',
};
- sp<AMessage> mNotify;
sp<NativeWindowWrapper> mNativeWindow;
sp<Source> mSource;
@@ -83,12 +82,10 @@
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<AMessage> ¬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::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<AMessage> 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 MediaBuffer;
struct NuPlayer::DecoderBase : public AHandler {
- DecoderBase();
+ DecoderBase(const sp<AMessage> ¬ify);
void configure(const sp<AMessage> &format);
void init();
@@ -71,6 +71,10 @@
void onRequestInputBuffers();
void scheduleRequestBuffers();
virtual void doRequestBuffers() = 0;
+ virtual void handleError(int32_t err);
+
+ sp<AMessage> 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 @@
const sp<AMessage> ¬ify,
const sp<Source> &source,
const sp<Renderer> &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 @@
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 @@
kWhatBufferConsumed = 'bufC',
};
- sp<AMessage> mNotify;
sp<Source> mSource;
sp<Renderer> 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 @@
mPauseStartedTimeRealUs = realUs;
}
-bool NuPlayer::Renderer::openAudioSink(
+status_t NuPlayer::Renderer::openAudioSink(
const sp<AMessage> &format,
bool offloadOnly,
bool hasVideo,
- uint32_t flags) {
+ uint32_t flags,
+ bool *isOffloaded) {
sp<AMessage> msg = new AMessage(kWhatOpenAudioSink, id());
msg->setMessage("format", format);
msg->setInt32("offload-only", offloadOnly);
@@ -264,9 +265,15 @@
sp<AMessage> 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 @@
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<AMessage> 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 @@
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 @@
}
}
-bool NuPlayer::Renderer::onOpenAudioSink(
+status_t NuPlayer::Renderer::onOpenAudioSink(
const sp<AMessage> &format,
bool offloadOnly,
bool hasVideo,
@@ -1383,7 +1392,7 @@
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 @@
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 @@
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 @@
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 @@
int64_t getVideoLateByUs();
void setPauseStartedTimeRealUs(int64_t realUs);
- bool openAudioSink(
+ status_t openAudioSink(
const sp<AMessage> &format,
bool offloadOnly,
bool hasVideo,
- uint32_t flags);
+ uint32_t flags,
+ bool *isOffloaded);
void closeAudioSink();
enum {
@@ -209,7 +210,7 @@
void onResume();
void onSetVideoFrameRate(float fps);
void onAudioOffloadTearDown(AudioOffloadTearDownReason reason);
- bool onOpenAudioSink(
+ status_t onOpenAudioSink(
const sp<AMessage> &format,
bool offloadOnly,
bool hasVideo,