Untangle MediaScanner error handling.

Bug: 5056917

Change-Id: I1a7a73579e3ba4e9709459329fc1901a28b0f4b1

5 files changed, 176 insertions, 139 deletions

diff --git a/include/media/mediascanner.h b/include/media/mediascanner.h
index 765c039..803bffb 100644
--- a/include/media/mediascanner.h
+++ b/include/media/mediascanner.h
@@ -23,23 +23,33 @@
 #include <utils/Errors.h>
 #include <pthread.h>
 
+struct dirent;
+
 namespace android {
 
 class MediaScannerClient;
 class StringArray;
 
+enum MediaScanResult {
+    // This file or directory was scanned successfully.
+    MEDIA_SCAN_RESULT_OK,
+    // This file or directory was skipped because it was not found, could
+    // not be opened, was of an unsupported type, or was malfored in some way.
+    MEDIA_SCAN_RESULT_SKIPPED,
+    // The scan should be aborted due to a fatal error such as out of memory
+    // or an exception.
+    MEDIA_SCAN_RESULT_ERROR,
+};
+
 struct MediaScanner {
     MediaScanner();
     virtual ~MediaScanner();
 
-    virtual status_t processFile(
-            const char *path, const char *mimeType,
-            MediaScannerClient &client) = 0;
+    virtual MediaScanResult processFile(
+            const char *path, const char *mimeType, MediaScannerClient &client) = 0;
 
-    typedef bool (*ExceptionCheck)(void* env);
-    virtual status_t processDirectory(
-            const char *path, MediaScannerClient &client,
-            ExceptionCheck exceptionCheck, void *exceptionEnv);
+    virtual MediaScanResult processDirectory(
+            const char *path, MediaScannerClient &client);
 
     void setLocale(const char *locale);
 
@@ -53,9 +63,11 @@ private:
     // current locale (like "ja_JP"), created/destroyed with strdup()/free()
     char *mLocale;
 
-    status_t doProcessDirectory(
-            char *path, int pathRemaining, MediaScannerClient &client,
-            bool noMedia, ExceptionCheck exceptionCheck, void *exceptionEnv);
+    MediaScanResult doProcessDirectory(
+            char *path, int pathRemaining, MediaScannerClient &client, bool noMedia);
+    MediaScanResult doProcessDirectoryEntry(
+            char *path, int pathRemaining, MediaScannerClient &client, bool noMedia,
+            struct dirent* entry, char* fileSpot);
 
     MediaScanner(const MediaScanner &);
     MediaScanner &operator=(const MediaScanner &);
@@ -68,13 +80,13 @@ public:
     virtual ~MediaScannerClient();
     void setLocale(const char* locale);
     void beginFile();
-    bool addStringTag(const char* name, const char* value);
+    status_t addStringTag(const char* name, const char* value);
     void endFile();
 
-    virtual bool scanFile(const char* path, long long lastModified,
+    virtual status_t scanFile(const char* path, long long lastModified,
             long long fileSize, bool isDirectory, bool noMedia) = 0;
-    virtual bool handleStringTag(const char* name, const char* value) = 0;
-    virtual bool setMimeType(const char* mimeType) = 0;
+    virtual status_t handleStringTag(const char* name, const char* value) = 0;
+    virtual status_t setMimeType(const char* mimeType) = 0;
 
 protected:
     void convertValues(uint32_t encoding);
diff --git a/include/media/stagefright/StagefrightMediaScanner.h b/include/media/stagefright/StagefrightMediaScanner.h
index 108acb4..6510a59 100644
--- a/include/media/stagefright/StagefrightMediaScanner.h
+++ b/include/media/stagefright/StagefrightMediaScanner.h
@@ -26,7 +26,7 @@ struct StagefrightMediaScanner : public MediaScanner {
     StagefrightMediaScanner();
     virtual ~StagefrightMediaScanner();
 
-    virtual status_t processFile(
+    virtual MediaScanResult processFile(
             const char *path, const char *mimeType,
             MediaScannerClient &client);
 
@@ -35,6 +35,10 @@ struct StagefrightMediaScanner : public MediaScanner {
 private:
     StagefrightMediaScanner(const StagefrightMediaScanner &);
     StagefrightMediaScanner &operator=(const StagefrightMediaScanner &);
+
+    MediaScanResult processFileInternal(
+            const char *path, const char *mimeType,
+            MediaScannerClient &client);
 };
 
 }  // namespace android
diff --git a/media/libmedia/MediaScanner.cpp b/media/libmedia/MediaScanner.cpp
index 45bdff4..41f8593 100644
--- a/media/libmedia/MediaScanner.cpp
+++ b/media/libmedia/MediaScanner.cpp
@@ -47,16 +47,15 @@ const char *MediaScanner::locale() const {
     return mLocale;
 }
 
-status_t MediaScanner::processDirectory(
-        const char *path, MediaScannerClient &client,
-        ExceptionCheck exceptionCheck, void *exceptionEnv) {
+MediaScanResult MediaScanner::processDirectory(
+        const char *path, MediaScannerClient &client) {
     int pathLength = strlen(path);
     if (pathLength >= PATH_MAX) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
     char* pathBuffer = (char *)malloc(PATH_MAX + 1);
     if (!pathBuffer) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_ERROR;
     }
 
     int pathRemaining = PATH_MAX - pathLength;
@@ -69,21 +68,18 @@ status_t MediaScanner::processDirectory(
 
     client.setLocale(locale());
 
-    status_t result =
-        doProcessDirectory(pathBuffer, pathRemaining, client, false, exceptionCheck, exceptionEnv);
+    MediaScanResult result = doProcessDirectory(pathBuffer, pathRemaining, client, false);
 
     free(pathBuffer);
 
     return result;
 }
 
-status_t MediaScanner::doProcessDirectory(
-        char *path, int pathRemaining, MediaScannerClient &client,
-        bool noMedia, ExceptionCheck exceptionCheck, void *exceptionEnv) {
+MediaScanResult MediaScanner::doProcessDirectory(
+        char *path, int pathRemaining, MediaScannerClient &client, bool noMedia) {
     // place to copy file or directory name
     char* fileSpot = path + strlen(path);
     struct dirent* entry;
-    struct stat statbuf;
 
     // Treat all files as non-media in directories that contain a  ".nomedia" file
     if (pathRemaining >= 8 /* strlen(".nomedia") */ ) {
@@ -99,76 +95,88 @@ status_t MediaScanner::doProcessDirectory(
 
     DIR* dir = opendir(path);
     if (!dir) {
-        LOGD("opendir %s failed, errno: %d", path, errno);
-        return UNKNOWN_ERROR;
+        LOGW("Error opening directory '%s', skipping: %s.", path, strerror(errno));
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
+    MediaScanResult result = MEDIA_SCAN_RESULT_OK;
     while ((entry = readdir(dir))) {
-        const char* name = entry->d_name;
-
-        // ignore "." and ".."
-        if (name[0] == '.' && (name[1] == 0 || (name[1] == '.' && name[2] == 0))) {
-            continue;
+        if (doProcessDirectoryEntry(path, pathRemaining, client, noMedia, entry, fileSpot)
+                == MEDIA_SCAN_RESULT_ERROR) {
+            result = MEDIA_SCAN_RESULT_ERROR;
+            break;
         }
+    }
+    closedir(dir);
+    return result;
+}
 
-        int nameLength = strlen(name);
-        if (nameLength + 1 > pathRemaining) {
-            // path too long!
-            continue;
-        }
-        strcpy(fileSpot, name);
-
-        int type = entry->d_type;
-        if (type == DT_UNKNOWN) {
-            // If the type is unknown, stat() the file instead.
-            // This is sometimes necessary when accessing NFS mounted filesystems, but
-            // could be needed in other cases well.
-            if (stat(path, &statbuf) == 0) {
-                if (S_ISREG(statbuf.st_mode)) {
-                    type = DT_REG;
-                } else if (S_ISDIR(statbuf.st_mode)) {
-                    type = DT_DIR;
-                }
-            } else {
-                LOGD("stat() failed for %s: %s", path, strerror(errno) );
+MediaScanResult MediaScanner::doProcessDirectoryEntry(
+        char *path, int pathRemaining, MediaScannerClient &client, bool noMedia,
+        struct dirent* entry, char* fileSpot) {
+    struct stat statbuf;
+    const char* name = entry->d_name;
+
+    // ignore "." and ".."
+    if (name[0] == '.' && (name[1] == 0 || (name[1] == '.' && name[2] == 0))) {
+        return MEDIA_SCAN_RESULT_SKIPPED;
+    }
+
+    int nameLength = strlen(name);
+    if (nameLength + 1 > pathRemaining) {
+        // path too long!
+        return MEDIA_SCAN_RESULT_SKIPPED;
+    }
+    strcpy(fileSpot, name);
+
+    int type = entry->d_type;
+    if (type == DT_UNKNOWN) {
+        // If the type is unknown, stat() the file instead.
+        // This is sometimes necessary when accessing NFS mounted filesystems, but
+        // could be needed in other cases well.
+        if (stat(path, &statbuf) == 0) {
+            if (S_ISREG(statbuf.st_mode)) {
+                type = DT_REG;
+            } else if (S_ISDIR(statbuf.st_mode)) {
+                type = DT_DIR;
             }
+        } else {
+            LOGD("stat() failed for %s: %s", path, strerror(errno) );
         }
-        if (type == DT_REG || type == DT_DIR) {
-            if (type == DT_DIR) {
-                bool childNoMedia = noMedia;
-                // set noMedia flag on directories with a name that starts with '.'
-                // for example, the Mac ".Trashes" directory
-                if (name[0] == '.')
-                    childNoMedia = true;
-
-                // report the directory to the client
-                if (stat(path, &statbuf) == 0) {
-                    client.scanFile(path, statbuf.st_mtime, 0, true, childNoMedia);
-                }
-
-                // and now process its contents
-                strcat(fileSpot, "/");
-                int err = doProcessDirectory(path, pathRemaining - nameLength - 1, client,
-                        childNoMedia, exceptionCheck, exceptionEnv);
-                if (err) {
-                    // pass exceptions up - ignore other errors
-                    if (exceptionCheck && exceptionCheck(exceptionEnv)) goto failure;
-                    LOGE("Error processing '%s' - skipping\n", path);
-                    continue;
-                }
-            } else {
-                stat(path, &statbuf);
-                client.scanFile(path, statbuf.st_mtime, statbuf.st_size, false, noMedia);
-                if (exceptionCheck && exceptionCheck(exceptionEnv)) goto failure;
+    }
+    if (type == DT_DIR) {
+        bool childNoMedia = noMedia;
+        // set noMedia flag on directories with a name that starts with '.'
+        // for example, the Mac ".Trashes" directory
+        if (name[0] == '.')
+            childNoMedia = true;
+
+        // report the directory to the client
+        if (stat(path, &statbuf) == 0) {
+            status_t status = client.scanFile(path, statbuf.st_mtime, 0,
+                    true /*isDirectory*/, childNoMedia);
+            if (status) {
+                return MEDIA_SCAN_RESULT_ERROR;
             }
         }
+
+        // and now process its contents
+        strcat(fileSpot, "/");
+        MediaScanResult result = doProcessDirectory(path, pathRemaining - nameLength - 1,
+                client, childNoMedia);
+        if (result == MEDIA_SCAN_RESULT_ERROR) {
+            return MEDIA_SCAN_RESULT_ERROR;
+        }
+    } else if (type == DT_REG) {
+        stat(path, &statbuf);
+        status_t status = client.scanFile(path, statbuf.st_mtime, statbuf.st_size,
+                false /*isDirectory*/, noMedia);
+        if (status) {
+            return MEDIA_SCAN_RESULT_ERROR;
+        }
     }
 
-    closedir(dir);
-    return OK;
-failure:
-    closedir(dir);
-    return -1;
+    return MEDIA_SCAN_RESULT_OK;
 }
 
 }  // namespace android
diff --git a/media/libmedia/MediaScannerClient.cpp b/media/libmedia/MediaScannerClient.cpp
index bd3596e..7a7aeb6 100644
--- a/media/libmedia/MediaScannerClient.cpp
+++ b/media/libmedia/MediaScannerClient.cpp
@@ -62,7 +62,7 @@ void MediaScannerClient::beginFile()
     mValues = new StringArray;
 }
 
-bool MediaScannerClient::addStringTag(const char* name, const char* value)
+status_t MediaScannerClient::addStringTag(const char* name, const char* value)
 {
     if (mLocaleEncoding != kEncodingNone) {
         // don't bother caching strings that are all ASCII.
@@ -212,8 +212,10 @@ void MediaScannerClient::endFile()
 
         // finally, push all name/value pairs to the client
         for (int i = 0; i < mNames->size(); i++) {
-            if (!handleStringTag(mNames->getEntry(i), mValues->getEntry(i)))
+            status_t status = handleStringTag(mNames->getEntry(i), mValues->getEntry(i));
+            if (status) {
                 break;
+            }
         }
     }
     // else addStringTag() has done all the work so we have nothing to do
diff --git a/media/libstagefright/StagefrightMediaScanner.cpp b/media/libstagefright/StagefrightMediaScanner.cpp
index 89faff7..571e8be 100644
--- a/media/libstagefright/StagefrightMediaScanner.cpp
+++ b/media/libstagefright/StagefrightMediaScanner.cpp
@@ -52,13 +52,13 @@ static bool FileHasAcceptableExtension(const char *extension) {
     return false;
 }
 
-static status_t HandleMIDI(
+static MediaScanResult HandleMIDI(
         const char *filename, MediaScannerClient *client) {
     // get the library configuration and do sanity check
     const S_EAS_LIB_CONFIG* pLibConfig = EAS_Config();
     if ((pLibConfig == NULL) || (LIB_VERSION != pLibConfig->libVersion)) {
         LOGE("EAS library/header mismatch\n");
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_ERROR;
     }
     EAS_I32 temp;
 
@@ -88,34 +88,41 @@ static status_t HandleMIDI(
     }
 
     if (result != EAS_SUCCESS) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
     char buffer[20];
     sprintf(buffer, "%ld", temp);
-    if (!client->addStringTag("duration", buffer)) return UNKNOWN_ERROR;
-
-    return OK;
+    status_t status = client->addStringTag("duration", buffer);
+    if (status) {
+        return MEDIA_SCAN_RESULT_ERROR;
+    }
+    return MEDIA_SCAN_RESULT_OK;
 }
 
-status_t StagefrightMediaScanner::processFile(
+MediaScanResult StagefrightMediaScanner::processFile(
         const char *path, const char *mimeType,
         MediaScannerClient &client) {
     LOGV("processFile '%s'.", path);
 
     client.setLocale(locale());
     client.beginFile();
+    MediaScanResult result = processFileInternal(path, mimeType, client);
+    client.endFile();
+    return result;
+}
 
+MediaScanResult StagefrightMediaScanner::processFileInternal(
+        const char *path, const char *mimeType,
+        MediaScannerClient &client) {
     const char *extension = strrchr(path, '.');
 
     if (!extension) {
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
     if (!FileHasAcceptableExtension(extension)) {
-        client.endFile();
-
-        return UNKNOWN_ERROR;
+        return MEDIA_SCAN_RESULT_SKIPPED;
     }
 
     if (!strcasecmp(extension, ".mid")
@@ -127,53 +134,57 @@ status_t StagefrightMediaScanner::processFile(
             || !strcasecmp(extension, ".rtx")
             || !strcasecmp(extension, ".ota")
             || !strcasecmp(extension, ".mxmf")) {
-        status_t status = HandleMIDI(path, &client);
-        if (status != OK) {
-            return status;
+        return HandleMIDI(path, &client);
+    }
+
+    sp<MediaMetadataRetriever> mRetriever(new MediaMetadataRetriever);
+
+    status_t status = mRetriever->setDataSource(path);
+    if (status) {
+        return MEDIA_SCAN_RESULT_ERROR;
+    }
+
+    const char *value;
+    if ((value = mRetriever->extractMetadata(
+                    METADATA_KEY_MIMETYPE)) != NULL) {
+        status = client.setMimeType(value);
+        if (status) {
+            return MEDIA_SCAN_RESULT_ERROR;
         }
-    } else {
-        sp<MediaMetadataRetriever> mRetriever(new MediaMetadataRetriever);
-
-         if (mRetriever->setDataSource(path) == OK) {
-            const char *value;
-            if ((value = mRetriever->extractMetadata(
-                            METADATA_KEY_MIMETYPE)) != NULL) {
-                client.setMimeType(value);
-            }
+    }
 
-            struct KeyMap {
-                const char *tag;
-                int key;
-            };
-            static const KeyMap kKeyMap[] = {
-                { "tracknumber", METADATA_KEY_CD_TRACK_NUMBER },
-                { "discnumber", METADATA_KEY_DISC_NUMBER },
-                { "album", METADATA_KEY_ALBUM },
-                { "artist", METADATA_KEY_ARTIST },
-                { "albumartist", METADATA_KEY_ALBUMARTIST },
-                { "composer", METADATA_KEY_COMPOSER },
-                { "genre", METADATA_KEY_GENRE },
-                { "title", METADATA_KEY_TITLE },
-                { "year", METADATA_KEY_YEAR },
-                { "duration", METADATA_KEY_DURATION },
-                { "writer", METADATA_KEY_WRITER },
-                { "compilation", METADATA_KEY_COMPILATION },
-                { "isdrm", METADATA_KEY_IS_DRM },
-            };
-            static const size_t kNumEntries = sizeof(kKeyMap) / sizeof(kKeyMap[0]);
-
-            for (size_t i = 0; i < kNumEntries; ++i) {
-                const char *value;
-                if ((value = mRetriever->extractMetadata(kKeyMap[i].key)) != NULL) {
-                    client.addStringTag(kKeyMap[i].tag, value);
-                }
+    struct KeyMap {
+        const char *tag;
+        int key;
+    };
+    static const KeyMap kKeyMap[] = {
+        { "tracknumber", METADATA_KEY_CD_TRACK_NUMBER },
+        { "discnumber", METADATA_KEY_DISC_NUMBER },
+        { "album", METADATA_KEY_ALBUM },
+        { "artist", METADATA_KEY_ARTIST },
+        { "albumartist", METADATA_KEY_ALBUMARTIST },
+        { "composer", METADATA_KEY_COMPOSER },
+        { "genre", METADATA_KEY_GENRE },
+        { "title", METADATA_KEY_TITLE },
+        { "year", METADATA_KEY_YEAR },
+        { "duration", METADATA_KEY_DURATION },
+        { "writer", METADATA_KEY_WRITER },
+        { "compilation", METADATA_KEY_COMPILATION },
+        { "isdrm", METADATA_KEY_IS_DRM },
+    };
+    static const size_t kNumEntries = sizeof(kKeyMap) / sizeof(kKeyMap[0]);
+
+    for (size_t i = 0; i < kNumEntries; ++i) {
+        const char *value;
+        if ((value = mRetriever->extractMetadata(kKeyMap[i].key)) != NULL) {
+            status = client.addStringTag(kKeyMap[i].tag, value);
+            if (status) {
+                return MEDIA_SCAN_RESULT_ERROR;
             }
         }
     }
 
-    client.endFile();
-
-    return OK;
+    return MEDIA_SCAN_RESULT_OK;
 }
 
 char *StagefrightMediaScanner::extractAlbumArt(int fd) {