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

Commit 4d318fdb authored by Gopalakrishnan Nallasamy's avatar Gopalakrishnan Nallasamy
Browse files

MPEG4Writer:Avoid deadlock when reset internally

For client initiated request, reset's return value is important
and hence wait for any previous internal reset to complete.
For reset initiated by control looper thread, prevent multiple
simultaneous calls to avoid deadlock.
Save reset status and return the same in subsequent calls.

Bug: 159082429
Bug: 159225772

Test:   atest android.media.cts.MediaMuxerTest \
        android.mediav2.cts.MuxerTest \
        android.mediav2.cts.MuxerUnitTest \
        android.media.cts.MediaRecorderTest \
        android.mediastress.cts.MediaRecorderStressTest \
        android.mediastress.cts.MediaMuxerStressTest

Change-Id: Idda58a65abf7f49989f9aec2a7c56413ee3aebdd
parent 86b5e43a
Loading
Loading
Loading
Loading
+60 −30
Original line number Diff line number Diff line
@@ -542,6 +542,7 @@ void MPEG4Writer::initInternal(int fd, bool isFirstSession) {
    mNumGrids = 0;
    mNextItemId = kItemIdBase;
    mHasRefs = false;
    mResetStatus = OK;
    mPreAllocFirstTime = true;
    mPrevAllTracksTotalMetaDataSizeEstimate = 0;

@@ -1027,6 +1028,11 @@ status_t MPEG4Writer::start(MetaData *param) {
    return OK;
}

status_t MPEG4Writer::stop() {
    // If reset was in progress, wait for it to complete.
    return reset(true, true);
}

status_t MPEG4Writer::pause() {
    ALOGW("MPEG4Writer: pause is not supported");
    return ERROR_UNSUPPORTED;
@@ -1141,8 +1147,12 @@ status_t MPEG4Writer::release() {
    return err;
}

void MPEG4Writer::finishCurrentSession() {
    reset(false /* stopSource */);
status_t MPEG4Writer::finishCurrentSession() {
    ALOGV("finishCurrentSession");
    /* Don't wait if reset is in progress already, that avoids deadlock
     * as finishCurrentSession() is called from control looper thread.
     */
    return reset(false, false);
}

status_t MPEG4Writer::switchFd() {
@@ -1164,11 +1174,32 @@ status_t MPEG4Writer::switchFd() {
    return err;
}

status_t MPEG4Writer::reset(bool stopSource) {
status_t MPEG4Writer::reset(bool stopSource, bool waitForAnyPreviousCallToComplete) {
    ALOGD("reset()");
    std::lock_guard<std::mutex> l(mResetMutex);
    std::unique_lock<std::mutex> lk(mResetMutex, std::defer_lock);
    if (waitForAnyPreviousCallToComplete) {
        /* stop=>reset from client needs the return value of reset call, hence wait here
         * if a reset was in process already.
         */
        lk.lock();
    } else if (!lk.try_lock()) {
        /* Internal reset from control looper thread shouldn't wait for any reset in
         * process already.
         */
        return INVALID_OPERATION;
    }

    if (mResetStatus != OK) {
        /* Don't have to proceed if reset has finished with an error before.
         * If there was no error before, proceeding reset would be harmless, as the
         * the call would return from the mInitCheck condition below.
         */
        return mResetStatus;
    }

    if (mInitCheck != OK) {
        return OK;
        mResetStatus = OK;
        return mResetStatus;
    } else {
        if (!mWriterThreadStarted ||
            !mStarted) {
@@ -1180,7 +1211,8 @@ status_t MPEG4Writer::reset(bool stopSource) {
            if (writerErr != OK) {
                retErr = writerErr;
            }
            return retErr;
            mResetStatus = retErr;
            return mResetStatus;
        }
    }

@@ -1227,7 +1259,8 @@ status_t MPEG4Writer::reset(bool stopSource) {
    if (err != OK && err != ERROR_MALFORMED) {
        // Ignoring release() return value as there was an "err" already.
        release();
        return err;
        mResetStatus = err;
        return mResetStatus;
    }

    // Fix up the size of the 'mdat' chunk.
@@ -1285,7 +1318,8 @@ status_t MPEG4Writer::reset(bool stopSource) {
    if (err == OK) {
        err = errRelease;
    }
    return err;
    mResetStatus = err;
    return mResetStatus;
}

/*
@@ -2426,31 +2460,27 @@ void MPEG4Writer::onMessageReceived(const sp<AMessage> &msg) {
            int fd = mNextFd;
            mNextFd = -1;
            mLock.unlock();
            finishCurrentSession();
            if (finishCurrentSession() == OK) {
                initInternal(fd, false /*isFirstSession*/);
            start(mStartMeta.get());
                status_t status = start(mStartMeta.get());
                mSwitchPending = false;
            notify(MEDIA_RECORDER_EVENT_INFO, MEDIA_RECORDER_INFO_NEXT_OUTPUT_FILE_STARTED, 0);
            break;
                if (status == OK)  {
                    notify(MEDIA_RECORDER_EVENT_INFO,
                           MEDIA_RECORDER_INFO_NEXT_OUTPUT_FILE_STARTED, 0);
                }
            }
        // ::write() or lseek64() wasn't a success, file could be malformed
        case kWhatIOError: {
            ALOGE("kWhatIOError");
            int32_t err;
            CHECK(msg->findInt32("err", &err));
            // Stop tracks' threads and main writer thread.
            stop();
            notify(MEDIA_RECORDER_EVENT_ERROR, MEDIA_RECORDER_ERROR_UNKNOWN, err);
            break;
        }
        // fallocate() failed, hence stop() and notify app.
        case kWhatFallocateError: {
            ALOGE("kWhatFallocateError");
        /* ::write() or lseek64() wasn't a success, file could be malformed.
         * Or fallocate() failed. reset() and notify client on both the cases.
         */
        case kWhatFallocateError: // fallthrough
        case kWhatIOError: {
            int32_t err;
            CHECK(msg->findInt32("err", &err));
            // Stop tracks' threads and main writer thread.
            stop();
            //TODO: introduce a suitable MEDIA_RECORDER_ERROR_* instead MEDIA_RECORDER_ERROR_UNKNOWN?
            // If reset already in process, don't wait for it complete to avoid deadlock.
            reset(true, false);
            //TODO: new MEDIA_RECORDER_ERROR_**** instead MEDIA_RECORDER_ERROR_UNKNOWN ?
            notify(MEDIA_RECORDER_EVENT_ERROR, MEDIA_RECORDER_ERROR_UNKNOWN, err);
            break;
        }
@@ -2458,7 +2488,7 @@ void MPEG4Writer::onMessageReceived(const sp<AMessage> &msg) {
         * Responding with other options could be added later if required.
         */
        case kWhatNoIOErrorSoFar: {
            ALOGD("kWhatNoIOErrorSoFar");
            ALOGV("kWhatNoIOErrorSoFar");
            sp<AMessage> response = new AMessage;
            response->setInt32("err", OK);
            sp<AReplyToken> replyID;
+6 −3
Original line number Diff line number Diff line
@@ -45,7 +45,7 @@ public:

    // 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 stop();
    virtual status_t pause();
    virtual bool reachedEOS();
    virtual status_t dump(int fd, const Vector<String16>& args);
@@ -125,12 +125,15 @@ private:
    bool mWriteSeekErr;
    bool mFallocateErr;
    bool mPreAllocationEnabled;
    status_t mResetStatus;

    sp<ALooper> mLooper;
    sp<AHandlerReflector<MPEG4Writer> > mReflector;

    Mutex mLock;
    // Serialize reset calls from client of MPEG4Writer and MP4WtrCtrlHlpLooper.
    std::mutex mResetMutex;
    // Serialize preallocation calls from different track threads.
    std::mutex mFallocMutex;
    bool mPreAllocFirstTime; // Pre-allocate space for file and track headers only once per file.
    uint64_t mPrevAllTracksTotalMetaDataSizeEstimate;
@@ -298,7 +301,7 @@ private:
    void writeGeoDataBox();
    void writeLatitude(int degreex10000);
    void writeLongitude(int degreex10000);
    void finishCurrentSession();
    status_t finishCurrentSession();

    void addDeviceMeta();
    void writeHdlr(const char *handlerType);
@@ -331,7 +334,7 @@ private:
    void sendSessionSummary();
    status_t release();
    status_t switchFd();
    status_t reset(bool stopSource = true);
    status_t reset(bool stopSource = true, bool waitForAnyPreviousCallToComplete = true);

    static uint32_t getMpeg4Time();