Make limitations of MPEG4Writer explicit

o No more than 2 tracks will be supported
o No more than one video and/or one audio tracks will be supported
o Only take video and/or audio track (for instance, no text tracks)
o If there is no track before start() is called, bail out.

At the same time, make sure the errors from addSource() report to
addTrack(), not to start().

Bug: 7991013

Change-Id: I1ca35aaeb75b5448d75ed2c6c10dd12ecea720ab

diff --git a/include/media/stagefright/MPEG4Writer.h b/include/media/stagefright/MPEG4Writer.h
index 3596b38..88df6b0 100644 (file)
--- a/include/media/stagefright/MPEG4Writer.h
+++ b/include/media/stagefright/MPEG4Writer.h
@@ -35,7 +35,13 @@ public:
     MPEG4Writer(const char *filename);
     MPEG4Writer(int fd);
 
+    // Limitations
+    // 1. No more than 2 tracks can be added
+    // 2. Only video or audio source can be added
+    // 3. No more than one video and/or one audio source can be added.
     virtual status_t addSource(const sp<MediaSource> &source);
+
+    // Returns INVALID_OPERATION if there is no source or track.
     virtual status_t start(MetaData *param = NULL);
     virtual status_t stop() { return reset(); }
     virtual status_t pause();

diff --git a/media/libstagefright/MPEG4Writer.cpp b/media/libstagefright/MPEG4Writer.cpp
index 056b47a..316f669 100644 (file)
--- a/media/libstagefright/MPEG4Writer.cpp
+++ b/media/libstagefright/MPEG4Writer.cpp
@@ -428,6 +428,42 @@ status_t MPEG4Writer::addSource(const sp<MediaSource> &source) {
         ALOGE("Attempt to add source AFTER recording is started");
         return UNKNOWN_ERROR;
     }
+
+    // At most 2 tracks can be supported.
+    if (mTracks.size() >= 2) {
+        ALOGE("Too many tracks (%d) to add", mTracks.size());
+        return ERROR_UNSUPPORTED;
+    }
+
+    CHECK(source.get() != NULL);
+
+    // A track of type other than video or audio is not supported.
+    const char *mime;
+    source->getFormat()->findCString(kKeyMIMEType, &mime);
+    bool isAudio = !strncasecmp(mime, "audio/", 6);
+    bool isVideo = !strncasecmp(mime, "video/", 6);
+    if (!isAudio && !isVideo) {
+        ALOGE("Track (%s) other than video or audio is not supported",
+            mime);
+        return ERROR_UNSUPPORTED;
+    }
+
+    // At this point, we know the track to be added is either
+    // video or audio. Thus, we only need to check whether it
+    // is an audio track or not (if it is not, then it must be
+    // a video track).
+
+    // No more than one video or one audio track is supported.
+    for (List<Track*>::iterator it = mTracks.begin();
+         it != mTracks.end(); ++it) {
+        if ((*it)->isAudio() == isAudio) {
+            ALOGE("%s track already exists", isAudio? "Audio": "Video");
+            return ERROR_UNSUPPORTED;
+        }
+    }
+
+    // This is the first track of either audio or video.
+    // Go ahead to add the track.
     Track *track = new Track(this, source, 1 + mTracks.size());
     mTracks.push_back(track);
 
@@ -435,6 +471,11 @@ status_t MPEG4Writer::addSource(const sp<MediaSource> &source) {
 }
 
 status_t MPEG4Writer::startTracks(MetaData *params) {
+    if (mTracks.empty()) {
+        ALOGE("No source added");
+        return INVALID_OPERATION;
+    }
+
     for (List<Track *>::iterator it = mTracks.begin();
          it != mTracks.end(); ++it) {
         status_t err = (*it)->start(params);

diff --git a/media/libstagefright/MediaMuxer.cpp b/media/libstagefright/MediaMuxer.cpp
index aefc270..21841b3 100644 (file)
--- a/media/libstagefright/MediaMuxer.cpp
+++ b/media/libstagefright/MediaMuxer.cpp
@@ -76,17 +76,17 @@ ssize_t MediaMuxer::addTrack(const sp<AMessage> &format) {
     convertMessageToMetaData(format, meta);
 
     sp<MediaAdapter> newTrack = new MediaAdapter(meta);
-    return mTrackList.add(newTrack);
+    status_t result = mWriter->addSource(newTrack);
+    if (result == OK) {
+        return mTrackList.add(newTrack);
+    }
+    return -1;
 }
 
 status_t MediaMuxer::start() {
     Mutex::Autolock autoLock(mMuxerLock);
-
     if (mState == INITED) {
         mState = STARTED;
-        for (size_t i = 0 ; i < mTrackList.size(); i++) {
-            mWriter->addSource(mTrackList[i]);
-        }
         return mWriter->start();
     } else {
         ALOGE("start() is called in invalid state %d", mState);
@@ -100,7 +100,9 @@ status_t MediaMuxer::stop() {
     if (mState == STARTED) {
         mState = STOPPED;
         for (size_t i = 0; i < mTrackList.size(); i++) {
-            mTrackList[i]->stop();
+            if (mTrackList[i]->stop() != OK) {
+                return INVALID_OPERATION;
+            }
         }
         return mWriter->stop();
     } else {