Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a3c766eb authored by Gopalakrishnan Nallasamy's avatar Gopalakrishnan Nallasamy
Browse files

MPEG4Writer:stop & notify,MediaMuxer:remove notify 

1) MPEG4Writer.cpp, Whenever there are critical errors its better
   that the threads are stopped before notifying its client.
2) Recent code changes(ag/9886712) in MediaMuxer.cpp in R to receive
   notifications from writer threads, MIGHT lead to deadlock in a
   scenario when the client thread was waiting for the
   writeSampleData to complete while holding mLock, and a
   notification was sent from track threads or the main  writer
   thread to MediaMuxer without stopping the source and/or
   stopping the writer threads. Though tough to reproduce(if at all)
   it's a bad design that should be avoided to keep the code simple
   and less error prone. Now that MPEG4Writer either stops
   the source or stop all writer threads based on the severity of
   the errors, it would be reflected in the next writeSampleData
   or stop call from the client.  Hence, removed the code changes
   for notifying error from writer to MediaMuxer class.
3) Added logs in MediaWriter to get details of notifications with
   track id.

Bug: 157600876

Test: android.media.cts.MediaMuxerTest
Test: android.mediav2.cts.MuxerTest
Test: android.media.cts.MediaRecorderTest
Test: android.mediastress.cts.MediaMuxerStressTest

Change-Id: Idb8996134010669261fa638907b497c0cf70de6b
parent bf74c089

media/libstagefright/MPEG4Writer.cpp

+3 −2
Original line number Diff line number Diff line
@@ -2401,8 +2401,8 @@ void MPEG4Writer::onMessageReceived(const sp<AMessage> &msg) { 
            int32_t err;

            CHECK(msg->findInt32("errno", &err));

            // Stop tracks' threads and main writer thread.

            notify(MEDIA_RECORDER_EVENT_ERROR, MEDIA_RECORDER_ERROR_UNKNOWN, err);

            stop();

            notify(MEDIA_RECORDER_EVENT_ERROR, MEDIA_RECORDER_ERROR_UNKNOWN, err);

            break;

        }

        // fallocate() failed, hence notify app about it and stop().
@@ -2410,9 +2410,10 @@ void MPEG4Writer::onMessageReceived(const sp<AMessage> &msg) { 
            ALOGE("kWhatFallocateError");

            int32_t err;

            CHECK(msg->findInt32("errno", &err));

            // Stop tracks' threads and main writer thread.

            stop();

            //TODO: introduce a suitable MEDIA_RECORDER_ERROR_* instead MEDIA_RECORDER_ERROR_UNKNOWN?

            notify(MEDIA_RECORDER_EVENT_ERROR, MEDIA_RECORDER_ERROR_UNKNOWN, err);

            stop();

            break;

        }

        default:

media/libstagefright/MediaMuxer.cpp

