Merge "MPEG4Writer:Avoid deadlock when reset internally"
diff --git a/media/libstagefright/MPEG4Writer.cpp b/media/libstagefright/MPEG4Writer.cpp
index 15f47be..823c010 100644
--- a/media/libstagefright/MPEG4Writer.cpp
+++ b/media/libstagefright/MPEG4Writer.cpp
@@ -542,6 +542,7 @@
mNumGrids = 0;
mNextItemId = kItemIdBase;
mHasRefs = false;
+ mResetStatus = OK;
mPreAllocFirstTime = true;
mPrevAllTracksTotalMetaDataSizeEstimate = 0;
@@ -1027,6 +1028,11 @@
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 @@
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 @@
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 @@
if (writerErr != OK) {
retErr = writerErr;
}
- return retErr;
+ mResetStatus = retErr;
+ return mResetStatus;
}
}
@@ -1227,7 +1259,8 @@
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 @@
if (err == OK) {
err = errRelease;
}
- return err;
+ mResetStatus = err;
+ return mResetStatus;
}
/*
@@ -1889,7 +1923,7 @@
ALOGD("ftruncate mPreAllocateFileEndOffset:%" PRId64 " mOffset:%" PRIu64
" mMdatEndOffset:%" PRIu64 " diff:%" PRId64, mPreAllocateFileEndOffset, mOffset,
mMdatEndOffset, mPreAllocateFileEndOffset - endOffset);
- if(ftruncate(mFd, endOffset) == -1) {
+ if (ftruncate(mFd, endOffset) == -1) {
ALOGE("ftruncate err:%s, %d, fd:%d", strerror(errno), errno, mFd);
status = false;
/* No need to post and handle(stop & notify client) error like it's done in preAllocate(),
@@ -2426,31 +2460,27 @@
int fd = mNextFd;
mNextFd = -1;
mLock.unlock();
- finishCurrentSession();
- initInternal(fd, false /*isFirstSession*/);
- start(mStartMeta.get());
- mSwitchPending = false;
- notify(MEDIA_RECORDER_EVENT_INFO, MEDIA_RECORDER_INFO_NEXT_OUTPUT_FILE_STARTED, 0);
+ if (finishCurrentSession() == OK) {
+ initInternal(fd, false /*isFirstSession*/);
+ status_t status = start(mStartMeta.get());
+ mSwitchPending = false;
+ if (status == OK) {
+ notify(MEDIA_RECORDER_EVENT_INFO,
+ MEDIA_RECORDER_INFO_NEXT_OUTPUT_FILE_STARTED, 0);
+ }
+ }
break;
}
- // ::write() or lseek64() wasn't a success, file could be malformed
+ /* ::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: {
- 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");
- 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 @@
* 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;
diff --git a/media/libstagefright/include/media/stagefright/MPEG4Writer.h b/media/libstagefright/include/media/stagefright/MPEG4Writer.h
index a1fe57c..40c4bfc 100644
--- a/media/libstagefright/include/media/stagefright/MPEG4Writer.h
+++ b/media/libstagefright/include/media/stagefright/MPEG4Writer.h
@@ -45,7 +45,7 @@
// 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 @@
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 @@
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 @@
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();