Merge "MediaCodec: fix possible crash at stop & error" (e39e35ff) · Commits · e / os / android_frameworks_av

media/libstagefright/MediaCodec.cpp

+35 −19

Original line number	Diff line number	Diff line
		@@ -2131,6 +2131,8 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		}

		bool sendErrorResponse = true;
		std::string origin{"kWhatError:"};
		origin += stateString(mState);

		switch (mState) {
		case INITIALIZING:
		@@ -2182,14 +2184,14 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		// be a shutdown complete notification after
		// all.

		// note that we're directly going from
		// note that we may be directly going from
		// STOPPING->UNINITIALIZED, instead of the
		// usual STOPPING->INITIALIZED state.
		setState(UNINITIALIZED);
		if (mState == RELEASING) {
		mComponentName.clear();
		}
		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages(origin + ":dead");
		sendErrorResponse = false;
		}
		break;
		@@ -2280,7 +2282,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		// released by ResourceManager.
		finalErr = DEAD_OBJECT;
		}
		postPendingRepliesAndDeferredMessages(finalErr);
		postPendingRepliesAndDeferredMessages(origin, finalErr);
		}
		break;
		}
		@@ -2328,7 +2330,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		MediaResource::CodecResource(mFlags & kFlagIsSecure, mIsVideo));
		}

		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages("kWhatComponentAllocated");
		break;
		}

		@@ -2367,7 +2369,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		mFlags \|= kFlagUsesSoftwareRenderer;
		}
		setState(CONFIGURED);
		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages("kWhatComponentConfigured");

		// augment our media metrics info, now that we know more things
		// such as what the codec extracted from any CSD passed in.
		@@ -2436,7 +2438,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		} else {
		response->setInt32("err", err);
		}
		postPendingRepliesAndDeferredMessages(response);
		postPendingRepliesAndDeferredMessages("kWhatInputSurfaceCreated", response);
		break;
		}

		@@ -2458,7 +2460,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		} else {
		response->setInt32("err", err);
		}
		postPendingRepliesAndDeferredMessages(response);
		postPendingRepliesAndDeferredMessages("kWhatInputSurfaceAccepted", response);
		break;
		}

		@@ -2476,7 +2478,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		if (msg->findInt32("err", &err)) {
		response->setInt32("err", err);
		}
		postPendingRepliesAndDeferredMessages(response);
		postPendingRepliesAndDeferredMessages("kWhatSignaledInputEOS", response);
		break;
		}

		@@ -2495,7 +2497,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		MediaResource::GraphicMemoryResource(getGraphicBufferSize()));
		}
		setState(STARTED);
		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages("kWhatStartCompleted");
		break;
		}

		@@ -2725,7 +2727,13 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		break;
		}
		setState(INITIALIZED);
		postPendingRepliesAndDeferredMessages();
		if (mReplyID) {
		postPendingRepliesAndDeferredMessages("kWhatStopCompleted");
		} else {
		ALOGW("kWhatStopCompleted: presumably an error occurred earlier, "
		"but the operation completed anyway. (last reply origin=%s)",
		mLastReplyOrigin.c_str());
		}
		break;
		}

		@@ -2749,7 +2757,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		mReleaseSurface.reset();

		if (mReplyID != nullptr) {
		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages("kWhatReleaseCompleted");
		}
		if (mAsyncReleaseCompleteNotification != nullptr) {
		flushMediametrics();
		@@ -2774,7 +2782,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		mCodec->signalResume();
		}

		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages("kWhatFlushCompleted");
		break;
		}

		@@ -3163,7 +3171,8 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		if (mState == FLUSHING \|\| mState == STOPPING
		\|\| mState == CONFIGURING \|\| mState == STARTING) {
		// mReply is always set if in these states.
		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages(
		std::string("kWhatRelease:") + stateString(mState));
		}

		if (mFlags & kFlagSawMediaServerDie) {
		@@ -3212,7 +3221,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
		// State transition replies are handled above, so this reply
		// would not be related to state transition. As we are
		// shutting down the component, just fail the operation.
		postPendingRepliesAndDeferredMessages(UNKNOWN_ERROR);
		postPendingRepliesAndDeferredMessages("kWhatRelease:reply", UNKNOWN_ERROR);
		}
		mReplyID = replyID;
		setState(msg->what() == kWhatStop ? STOPPING : RELEASING);
		@@ -3228,7 +3237,7 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {

		if (asyncNotify != nullptr) {
		mResourceManagerProxy->markClientForPendingRemoval();
		postPendingRepliesAndDeferredMessages();
		postPendingRepliesAndDeferredMessages("kWhatRelease:async");
		asyncNotifyPost.clear();
		mAsyncReleaseCompleteNotification = asyncNotify;
		}
		@@ -4303,16 +4312,23 @@ status_t MediaCodec::amendOutputFormatWithCodecSpecificData(
		return OK;
		}

		void MediaCodec::postPendingRepliesAndDeferredMessages(status_t err /* = OK */) {
		void MediaCodec::postPendingRepliesAndDeferredMessages(
		std::string origin, status_t err /* = OK */) {
		sp<AMessage> response{new AMessage};
		if (err != OK) {
		response->setInt32("err", err);
		}
		postPendingRepliesAndDeferredMessages(response);
		postPendingRepliesAndDeferredMessages(origin, response);
		}

		void MediaCodec::postPendingRepliesAndDeferredMessages(const sp<AMessage> &response) {
		CHECK(mReplyID);
		void MediaCodec::postPendingRepliesAndDeferredMessages(
		std::string origin, const sp<AMessage> &response) {
		LOG_ALWAYS_FATAL_IF(
		!mReplyID,
		"postPendingRepliesAndDeferredMessages: mReplyID == null, from %s following %s",
		origin.c_str(),
		mLastReplyOrigin.c_str());
		mLastReplyOrigin = origin;
		response->postReply(mReplyID);
		mReplyID.clear();
		ALOGV_IF(!mDeferredMessages.empty(),

media/libstagefright/include/media/stagefright/MediaCodec.h

+3 −2

Original line number	Diff line number	Diff line
		@@ -366,6 +366,7 @@ private:
		AString mOwnerName;
		sp<MediaCodecInfo> mCodecInfo;
		sp<AReplyToken> mReplyID;
		std::string mLastReplyOrigin;
		std::vector<sp<AMessage>> mDeferredMessages;
		uint32_t mFlags;
		status_t mStickyError;
		@@ -489,8 +490,8 @@ private:
		bool hasPendingBuffer(int portIndex);
		bool hasPendingBuffer();

		void postPendingRepliesAndDeferredMessages(status_t err = OK);
		void postPendingRepliesAndDeferredMessages(const sp<AMessage> &response);
		void postPendingRepliesAndDeferredMessages(std::string origin, status_t err = OK);
		void postPendingRepliesAndDeferredMessages(std::string origin, const sp<AMessage> &response);

		/* called to get the last codec error when the sticky flag is set.
		* if no such codec error is found, returns UNKNOWN_ERROR.

media/libstagefright/tests/mediacodec/Android.bp

+5 −0

Original line number	Diff line number	Diff line
		@@ -23,7 +23,12 @@ cc_test {
		"MediaTestHelper.cpp",
		],

		header_libs: [
		"libmediadrm_headers",
		],

		shared_libs: [
		"libgui",
		"libmedia",
		"libmedia_codeclist",
		"libmediametrics",

media/libstagefright/tests/mediacodec/MediaCodecTest.cpp

+105 −23

Original line number	Diff line number	Diff line
		@@ -20,6 +20,8 @@
		#include <gmock/gmock.h>
		#include <gtest/gtest.h>

		#include <gui/Surface.h>
		#include <mediadrm/ICrypto.h>
		#include <media/stagefright/CodecBase.h>
		#include <media/stagefright/MediaCodec.h>
		#include <media/stagefright/MediaCodecListWriter.h>
		@@ -152,6 +154,37 @@ private:
		using namespace android;
		using ::testing::_;

		static sp<MediaCodec> SetupMediaCodec(
		const AString &owner,
		const AString &codecName,
		const AString &mediaType,
		const sp<ALooper> &looper,
		std::function<sp<CodecBase>(const AString &name, const char *owner)> getCodecBase) {
		std::shared_ptr<MediaCodecListWriter> listWriter =
		MediaTestHelper::CreateCodecListWriter();
		std::unique_ptr<MediaCodecInfoWriter> infoWriter = listWriter->addMediaCodecInfo();
		infoWriter->setName(codecName.c_str());
		infoWriter->setOwner(owner.c_str());
		infoWriter->addMediaType(mediaType.c_str());
		std::vector<sp<MediaCodecInfo>> codecInfos;
		MediaTestHelper::WriteCodecInfos(listWriter, &codecInfos);
		std::function<status_t(const AString &, sp<MediaCodecInfo> *)> getCodecInfo =
		[codecInfos](const AString &name, sp<MediaCodecInfo> *info) -> status_t {
		auto it = std::find_if(
		codecInfos.begin(), codecInfos.end(),
		[&name](const sp<MediaCodecInfo> &info) {
		return name.equalsIgnoreCase(info->getCodecName());
		});

		info = (it == codecInfos.end()) ? nullptr : it;
		return (*info) ? OK : NAME_NOT_FOUND;
		};

		looper->start();
		return MediaTestHelper::CreateCodec(
		codecName, looper, getCodecBase, getCodecInfo);
		}

		TEST(MediaCodecTest, ReclaimReleaseRace) {
		// Test scenario:
		//
		@@ -202,30 +235,9 @@ TEST(MediaCodecTest, ReclaimReleaseRace) {
		return mockCodec;
		};

		std::shared_ptr<MediaCodecListWriter> listWriter =
		MediaTestHelper::CreateCodecListWriter();
		std::unique_ptr<MediaCodecInfoWriter> infoWriter = listWriter->addMediaCodecInfo();
		infoWriter->setName(kCodecName.c_str());
		infoWriter->setOwner(kCodecOwner.c_str());
		infoWriter->addMediaType(kMediaType.c_str());
		std::vector<sp<MediaCodecInfo>> codecInfos;
		MediaTestHelper::WriteCodecInfos(listWriter, &codecInfos);
		std::function<status_t(const AString &, sp<MediaCodecInfo> *)> getCodecInfo =
		[codecInfos](const AString &name, sp<MediaCodecInfo> *info) -> status_t {
		auto it = std::find_if(
		codecInfos.begin(), codecInfos.end(),
		[&name](const sp<MediaCodecInfo> &info) {
		return name.equalsIgnoreCase(info->getCodecName());
		});

		info = (it == codecInfos.end()) ? nullptr : it;
		return (*info) ? OK : NAME_NOT_FOUND;
		};

		sp<ALooper> looper{new ALooper};
		looper->start();
		sp<MediaCodec> codec = MediaTestHelper::CreateCodec(
		kCodecName, looper, getCodecBase, getCodecInfo);
		sp<MediaCodec> codec = SetupMediaCodec(
		kCodecOwner, kCodecName, kMediaType, looper, getCodecBase);
		ASSERT_NE(nullptr, codec) << "Codec must not be null";
		ASSERT_NE(nullptr, mockCodec) << "MockCodec must not be null";
		std::promise<void> reclaimCompleted;
		@@ -266,3 +278,73 @@ TEST(MediaCodecTest, ReclaimReleaseRace) {
		<< "release timed out";
		looper->stop();
		}

		TEST(MediaCodecTest, ErrorWhileStopping) {
		// Test scenario:
		//
		// 1) Client thread calls stop(); MediaCodec looper thread calls
		// initiateShutdown(); shutdown is being handled at the component thread.
		// 2) Error occurred, but the shutdown operation is still being done.
		// 3) MediaCodec looper thread handles the error.
		// 4) Component thread completes shutdown and posts onStopCompleted()

		static const AString kCodecName{"test.codec"};
		static const AString kCodecOwner{"nobody"};
		static const AString kMediaType{"video/x-test"};

		std::promise<void> errorOccurred;
		sp<MockCodec> mockCodec;
		std::function<sp<CodecBase>(const AString &name, const char *owner)> getCodecBase =
		[&mockCodec, &errorOccurred](const AString &, const char *) {
		mockCodec = new MockCodec([](const std::shared_ptr<MockBufferChannel> &) {
		// No mock setup, as we don't expect any buffer operations
		// in this scenario.
		});
		ON_CALL(*mockCodec, initiateAllocateComponent(_))
		.WillByDefault([mockCodec](const sp<AMessage> &) {
		mockCodec->callback()->onComponentAllocated(kCodecName.c_str());
		});
		ON_CALL(*mockCodec, initiateConfigureComponent(_))
		.WillByDefault([mockCodec](const sp<AMessage> &msg) {
		mockCodec->callback()->onComponentConfigured(
		msg->dup(), msg->dup());
		});
		ON_CALL(*mockCodec, initiateStart())
		.WillByDefault([mockCodec]() {
		mockCodec->callback()->onStartCompleted();
		});
		ON_CALL(*mockCodec, initiateShutdown(true))
		.WillByDefault([mockCodec, &errorOccurred](bool) {
		mockCodec->callback()->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
		// Mark that 1) and 2) are complete.
		errorOccurred.set_value();
		});
		ON_CALL(*mockCodec, initiateShutdown(false))
		.WillByDefault([mockCodec](bool) {
		mockCodec->callback()->onReleaseCompleted();
		});
		return mockCodec;
		};

		sp<ALooper> looper{new ALooper};
		sp<MediaCodec> codec = SetupMediaCodec(
		kCodecOwner, kCodecName, kMediaType, looper, getCodecBase);
		ASSERT_NE(nullptr, codec) << "Codec must not be null";
		ASSERT_NE(nullptr, mockCodec) << "MockCodec must not be null";

		std::thread([mockCodec, &errorOccurred]{
		// Simulate component thread that handles stop()
		errorOccurred.get_future().wait();
		// Error occurred but shutdown request still got processed.
		mockCodec->callback()->onStopCompleted();
		}).detach();

		codec->configure(new AMessage, nullptr, nullptr, 0);
		codec->start();
		codec->stop();
		// Sleep here to give time for the MediaCodec looper thread
		// to process the messages.
		std::this_thread::sleep_for(std::chrono::milliseconds(100));
		codec->release();
		looper->stop();
		}