am 4c56f8e0: am b65842dd: Merge "Fix NuPlayer assertion on failure to create AudioTrack" into lmp-mr1-dev

* commit '4c56f8e0329cbdd0f3406bde96fc6f0bba65664d':
  Fix NuPlayer assertion on failure to create AudioTrack

10 files changed, 88 insertions, 82 deletions

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<AMessage> &msg) {
                 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 @@ void NuPlayer::onMessageReceived(const sp<AMessage> &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<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;
-    }
-
-    mOffloadAudio = mRenderer->openAudioSink(
-            format, offloadOnly, hasVideo, flags);
+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.
 
-    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 c64b32f..bb32eac 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<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 @@ NuPlayer::Decoder::Decoder(
         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 @@ 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<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 @@ 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<AMessage> mNotify;
     sp<NativeWindowWrapper> mNativeWindow;
 
     sp<Source> 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<AMessage> &notify)
+    :  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<AMessage> &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<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 MediaCodec;
 struct MediaBuffer;
 
 struct NuPlayer::DecoderBase : public AHandler {
-    DecoderBase();
+    DecoderBase(const sp<AMessage> &notify);
 
     void configure(const sp<AMessage> &format);
     void init();
@@ -71,6 +71,10 @@ protected:
     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 @@ NuPlayer::DecoderPassThrough::DecoderPassThrough(
         const sp<AMessage> &notify,
         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 @@ void NuPlayer::DecoderPassThrough::onConfigure(const sp<AMessage> &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<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 @@ void NuPlayer::Renderer::setPauseStartedTimeRealUs(int64_t realUs) {
     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 @@ bool NuPlayer::Renderer::openAudioSink(
     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 @@ void NuPlayer::Renderer::onMessageReceived(const sp<AMessage> &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<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 @@ 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<AMessage> &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<AMessage> &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<AMessage> &format,
             bool offloadOnly,
             bool hasVideo,