HTTPStream and HTTPDataSource now support cancellation of the connection process, AwesomePlayer takes advantage of this in cases where ::reset() or ::suspend() is called while in the preparation phase to bail out early. Also fixes in issue where the audio codec was not properly stopped if no audio player object ever took ownership.

Change-Id: I6d73defe6d276693853a469db267bb2668d07af5
related-to-bugs: 2475845,2414536

8 files changed, 217 insertions, 56 deletions

diff --git a/include/media/stagefright/HTTPDataSource.h b/include/media/stagefright/HTTPDataSource.h
index 23522bd..b6176eb 100644
--- a/include/media/stagefright/HTTPDataSource.h
+++ b/include/media/stagefright/HTTPDataSource.h
@@ -20,6 +20,7 @@
 
 #include <media/stagefright/DataSource.h>
 #include <utils/String8.h>
+#include <utils/threads.h>
 
 namespace android {
 
@@ -35,6 +36,9 @@ public:
             const char *uri,
             const KeyedVector<String8, String8> *headers = NULL);
 
+    status_t connect();
+    void disconnect();
+
     virtual status_t initCheck() const;
 
     virtual ssize_t readAt(off_t offset, void *data, size_t size);
@@ -53,8 +57,21 @@ private:
         kBufferSize = 32 * 1024
     };
 
+    enum State {
+        DISCONNECTED,
+        CONNECTING,
+        CONNECTED
+    };
+
+    State mState;
+    mutable Mutex mStateLock;
+
     String8 mHeaders;
 
+    String8 mStartingHost;
+    String8 mStartingPath;
+    int mStartingPort;
+
     HTTPStream *mHttp;
     char *mHost;
     int mPort;
@@ -67,11 +84,7 @@ private:
     bool mContentLengthValid;
     unsigned long long mContentLength;
 
-    status_t mInitCheck;
-
-    void init(
-            const char *_host, int port, const char *_path,
-            const KeyedVector<String8, String8> *headers);
+    void init(const KeyedVector<String8, String8> *headers);
 
     ssize_t sendRangeRequest(size_t offset);
     void initHeaders(const KeyedVector<String8, String8> *overrides);
diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp
index e00ba47..10b7be3 100644
--- a/media/libstagefright/AwesomePlayer.cpp
+++ b/media/libstagefright/AwesomePlayer.cpp
@@ -26,6 +26,7 @@
 
 #include <binder/IPCThreadState.h>
 #include <media/stagefright/AudioPlayer.h>
+#include <media/stagefright/CachingDataSource.h>
 #include <media/stagefright/DataSource.h>
 #include <media/stagefright/FileSource.h>
 #include <media/stagefright/MediaBuffer.h>