+4 −41
Original line number Diff line number Diff line
@@ -48,8 +48,7 @@ static bool isMp4Format(MediaMuxer::OutputFormat format) { 


MediaMuxer::MediaMuxer(int fd, OutputFormat format)

    : mFormat(format),

      mState(UNINITIALIZED),

      mError(OK) {

      mState(UNINITIALIZED) {

    if (isMp4Format(format)) {

        mWriter = new MPEG4Writer(fd);

    } else if (format == OUTPUT_FORMAT_WEBM) {
@@ -59,7 +58,6 @@ MediaMuxer::MediaMuxer(int fd, OutputFormat format) 
    }



    if (mWriter != NULL) {

        mWriter->setMuxerListener(this);

        mFileMeta = new MetaData;

        if (format == OUTPUT_FORMAT_HEIF) {

            // Note that the key uses recorder file types.
@@ -157,26 +155,16 @@ status_t MediaMuxer::start() { 


status_t MediaMuxer::stop() {

    Mutex::Autolock autoLock(mMuxerLock);

    if (mState == STARTED || mState == ERROR) {

    if (mState == STARTED) {

        mState = STOPPED;

        for (size_t i = 0; i < mTrackList.size(); i++) {

            if (mTrackList[i]->stop() != OK) {

                return INVALID_OPERATION;

            }

        }

        // Unlock this mutex to allow notify to be called during stop process.

        mMuxerLock.unlock();

        status_t err = mWriter->stop();

        mMuxerLock.lock();

        if (err != OK || mError != OK) {

            ALOGE("stop err: %d, mError:%d", err, mError);

        }

        /* Prioritize mError over err as writer would have got stopped on any

         * internal error and notified muxer already.  Clients might issue

         * stop again later, and mWriter->stop() would return success.

         */

        if (mError != OK) {

            err = mError;

        if (err != OK) {

            ALOGE("stop() err: %d", err);

        }

        return err;

    } else {
@@ -232,29 +220,4 @@ status_t MediaMuxer::writeSampleData(const sp<ABuffer> &buffer, size_t trackInde 
    return currentTrack->pushBuffer(mediaBuffer);

}



void MediaMuxer::notify(int msg, int ext1, int ext2) {

    switch (msg) {

        case MEDIA_RECORDER_EVENT_ERROR:

        case MEDIA_RECORDER_TRACK_EVENT_ERROR: {

            Mutex::Autolock autoLock(mMuxerLock);

            mState = ERROR;

            mError = ext2;

            ALOGW("message received msg=%d, ext1=%d, ext2=%d", msg, ext1, ext2);

            break;

        }

        case MEDIA_RECORDER_EVENT_INFO: {

            if (ext1 == MEDIA_RECORDER_INFO_UNKNOWN) {

                Mutex::Autolock autoLock(mMuxerLock);

                mState = ERROR;

                mError = ext2;

                ALOGW("message received msg=%d, ext1=%d, ext2=%d", msg, ext1, ext2);

            }

            break;

        }

        default:

            // Ignore INFO and other notifications for now.

            break;

    }

}



}  // namespace android

media/libstagefright/include/media/stagefright/MediaMuxer.h

+1 −5
Original line number Diff line number Diff line
@@ -117,8 +117,6 @@ public: 
    status_t writeSampleData(const sp<ABuffer> &buffer, size_t trackIndex,

                             int64_t timeUs, uint32_t flags) ;



    void notify(int msg, int ext1, int ext2);



private:

    const OutputFormat mFormat;

    sp<MediaWriter> mWriter;
@@ -130,11 +128,9 @@ private: 
        UNINITIALIZED,

        INITIALIZED,

        STARTED,

        STOPPED,

        ERROR

        STOPPED

    };

    State mState;

    status_t mError;



    DISALLOW_EVIL_CONSTRUCTORS(MediaMuxer);

};

media/libstagefright/include/media/stagefright/MediaWriter.h

+10 −8
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@ 
#include <utils/RefBase.h>

#include <media/stagefright/MediaSource.h>

#include <media/IMediaRecorderClient.h>

#include <media/stagefright/MediaMuxer.h>

#include <media/mediarecorder.h>



namespace android {
@@ -46,7 +46,6 @@ struct MediaWriter : public RefBase { 
    virtual void setListener(const sp<IMediaRecorderClient>& listener) {

        mListener = listener;

    }

    virtual void setMuxerListener(const wp<MediaMuxer>& muxer) { mMuxer = muxer; }



    virtual status_t dump(int /*fd*/, const Vector<String16>& /*args*/) {

        return OK;
@@ -61,17 +60,20 @@ protected: 
    int64_t mMaxFileSizeLimitBytes;

    int64_t mMaxFileDurationLimitUs;

    sp<IMediaRecorderClient> mListener;

    wp<MediaMuxer> mMuxer;



    void notify(int msg, int ext1, int ext2) {

        ALOG(LOG_VERBOSE, "MediaWriter", "notify msg:%d, ext1:%d, ext2:%d", msg, ext1, ext2);

        if (msg == MEDIA_RECORDER_TRACK_EVENT_INFO || msg == MEDIA_RECORDER_TRACK_EVENT_ERROR) {

            uint32_t trackId = (ext1 >> 28) & 0xf;

            int type = ext1 & 0xfffffff;

            ALOG(LOG_VERBOSE, "MediaWriter", "Track event err/info msg:%d, trackId:%u, type:%d,"

                                             "val:%d", msg, trackId, type, ext2);

        } else {

            ALOG(LOG_VERBOSE, "MediaWriter", "Recorder event msg:%d, ext1:%d, ext2:%d",

                                              msg, ext1, ext2);

        }

        if (mListener != nullptr) {

            mListener->notify(msg, ext1, ext2);

        }

        sp<MediaMuxer> muxer = mMuxer.promote();

        if (muxer != nullptr) {

            muxer->notify(msg, ext1, ext2);

        }

    }

private:

    MediaWriter(const MediaWriter &);