CCodec: fix corner cases
- Catch invalid createInputSurface requests.
- Move buffer allocation from setComponent() to start().
- Reset input surface at stop().
- Ignore flushed work.
Bug: 74073134
Test: adb shell setprop debug.stagefright.ccodec yes
Test: adb shell setprop debug.stagefright.omx_default_rank 1000
Test: adb shell killall mediaserver
Test: atest android.media.cts.MediaCodecTest
Change-Id: If95d3050e14fa9592d7eff4136a57326008455a0
(cherry picked from commit e1c3b395209f2c03a290bf0c91c4d96dff5edf00)
diff --git a/media/libstagefright/CCodec.cpp b/media/libstagefright/CCodec.cpp
index f62ee41..a144382 100644
--- a/media/libstagefright/CCodec.cpp
+++ b/media/libstagefright/CCodec.cpp
@@ -238,14 +238,25 @@
return mChannel;
}
+status_t CCodec::tryAndReportOnError(std::function<status_t()> job) {
+ status_t err = job();
+ if (err != C2_OK) {
+ mCallback->onError(err, ACTION_CODE_FATAL);
+ }
+ return err;
+}
+
void CCodec::initiateAllocateComponent(const sp<AMessage> &msg) {
- {
+ auto setAllocating = [this] {
Mutexed<State>::Locked state(mState);
if (state->get() != RELEASED) {
- mCallback->onError(INVALID_OPERATION, ACTION_CODE_FATAL);
- return;
+ return INVALID_OPERATION;
}
state->set(ALLOCATING);
+ return OK;
+ };
+ if (tryAndReportOnError(setAllocating) != OK) {
+ return;
}
AString componentName;
@@ -259,7 +270,7 @@
}
void CCodec::allocate(const AString &componentName) {
- // TODO: use C2ComponentStore to create component
+ ALOGV("allocate(%s)", componentName.c_str());
mListener.reset(new CCodecListener(this));
std::shared_ptr<C2Component> comp;
@@ -282,29 +293,30 @@
}
ALOGV("Success Create component: %s", componentName.c_str());
comp->setListener_vb(mListener, C2_MAY_BLOCK);
- {
+ mChannel->setComponent(comp);
+ auto setAllocated = [this, comp] {
Mutexed<State>::Locked state(mState);
if (state->get() != ALLOCATING) {
state->set(RELEASED);
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
state->set(ALLOCATED);
state->comp = comp;
+ return OK;
+ };
+ if (tryAndReportOnError(setAllocated) != OK) {
+ return;
}
- mChannel->setComponent(comp);
mCallback->onComponentAllocated(comp->intf()->getName().c_str());
}
void CCodec::initiateConfigureComponent(const sp<AMessage> &format) {
- {
+ auto checkAllocated = [this] {
Mutexed<State>::Locked state(mState);
- if (state->get() != ALLOCATED) {
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- return;
- }
+ return (state->get() != ALLOCATED) ? UNKNOWN_ERROR : OK;
+ };
+ if (tryAndReportOnError(checkAllocated) != OK) {
+ return;
}
sp<AMessage> msg(new AMessage(kWhatConfigure, this));
@@ -314,21 +326,22 @@
void CCodec::configure(const sp<AMessage> &msg) {
std::shared_ptr<C2ComponentInterface> intf;
- {
+ auto checkAllocated = [this, &intf] {
Mutexed<State>::Locked state(mState);
if (state->get() != ALLOCATED) {
state->set(RELEASED);
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
intf = state->comp->intf();
+ return OK;
+ };
+ if (tryAndReportOnError(checkAllocated) != OK) {
+ return;
}
sp<AMessage> inputFormat(new AMessage);
sp<AMessage> outputFormat(new AMessage);
- if (status_t err = [=] {
+ auto doConfig = [=] {
AString mime;
if (!msg->findString("mime", &mime)) {
return BAD_VALUE;
@@ -339,6 +352,11 @@
encoder = false;
}
+ // TODO: read from intf()
+ if ((!encoder) != (intf->getName().find("encoder") == std::string::npos)) {
+ return UNKNOWN_ERROR;
+ }
+
sp<RefBase> obj;
if (msg->findObject("native-window", &obj)) {
sp<Surface> surface = static_cast<Surface *>(obj.get());
@@ -392,8 +410,8 @@
// TODO
return OK;
- }() != OK) {
- mCallback->onError(err, ACTION_CODE_FATAL);
+ };
+ if (tryAndReportOnError(doConfig) != OK) {
return;
}
@@ -406,6 +424,22 @@
}
void CCodec::initiateCreateInputSurface() {
+ status_t err = [this] {
+ Mutexed<State>::Locked state(mState);
+ if (state->get() != ALLOCATED) {
+ return UNKNOWN_ERROR;
+ }
+ // TODO: read it from intf() properly.
+ if (state->comp->intf()->getName().find("encoder") == std::string::npos) {
+ return INVALID_OPERATION;
+ }
+ return OK;
+ }();
+ if (err != OK) {
+ mCallback->onInputSurfaceCreationFailed(err);
+ return;
+ }
+
(new AMessage(kWhatCreateInputSurface, this))->post();
}
@@ -477,15 +511,16 @@
}
void CCodec::initiateStart() {
- {
+ auto setStarting = [this] {
Mutexed<State>::Locked state(mState);
if (state->get() != ALLOCATED) {
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
state->set(STARTING);
+ return OK;
+ };
+ if (tryAndReportOnError(setStarting) != OK) {
+ return;
}
(new AMessage(kWhatStart, this))->post();
@@ -493,16 +528,18 @@
void CCodec::start() {
std::shared_ptr<C2Component> comp;
- {
+ auto checkStarting = [this, &comp] {
Mutexed<State>::Locked state(mState);
if (state->get() != STARTING) {
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
comp = state->comp;
+ return OK;
+ };
+ if (tryAndReportOnError(checkStarting) != OK) {
+ return;
}
+
c2_status_t err = comp->start();
if (err != C2_OK) {
// TODO: convert err into status_t
@@ -516,17 +553,22 @@
inputFormat = formats->inputFormat;
outputFormat = formats->outputFormat;
}
- mChannel->start(inputFormat, outputFormat);
+ status_t err2 = mChannel->start(inputFormat, outputFormat);
+ if (err2 != OK) {
+ mCallback->onError(err2, ACTION_CODE_FATAL);
+ return;
+ }
- {
+ auto setRunning = [this] {
Mutexed<State>::Locked state(mState);
if (state->get() != STARTING) {
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
state->set(RUNNING);
+ return OK;
+ };
+ if (tryAndReportOnError(setRunning) != OK) {
+ return;
}
mCallback->onStartCompleted();
}
@@ -652,13 +694,26 @@
}
void CCodec::signalFlush() {
- {
+ status_t err = [this] {
Mutexed<State>::Locked state(mState);
+ if (state->get() == FLUSHED) {
+ return ALREADY_EXISTS;
+ }
if (state->get() != RUNNING) {
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- return;
+ return UNKNOWN_ERROR;
}
state->set(FLUSHING);
+ return OK;
+ }();
+ switch (err) {
+ case ALREADY_EXISTS:
+ mCallback->onFlushCompleted();
+ return;
+ case OK:
+ break;
+ default:
+ mCallback->onError(err, ACTION_CODE_FATAL);
+ return;
}
(new AMessage(kWhatFlush, this))->post();
@@ -666,15 +721,16 @@
void CCodec::flush() {
std::shared_ptr<C2Component> comp;
- {
+ auto checkFlushing = [this, &comp] {
Mutexed<State>::Locked state(mState);
if (state->get() != FLUSHING) {
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
comp = state->comp;
+ return OK;
+ };
+ if (tryAndReportOnError(checkFlushing) != OK) {
+ return;
}
mChannel->stop();
@@ -696,18 +752,19 @@
}
void CCodec::signalResume() {
- {
+ auto setResuming = [this] {
Mutexed<State>::Locked state(mState);
if (state->get() != FLUSHED) {
- state.unlock();
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
- state.lock();
- return;
+ return UNKNOWN_ERROR;
}
state->set(RESUMING);
+ return OK;
+ };
+ if (tryAndReportOnError(setResuming) != OK) {
+ return;
}
- mChannel->start(nullptr, nullptr);
+ (void)mChannel->start(nullptr, nullptr);
{
Mutexed<State>::Locked state(mState);
@@ -727,6 +784,8 @@
}
void CCodec::signalEndOfInputStream() {
+ // TODO
+ mCallback->onSignaledInputEOS(INVALID_OPERATION);
}
void CCodec::signalRequestIDRFrame() {
@@ -735,9 +794,7 @@
void CCodec::onWorkDone(std::list<std::unique_ptr<C2Work>> &workItems) {
Mutexed<std::list<std::unique_ptr<C2Work>>>::Locked queue(mWorkDoneQueue);
- for (std::unique_ptr<C2Work> &item : workItems) {
- queue->push_back(std::move(item));
- }
+ queue->splice(queue->end(), workItems);
(new AMessage(kWhatWorkDone, this))->post();
}
@@ -834,8 +891,8 @@
}
ALOGW("previous call to %s exceeded timeout", name.c_str());
- mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
initiateRelease(false);
+ mCallback->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
}
} // namespace android
diff --git a/media/libstagefright/CCodecBufferChannel.cpp b/media/libstagefright/CCodecBufferChannel.cpp
index 5ce49d9..de78c80 100644
--- a/media/libstagefright/CCodecBufferChannel.cpp
+++ b/media/libstagefright/CCodecBufferChannel.cpp
@@ -60,7 +60,10 @@
/**
* Set format for MediaCodec-facing buffers.
*/
- void setFormat(const sp<AMessage> &format) { mFormat = format; }
+ void setFormat(const sp<AMessage> &format) {
+ CHECK(format != nullptr);
+ mFormat = format;
+ }
/**
* Returns true if the buffers are operating under array mode.
@@ -585,6 +588,7 @@
size_t *index,
sp<MediaCodecBuffer> *clientBuffer) override {
sp<Codec2Buffer> newBuffer = wrap(buffer);
+ newBuffer->setFormat(mFormat);
*index = mImpl.assignSlot(newBuffer);
*clientBuffer = newBuffer;
return true;
@@ -735,55 +739,6 @@
void CCodecBufferChannel::setComponent(const std::shared_ptr<C2Component> &component) {
mComponent = component;
-
- C2StreamFormatConfig::input inputFormat(0u);
- C2StreamFormatConfig::output outputFormat(0u);
- c2_status_t err = mComponent->intf()->query_vb(
- { &inputFormat, &outputFormat },
- {},
- C2_DONT_BLOCK,
- nullptr);
- if (err != C2_OK) {
- // TODO: error
- return;
- }
-
- {
- Mutexed<std::unique_ptr<InputBuffers>>::Locked buffers(mInputBuffers);
-
- bool graphic = (inputFormat.value == C2FormatVideo);
- if (graphic) {
- buffers->reset(new GraphicInputBuffers);
- } else {
- buffers->reset(new LinearInputBuffers);
- }
-
- ALOGV("graphic = %s", graphic ? "true" : "false");
- std::shared_ptr<C2BlockPool> pool;
- if (graphic) {
- err = GetCodec2BlockPool(C2BlockPool::BASIC_GRAPHIC, component, &pool);
- } else {
- err = CreateCodec2BlockPool(C2PlatformAllocatorStore::ION,
- component, &pool);
- }
-
- if (err == C2_OK) {
- (*buffers)->setPool(pool);
- } else {
- // TODO: error
- }
- }
-
- {
- Mutexed<std::unique_ptr<OutputBuffers>>::Locked buffers(mOutputBuffers);
-
- bool graphic = (outputFormat.value == C2FormatVideo);
- if (graphic) {
- buffers->reset(new GraphicOutputBuffers);
- } else {
- buffers->reset(new LinearOutputBuffers);
- }
- }
}
status_t CCodecBufferChannel::setInputSurface(
@@ -812,6 +767,7 @@
if (buffer->meta()->findInt32("csd", &tmp) && tmp) {
flags |= C2FrameData::FLAG_CODEC_CONFIG;
}
+ ALOGV("queueInputBuffer: buffer->size() = %zu", buffer->size());
std::unique_ptr<C2Work> work(new C2Work);
work->input.flags = (C2FrameData::flags_t)flags;
work->input.ordinal.timestamp = timeUs;
@@ -968,13 +924,54 @@
(*buffers)->getArray(array);
}
-void CCodecBufferChannel::start(const sp<AMessage> &inputFormat, const sp<AMessage> &outputFormat) {
+status_t CCodecBufferChannel::start(
+ const sp<AMessage> &inputFormat, const sp<AMessage> &outputFormat) {
+ C2StreamFormatConfig::input iStreamFormat(0u);
+ C2StreamFormatConfig::output oStreamFormat(0u);
+ c2_status_t err = mComponent->intf()->query_vb(
+ { &iStreamFormat, &oStreamFormat },
+ {},
+ C2_DONT_BLOCK,
+ nullptr);
+ if (err != C2_OK) {
+ return UNKNOWN_ERROR;
+ }
+
if (inputFormat != nullptr) {
Mutexed<std::unique_ptr<InputBuffers>>::Locked buffers(mInputBuffers);
+
+ bool graphic = (iStreamFormat.value == C2FormatVideo);
+ if (graphic) {
+ buffers->reset(new GraphicInputBuffers);
+ } else {
+ buffers->reset(new LinearInputBuffers);
+ }
(*buffers)->setFormat(inputFormat);
+
+ ALOGV("graphic = %s", graphic ? "true" : "false");
+ std::shared_ptr<C2BlockPool> pool;
+ if (graphic) {
+ err = GetCodec2BlockPool(C2BlockPool::BASIC_GRAPHIC, mComponent, &pool);
+ } else {
+ err = CreateCodec2BlockPool(C2PlatformAllocatorStore::ION,
+ mComponent, &pool);
+ }
+ if (err == C2_OK) {
+ (*buffers)->setPool(pool);
+ } else {
+ // TODO: error
+ }
}
+
if (outputFormat != nullptr) {
Mutexed<std::unique_ptr<OutputBuffers>>::Locked buffers(mOutputBuffers);
+
+ bool graphic = (oStreamFormat.value == C2FormatVideo);
+ if (graphic) {
+ buffers->reset(new GraphicOutputBuffers);
+ } else {
+ buffers->reset(new LinearOutputBuffers);
+ }
(*buffers)->setFormat(outputFormat);
}
@@ -989,9 +986,7 @@
if (!(*buffers)->requestNewBuffer(&index, &buffer)) {
if (i == 0) {
ALOGE("start: cannot allocate memory at all");
- buffers.unlock();
- mOnError(NO_MEMORY, ACTION_CODE_FATAL);
- buffers.lock();
+ return NO_MEMORY;
} else {
ALOGV("start: cannot allocate memory, only %zu buffers allocated", i);
}
@@ -1003,6 +998,7 @@
} else {
(void)mInputSurface->connect(mComponent);
}
+ return OK;
}
void CCodecBufferChannel::stop() {
@@ -1010,6 +1006,7 @@
mFirstValidFrameIndex = mFrameIndex.load();
if (mInputSurface != nullptr) {
mInputSurface->disconnect();
+ mInputSurface.reset();
}
}
@@ -1025,8 +1022,13 @@
}
void CCodecBufferChannel::onWorkDone(const std::unique_ptr<C2Work> &work) {
- if (work->result != OK) {
- ALOGE("work failed to complete: %d", work->result);
+ if (work->result != C2_OK) {
+ if (work->result == C2_NOT_FOUND) {
+ // TODO: Define what flushed work's result is.
+ ALOGD("flushed work; ignored.");
+ return;
+ }
+ ALOGD("work failed to complete: %d", work->result);
mOnError(work->result, ACTION_CODE_FATAL);
return;
}
diff --git a/media/libstagefright/include/CCodecBufferChannel.h b/media/libstagefright/include/CCodecBufferChannel.h
index eb3255f..51eee10 100644
--- a/media/libstagefright/include/CCodecBufferChannel.h
+++ b/media/libstagefright/include/CCodecBufferChannel.h
@@ -88,7 +88,7 @@
* Start queueing buffers to the component. This object should never queue
* buffers before this call.
*/
- void start(const sp<AMessage> &inputFormat, const sp<AMessage> &outputFormat);
+ status_t start(const sp<AMessage> &inputFormat, const sp<AMessage> &outputFormat);
/**
* Stop queueing buffers to the component. This object should never queue
diff --git a/media/libstagefright/include/media/stagefright/CCodec.h b/media/libstagefright/include/media/stagefright/CCodec.h
index 3a2670d..078b03e 100644
--- a/media/libstagefright/include/media/stagefright/CCodec.h
+++ b/media/libstagefright/include/media/stagefright/CCodec.h
@@ -69,6 +69,8 @@
private:
typedef std::chrono::time_point<std::chrono::steady_clock> TimePoint;
+ status_t tryAndReportOnError(std::function<status_t()> job);
+
void initiateStop();
void initiateRelease(bool sendCallback = true);