@@ -318,6 +319,14 @@ void AwesomePlayer::reset() {
 }
 
 void AwesomePlayer::reset_l() {
+    if (mFlags & PREPARING) {
+        mFlags |= PREPARE_CANCELLED;
+        if (mConnectingDataSource != NULL) {
+            LOGI("interrupting the connection process");
+            mConnectingDataSource->disconnect();
+        }
+    }
+
     while (mFlags & PREPARING) {
         mPreparedCondition.wait(mLock);
     }
@@ -337,6 +346,12 @@ void AwesomePlayer::reset_l() {
     // If we did this later, audio would continue playing while we
     // shutdown the video-related resources and the player appear to
     // not be as responsive to a reset request.
+    if (mAudioPlayer == NULL && mAudioSource != NULL) {
+        // If we had an audio player, it would have effectively
+        // taken possession of the audio source and stopped it when
+        // _it_ is stopped. Otherwise this is still our responsibility.
+        mAudioSource->stop();
+    }
     mAudioSource.clear();
 
     if (mTimeSource != mAudioPlayer) {
@@ -1039,8 +1054,29 @@ status_t AwesomePlayer::prepareAsync_l() {
 }
 
 status_t AwesomePlayer::finishSetDataSource_l() {
-    sp<DataSource> dataSource =
-        DataSource::CreateFromURI(mUri.string(), &mUriHeaders);
+    sp<DataSource> dataSource;
+
+    if (!strncasecmp("http://", mUri.string(), 7)) {
+        mConnectingDataSource = new HTTPDataSource(mUri, &mUriHeaders);
+
+        mLock.unlock();
+        status_t err = mConnectingDataSource->connect();
+        mLock.lock();
+
+        if (err != OK) {
+            mConnectingDataSource.clear();
+
+            LOGI("mConnectingDataSource->connect() returned %d", err);
+            return err;
+        }
+
+        dataSource = new CachingDataSource(
+                mConnectingDataSource, 32 * 1024, 20);
+
+        mConnectingDataSource.clear();
+    } else {
+        dataSource = DataSource::CreateFromURI(mUri.string(), &mUriHeaders);
+    }
 
     if (dataSource == NULL) {
         return UNKNOWN_ERROR;
@@ -1067,7 +1103,7 @@ void AwesomePlayer::abortPrepare(status_t err) {
     }
 
     mPrepareResult = err;
-    mFlags &= ~PREPARING;
+    mFlags &= ~(PREPARING|PREPARE_CANCELLED);
     mAsyncPrepareEvent = NULL;
     mPreparedCondition.broadcast();
 }
@@ -1078,6 +1114,12 @@ void AwesomePlayer::onPrepareAsyncEvent() {
     {
         Mutex::Autolock autoLock(mLock);
 
+        if (mFlags & PREPARE_CANCELLED) {
+            LOGI("prepare was cancelled before doing anything");
+            abortPrepare(UNKNOWN_ERROR);
+            return;
+        }
+
         if (mUri.size() > 0) {
             status_t err = finishSetDataSource_l();
 
@@ -1109,7 +1151,19 @@ void AwesomePlayer::onPrepareAsyncEvent() {
     }
 
     if (prefetcher != NULL) {
+        {
+            Mutex::Autolock autoLock(mLock);
+            if (mFlags & PREPARE_CANCELLED) {
+                LOGI("prepare was cancelled before preparing the prefetcher");
+                abortPrepare(UNKNOWN_ERROR);
+                return;
+            }
+        }
+
+        LOGI("calling prefetcher->prepare()");
         prefetcher->prepare();
+        LOGV("prefetcher is done preparing");
+
         prefetcher.clear();
     }
 
@@ -1126,20 +1180,28 @@ void AwesomePlayer::onPrepareAsyncEvent() {
     }
 
     mPrepareResult = OK;
-    mFlags &= ~PREPARING;
+    mFlags &= ~(PREPARING|PREPARE_CANCELLED);
     mFlags |= PREPARED;
     mAsyncPrepareEvent = NULL;
     mPreparedCondition.broadcast();
 }
 
 status_t AwesomePlayer::suspend() {
-    LOGI("suspend");
+    LOGV("suspend");
     Mutex::Autolock autoLock(mLock);
 
     if (mSuspensionState != NULL) {
         return INVALID_OPERATION;
     }
 
+    if (mFlags & PREPARING) {
+        mFlags |= PREPARE_CANCELLED;
+        if (mConnectingDataSource != NULL) {
+            LOGI("interrupting the connection process");
+            mConnectingDataSource->disconnect();
+        }
+    }
+
     while (mFlags & PREPARING) {
         mPreparedCondition.wait(mLock);
     }
@@ -1180,7 +1242,7 @@ status_t AwesomePlayer::suspend() {
 }
 
 status_t AwesomePlayer::resume() {
-    LOGI("resume");
+    LOGV("resume");
     Mutex::Autolock autoLock(mLock);
 
     if (mSuspensionState == NULL) {
diff --git a/media/libstagefright/DataSource.cpp b/media/libstagefright/DataSource.cpp
index 8468a07..284e3bc 100644
--- a/media/libstagefright/DataSource.cpp
+++ b/media/libstagefright/DataSource.cpp
@@ -101,8 +101,11 @@ sp<DataSource> DataSource::CreateFromURI(
     if (!strncasecmp("file://", uri, 7)) {
         source = new FileSource(uri + 7);
     } else if (!strncasecmp("http://", uri, 7)) {
-        source = new HTTPDataSource(uri, headers);
-        source = new CachingDataSource(source, 32 * 1024, 20);
+        sp<HTTPDataSource> httpSource = new HTTPDataSource(uri, headers);
+        if (httpSource->connect() != OK) {
+            return NULL;
+        }
+        source = new CachingDataSource(httpSource, 32 * 1024, 20);
     } else {
         // Assume it's a filename.
         source = new FileSource(uri);
diff --git a/media/libstagefright/HTTPDataSource.cpp b/media/libstagefright/HTTPDataSource.cpp
index d79c1bd..451fc26 100644
--- a/media/libstagefright/HTTPDataSource.cpp
+++ b/media/libstagefright/HTTPDataSource.cpp
@@ -126,41 +126,71 @@ HTTPDataSource::HTTPDataSource(
         host = string(host, 0, colon - host.c_str());
     }
 
-    init(host.c_str(), port, path.c_str(), headers);
+    mStartingHost = host.c_str();
+    mStartingPath = path.c_str();
+    mStartingPort = port;
+
+    init(headers);
 }
 
 HTTPDataSource::HTTPDataSource(
         const char *_host, int port, const char *_path,
         const KeyedVector<String8, String8> *headers) {
-    init(_host, port, _path, headers);
+    mStartingHost = _host;
+    mStartingPath = _path;
+    mStartingPort = port;
+
+    init(headers);
 }
 
-void HTTPDataSource::init(
-        const char *_host, int port, const char *_path,
-        const KeyedVector<String8, String8> *headers) {
+void HTTPDataSource::init(const KeyedVector<String8, String8> *headers) {
+    mState = DISCONNECTED;
     mHttp = new HTTPStream;
+
     mHost = NULL;
     mPort = 0;
-    mPath = NULL,
+    mPath = NULL;
+
+    initHeaders(headers);
+
     mBuffer = malloc(kBufferSize);
     mBufferLength = 0;
     mBufferOffset = 0;
-    mContentLengthValid = false;
+}
 
-    initHeaders(headers);
+status_t HTTPDataSource::connect() {
+    {
+        Mutex::Autolock autoLock(mStateLock);
 
-    string host = _host;
-    string path = _path;
+        if (mState != DISCONNECTED) {
+            return ERROR_ALREADY_CONNECTED;
+        }
+
+        mState = CONNECTING;
+    }
+
+    mContentLengthValid = false;
+
+    string host = mStartingHost.string();
+    string path = mStartingPath.string();
+    int port = mStartingPort;
 
     LOGI("Connecting to host '%s', port %d, path '%s'",
          host.c_str(), port, path.c_str());
 
     int numRedirectsRemaining = 5;
     do {
-        mInitCheck = mHttp->connect(host.c_str(), port);
+        status_t err = mHttp->connect(host.c_str(), port);
 
-        if (mInitCheck != OK) {
-            return;
+        if (err != OK) {
+            Mutex::Autolock autoLock(mStateLock);
+
+            if (mState != CONNECTING) {
+                LOGV("connect() cancelled");
+            }
+            mState = DISCONNECTED;
+
+            return err;
         }
     } while (PerformRedirectIfNecessary(mHttp, mHeaders, &host, &path, &port)
              && numRedirectsRemaining-- > 0);
@@ -175,17 +205,44 @@ void HTTPDataSource::init(
     mHost = strdup(host.c_str());
     mPort = port;
     mPath = strdup(path.c_str());
+
+    Mutex::Autolock autoLock(mStateLock);
+
+    if (mState != CONNECTING) {
+        // disconnect was called when we had just successfully connected.
+        LOGV("connect() cancelled (we had just succeeded connecting)");
+
+        mHttp->disconnect();
+        return UNKNOWN_ERROR;
+    }
+
+    mState = CONNECTED;
+    return OK;
+}
+
+void HTTPDataSource::disconnect() {
+    Mutex::Autolock autoLock(mStateLock);
+
+    if (mState == CONNECTING || mState == CONNECTED) {
+        mHttp->disconnect();
+        mState = DISCONNECTED;
+    }
 }
 
 status_t HTTPDataSource::initCheck() const {
-    return mInitCheck;
+    Mutex::Autolock autoLock(mStateLock);
+
+    return (mState == CONNECTED) ? (status_t)OK : ERROR_NOT_CONNECTED;
 }
 
 status_t HTTPDataSource::getSize(off_t *size) {
     *size = 0;
 
-    if (mInitCheck != OK) {
-        return mInitCheck;
+    {
+        Mutex::Autolock autoLock(mStateLock);
+        if (mState != CONNECTED) {
+            return ERROR_NOT_CONNECTED;
+        }
     }
 
     if (!mContentLengthValid) {
@@ -198,7 +255,10 @@ status_t HTTPDataSource::getSize(off_t *size) {
 }
 
 HTTPDataSource::~HTTPDataSource() {
-    mHttp->disconnect();
+    disconnect();
+
+    delete mHttp;
+    mHttp = NULL;
 
     free(mBuffer);
     mBuffer = NULL;
@@ -212,9 +272,6 @@ HTTPDataSource::~HTTPDataSource() {
         free(mHost);
         mHost = NULL;
     }
-
-    delete mHttp;
-    mHttp = NULL;
 }
 
 ssize_t HTTPDataSource::sendRangeRequest(size_t offset) {
@@ -271,6 +328,13 @@ ssize_t HTTPDataSource::sendRangeRequest(size_t offset) {
 ssize_t HTTPDataSource::readAt(off_t offset, void *data, size_t size) {
     LOGV("readAt %ld, size %d", offset, size);
 
+    {
+        Mutex::Autolock autoLock(mStateLock);
+        if (mState != CONNECTED) {
+            return ERROR_NOT_CONNECTED;
+        }
+    }
+
     if (offset >= mBufferOffset
             && offset < (off_t)(mBufferOffset + mBufferLength)) {
         size_t num_bytes_available = mBufferLength - (offset - mBufferOffset);
@@ -304,7 +368,7 @@ ssize_t HTTPDataSource::readAt(off_t offset, void *data, size_t size) {
     mBufferOffset = offset;
 
     if (mContentLengthValid
-            && mBufferOffset + contentLength >= mContentLength) {
+            && mBufferOffset + contentLength >= (off_t)mContentLength) {
         // If we never triggered a range request but know the content length,
         // make sure to not read more data than there could be, otherwise
         // we'd block indefinitely if the server doesn't close the connection.
diff --git a/media/libstagefright/HTTPStream.cpp b/media/libstagefright/HTTPStream.cpp
index 2c5da68..66eadf6 100644
--- a/media/libstagefright/HTTPStream.cpp
+++ b/media/libstagefright/HTTPStream.cpp
@@ -44,12 +44,19 @@ HTTPStream::~HTTPStream() {
 }
 
 status_t HTTPStream::connect(const char *server, int port) {
+    Mutex::Autolock autoLock(mLock);
+
     status_t err = OK;
 
     if (mState == CONNECTED) {
         return ERROR_ALREADY_CONNECTED;
     }
 
+    struct hostent *ent = gethostbyname(server);
+    if (ent == NULL) {
+        return ERROR_UNKNOWN_HOST;
+    }
+
     CHECK_EQ(mSocket, -1);
     mSocket = socket(AF_INET, SOCK_STREAM, 0);
 
@@ -57,11 +64,11 @@ status_t HTTPStream::connect(const char *server, int port) {
         return UNKNOWN_ERROR;
     }
 
-    struct hostent *ent = gethostbyname(server);
-    if (ent == NULL) {
-        err = ERROR_UNKNOWN_HOST;
-        goto exit1;
-    }
+    mState = CONNECTING;
+
+    int s = mSocket;
+
+    mLock.unlock();
 
     struct sockaddr_in addr;
     addr.sin_family = AF_INET;
@@ -69,24 +76,31 @@ status_t HTTPStream::connect(const char *server, int port) {
     addr.sin_addr.s_addr = *(in_addr_t *)ent->h_addr;
     memset(addr.sin_zero, 0, sizeof(addr.sin_zero));
 
-    if (::connect(mSocket, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
-        err = ERROR_CANNOT_CONNECT;
-        goto exit1;
+    int res = ::connect(s, (const struct sockaddr *)&addr, sizeof(addr));
+
+    mLock.lock();
+
+    if (mState != CONNECTING) {
+        return UNKNOWN_ERROR;
     }
 
-    mState = CONNECTED;
+    if (res < 0) {
+        close(mSocket);
+        mSocket = -1;
 
-    return OK;
+        mState = READY;
+        return UNKNOWN_ERROR;
+    }
 
-exit1:
-    close(mSocket);
-    mSocket = -1;
+    mState = CONNECTED;
 
-    return err;
+    return OK;
 }
 
 status_t HTTPStream::disconnect() {
-    if (mState != CONNECTED) {
+    Mutex::Autolock autoLock(mLock);
+
+    if (mState != CONNECTED && mState != CONNECTING) {
         return ERROR_NOT_CONNECTED;
     }
 
diff --git a/media/libstagefright/StagefrightMetadataRetriever.cpp b/media/libstagefright/StagefrightMetadataRetriever.cpp
index c0cf97c..4679207 100644
--- a/media/libstagefright/StagefrightMetadataRetriever.cpp
+++ b/media/libstagefright/StagefrightMetadataRetriever.cpp
@@ -23,7 +23,6 @@
 #include <media/stagefright/ColorConverter.h>
 #include <media/stagefright/DataSource.h>
 #include <media/stagefright/FileSource.h>
-#include <media/stagefright/HTTPDataSource.h>
 #include <media/stagefright/MediaDebug.h>
 #include <media/stagefright/MediaExtractor.h>
 #include <media/stagefright/MetaData.h>
diff --git a/media/libstagefright/include/AwesomePlayer.h b/media/libstagefright/include/AwesomePlayer.h
index 7106524..32c28c1 100644
--- a/media/libstagefright/include/AwesomePlayer.h
+++ b/media/libstagefright/include/AwesomePlayer.h
@@ -22,6 +22,7 @@
 
 #include <media/MediaPlayerInterface.h>
 #include <media/stagefright/DataSource.h>
+#include <media/stagefright/HTTPDataSource.h>
 #include <media/stagefright/OMXClient.h>
 #include <utils/threads.h>
 
@@ -87,12 +88,13 @@ private:
     friend struct AwesomeEvent;
 
     enum Flags {
-        PLAYING     = 1,
-        LOOPING     = 2,
-        FIRST_FRAME = 4,
-        PREPARING   = 8,
-        PREPARED    = 16,
-        AT_EOS      = 32,
+        PLAYING             = 1,
+        LOOPING             = 2,
+        FIRST_FRAME         = 4,
+        PREPARING           = 8,
+        PREPARED            = 16,
+        AT_EOS              = 32,
+        PREPARE_CANCELLED   = 64,
     };
 
     mutable Mutex mLock;
@@ -160,6 +162,7 @@ private:
     MediaBuffer *mVideoBuffer;
 
     sp<Prefetcher> mPrefetcher;
+    sp<HTTPDataSource> mConnectingDataSource;
 
     struct SuspensionState {
         String8 mUri;
diff --git a/media/libstagefright/include/HTTPStream.h b/media/libstagefright/include/HTTPStream.h
index 43ef614..5d638f3 100644
--- a/media/libstagefright/include/HTTPStream.h
+++ b/media/libstagefright/include/HTTPStream.h
@@ -24,6 +24,7 @@
 
 #include <media/stagefright/MediaErrors.h>
 #include <utils/KeyedVector.h>
+#include <utils/threads.h>
 
 namespace android {
 
@@ -54,10 +55,12 @@ public:
 private:
     enum State {
         READY,
+        CONNECTING,
         CONNECTED
     };
 
     State mState;
+    Mutex mLock;
     int mSocket;
 
     KeyedVector<string, string> mHeaders;