Release input buffers that correspond to no output
This CL makes the pipeline control aware of the error case in which the
output is simply not produced after input buffers are released.
Bug: 116297102
Test: m cts && cts-tradefed run cts -m CtsSecurityTestCases \
-t android.security.cts.StagefrightTest#testStagefright_cve_2016_3755
Test: m cts && cts-tradefed run cts -m CtsMediaTestCases \
-t android.media.cts.MediaCodecTest
Test: m cts && cts-tradefed run cts-dev -m CtsMediaTestCases \
--compatibility:module-arg \
CtsMediaTestCases:include-annotation:\
android.platform.test.annotations.RequiresDevice
Test: m cts && cts-tradefed run cts -m CtsSecurityTestCases
Change-Id: Ib24046d1e597b8ec379f58b58f9954af387be989
(cherry picked from commit 142fd2cd80d2adbd9eb61fd309e6d9871517a1ea)
diff --git a/codec2/hidl/client/client.cpp b/codec2/hidl/client/client.cpp
index 6ad6a15..ff67681 100644
--- a/codec2/hidl/client/client.cpp
+++ b/codec2/hidl/client/client.cpp
@@ -342,12 +342,13 @@
return Void();
}
// release input buffers potentially held by the component from queue
+ size_t numDiscardedInputBuffers = 0;
std::shared_ptr<Codec2Client::Component> strongComponent = component.lock();
if (strongComponent) {
- strongComponent->handleOnWorkDone(workItems);
+ numDiscardedInputBuffers = strongComponent->handleOnWorkDone(workItems);
}
if (std::shared_ptr<Codec2Client::Listener> listener = base.lock()) {
- listener->onWorkDone(component, workItems);
+ listener->onWorkDone(component, workItems, numDiscardedInputBuffers);
} else {
ALOGD("onWorkDone -- listener died.");
}
@@ -841,7 +842,7 @@
return static_cast<c2_status_t>(static_cast<Status>(transResult));
}
-void Codec2Client::Component::handleOnWorkDone(
+size_t Codec2Client::Component::handleOnWorkDone(
const std::list<std::unique_ptr<C2Work>> &workItems) {
// Input buffers' lifetime management
std::vector<uint64_t> inputDone;
@@ -856,6 +857,7 @@
}
}
+ size_t numDiscardedInputBuffers = 0;
{
std::lock_guard<std::mutex> lock(mInputBuffersMutex);
for (uint64_t inputIndex : inputDone) {
@@ -870,6 +872,7 @@
(long long)inputIndex, it->second.size());
mInputBuffers.erase(it);
mInputBufferCount.erase(inputIndex);
+ ++numDiscardedInputBuffers;
}
}
}
@@ -884,6 +887,7 @@
if (igbp) {
holdBufferQueueBlocks(workItems, igbp, bqId, generation);
}
+ return numDiscardedInputBuffers;
}
std::shared_ptr<C2Buffer> Codec2Client::Component::freeInputBuffer(
diff --git a/codec2/hidl/client/include/codec2/hidl/client.h b/codec2/hidl/client/include/codec2/hidl/client.h
index 01da733..fb59ad6 100644
--- a/codec2/hidl/client/include/codec2/hidl/client.h
+++ b/codec2/hidl/client/include/codec2/hidl/client.h
@@ -243,19 +243,31 @@
struct Codec2Client::Listener {
+ // This is called when the component produces some output.
+ //
+ // numDiscardedInputBuffers is the number of input buffers contained in
+ // workItems that have just become unused. Note that workItems may contain
+ // more input buffers than numDiscardedInputBuffers because buffers that
+ // have been previously reported by onInputBufferDone() are not counted
+ // towards numDiscardedInputBuffers, but may still show up in workItems.
virtual void onWorkDone(
const std::weak_ptr<Component>& comp,
- std::list<std::unique_ptr<C2Work>>& workItems) = 0;
+ std::list<std::unique_ptr<C2Work>>& workItems,
+ size_t numDiscardedInputBuffers) = 0;
+ // This is called when the component goes into a tripped state.
virtual void onTripped(
const std::weak_ptr<Component>& comp,
const std::vector<std::shared_ptr<C2SettingResult>>& settingResults
) = 0;
+ // This is called when the component encounters an error.
virtual void onError(
const std::weak_ptr<Component>& comp,
uint32_t errorCode) = 0;
+ // This is called when the process that hosts the component shuts down
+ // unexpectedly.
virtual void onDeath(
const std::weak_ptr<Component>& comp) = 0;
@@ -284,6 +296,7 @@
RenderedFrame(const RenderedFrame&) = default;
};
+ // This is called when the component becomes aware of frames being rendered.
virtual void onFramesRendered(
const std::vector<RenderedFrame>& renderedFrames) = 0;
@@ -408,7 +421,8 @@
friend struct Codec2Client;
struct HidlListener;
- void handleOnWorkDone(const std::list<std::unique_ptr<C2Work>> &workItems);
+ // Return the number of input buffers that should be discarded.
+ size_t handleOnWorkDone(const std::list<std::unique_ptr<C2Work>> &workItems);
// Remove an input buffer from mInputBuffers and return it.
std::shared_ptr<C2Buffer> freeInputBuffer(uint64_t frameIndex, size_t bufferIndex);
diff --git a/media/sfplugin/CCodec.cpp b/media/sfplugin/CCodec.cpp
index 7609642..baf9cb6 100644
--- a/media/sfplugin/CCodec.cpp
+++ b/media/sfplugin/CCodec.cpp
@@ -449,13 +449,14 @@
virtual void onWorkDone(
const std::weak_ptr<Codec2Client::Component>& component,
- std::list<std::unique_ptr<C2Work>>& workItems) override {
+ std::list<std::unique_ptr<C2Work>>& workItems,
+ size_t numDiscardedInputBuffers) override {
(void)component;
sp<CCodec> codec(mCodec.promote());
if (!codec) {
return;
}
- codec->onWorkDone(workItems);
+ codec->onWorkDone(workItems, numDiscardedInputBuffers);
}
virtual void onTripped(
@@ -1423,10 +1424,22 @@
config->setParameters(comp, params, C2_MAY_BLOCK);
}
-void CCodec::onWorkDone(std::list<std::unique_ptr<C2Work>> &workItems) {
- {
- Mutexed<std::list<std::unique_ptr<C2Work>>>::Locked queue(mWorkDoneQueue);
- queue->splice(queue->end(), workItems);
+void CCodec::onWorkDone(std::list<std::unique_ptr<C2Work>> &workItems,
+ size_t numDiscardedInputBuffers) {
+ if (!workItems.empty()) {
+ {
+ Mutexed<std::list<size_t>>::Locked numDiscardedInputBuffersQueue(
+ mNumDiscardedInputBuffersQueue);
+ numDiscardedInputBuffersQueue->insert(
+ numDiscardedInputBuffersQueue->end(),
+ workItems.size() - 1, 0);
+ numDiscardedInputBuffersQueue->emplace_back(
+ numDiscardedInputBuffers);
+ }
+ {
+ Mutexed<std::list<std::unique_ptr<C2Work>>>::Locked queue(mWorkDoneQueue);
+ queue->splice(queue->end(), workItems);
+ }
}
(new AMessage(kWhatWorkDone, this))->post();
}
@@ -1502,6 +1515,7 @@
}
case kWhatWorkDone: {
std::unique_ptr<C2Work> work;
+ size_t numDiscardedInputBuffers;
bool shouldPost = false;
{
Mutexed<std::list<std::unique_ptr<C2Work>>>::Locked queue(mWorkDoneQueue);
@@ -1512,6 +1526,16 @@
queue->pop_front();
shouldPost = !queue->empty();
}
+ {
+ Mutexed<std::list<size_t>>::Locked numDiscardedInputBuffersQueue(
+ mNumDiscardedInputBuffersQueue);
+ if (numDiscardedInputBuffersQueue->empty()) {
+ numDiscardedInputBuffers = 0;
+ } else {
+ numDiscardedInputBuffers = numDiscardedInputBuffersQueue->front();
+ numDiscardedInputBuffersQueue->pop_front();
+ }
+ }
if (shouldPost) {
(new AMessage(kWhatWorkDone, this))->post();
}
@@ -1579,7 +1603,8 @@
}
mChannel->onWorkDone(
std::move(work), changed ? config->mOutputFormat : nullptr,
- initData.hasChanged() ? initData.update().get() : nullptr);
+ initData.hasChanged() ? initData.update().get() : nullptr,
+ numDiscardedInputBuffers);
break;
}
case kWhatWatch: {
diff --git a/media/sfplugin/CCodec.h b/media/sfplugin/CCodec.h
index a02963c..78b009e 100644
--- a/media/sfplugin/CCodec.h
+++ b/media/sfplugin/CCodec.h
@@ -66,7 +66,8 @@
virtual void signalRequestIDRFrame() override;
void initiateReleaseIfStuck();
- void onWorkDone(std::list<std::unique_ptr<C2Work>> &workItems);
+ void onWorkDone(std::list<std::unique_ptr<C2Work>> &workItems,
+ size_t numDiscardedInputBuffers);
void onInputBufferDone(const std::shared_ptr<C2Buffer>& buffer);
protected:
@@ -172,6 +173,7 @@
typedef CCodecConfig Config;
Mutexed<Config> mConfig;
Mutexed<std::list<std::unique_ptr<C2Work>>> mWorkDoneQueue;
+ Mutexed<std::list<size_t>> mNumDiscardedInputBuffersQueue;
friend class CCodecCallbackImpl;
diff --git a/media/sfplugin/CCodecBufferChannel.cpp b/media/sfplugin/CCodecBufferChannel.cpp
index 90b33de..bf14e09 100644
--- a/media/sfplugin/CCodecBufferChannel.cpp
+++ b/media/sfplugin/CCodecBufferChannel.cpp
@@ -1330,7 +1330,7 @@
// CCodecBufferChannel::PipelineCapacity
CCodecBufferChannel::PipelineCapacity::PipelineCapacity()
- : input(0), lentInput(0), component(0), output(0),
+ : input(0), component(0), output(0),
mName("<UNKNOWN COMPONENT>") {
}
@@ -1341,15 +1341,14 @@
const char* newName,
const char* callerTag) {
input.store(newInput, std::memory_order_relaxed);
- lentInput.store(0, std::memory_order_relaxed);
component.store(newComponent, std::memory_order_relaxed);
output.store(newOutput, std::memory_order_relaxed);
mName = newName;
ALOGV("[%s] %s -- PipelineCapacity::initialize(): "
"pipeline availability initialized ==> "
- "input = %d (-%d), component = %d, output = %d",
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
- newInput, 0, newComponent, newOutput);
+ newInput, newComponent, newOutput);
}
bool CCodecBufferChannel::PipelineCapacity::allocate(const char* callerTag) {
@@ -1359,10 +1358,9 @@
if (prevInput > 0 && prevComponent > 0 && prevOutput > 0) {
ALOGV("[%s] %s -- PipelineCapacity::allocate() returns true: "
"pipeline availability -1 all ==> "
- "input = %d (-%d), component = %d, output = %d",
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
prevInput - 1,
- lentInput.load(std::memory_order_relaxed),
prevComponent - 1,
prevOutput - 1);
return true;
@@ -1372,10 +1370,9 @@
output.fetch_add(1, std::memory_order_relaxed);
ALOGV("[%s] %s -- PipelineCapacity::allocate() returns false: "
"pipeline availability unchanged ==> "
- "input = %d (-%d), component = %d, output = %d",
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
prevInput,
- lentInput.load(std::memory_order_relaxed),
prevComponent,
prevOutput);
return false;
@@ -1387,54 +1384,28 @@
int prevOutput = output.fetch_add(1, std::memory_order_relaxed);
ALOGV("[%s] %s -- PipelineCapacity::free(): "
"pipeline availability +1 all ==> "
- "input = %d (-%d), component = %d, output = %d",
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
prevInput + 1,
- lentInput.load(std::memory_order_relaxed),
prevComponent + 1,
prevOutput + 1);
}
-int CCodecBufferChannel::PipelineCapacity::lendInputSlot(
+int CCodecBufferChannel::PipelineCapacity::freeInputSlots(
+ size_t numDiscardedInputBuffers,
const char* callerTag) {
- int prevInput = input.fetch_add(1, std::memory_order_relaxed);
- int prevLentInput = lentInput.fetch_add(1, std::memory_order_relaxed);
- ALOGV("[%s] %s -- PipelineCapacity::lendInputSlot(): "
- "pipeline availability +1 (-1) input ==> "
- "input = %d (-%d), component = %d, output = %d",
+ int prevInput = input.fetch_add(numDiscardedInputBuffers,
+ std::memory_order_relaxed);
+ ALOGV("[%s] %s -- PipelineCapacity::freeInputSlots(%zu): "
+ "pipeline availability +%zu input ==> "
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
- prevInput + 1,
- prevLentInput + 1,
+ numDiscardedInputBuffers,
+ numDiscardedInputBuffers,
+ prevInput + static_cast<int>(numDiscardedInputBuffers),
component.load(std::memory_order_relaxed),
output.load(std::memory_order_relaxed));
- return prevInput + 1;
-}
-
-int CCodecBufferChannel::PipelineCapacity::freeInputSlot(
- const char* callerTag) {
- int prevLentInput = lentInput.fetch_sub(1, std::memory_order_relaxed);
- if (prevLentInput > 0) {
- ALOGV("[%s] %s -- PipelineCapacity::freeInputSlot(): "
- "pipeline availability +0 (+1) input ==> "
- "input = %d (-%d), component = %d, output = %d",
- mName, callerTag ? callerTag : "*",
- input.load(std::memory_order_relaxed),
- prevLentInput - 1,
- component.load(std::memory_order_relaxed),
- output.load(std::memory_order_relaxed));
- return input.load(std::memory_order_relaxed);
- }
- lentInput.fetch_add(1, std::memory_order_relaxed);
- int prevInput = input.fetch_add(1, std::memory_order_relaxed);
- ALOGV("[%s] %s -- PipelineCapacity::freeInputSlot(): "
- "pipeline availability +1 (+0) input ==> "
- "input = %d (-%d), component = %d, output = %d",
- mName, callerTag ? callerTag : "*",
- prevInput + 1,
- prevLentInput,
- component.load(std::memory_order_relaxed),
- output.load(std::memory_order_relaxed));
- return prevInput + 1;
+ return prevInput + numDiscardedInputBuffers;
}
int CCodecBufferChannel::PipelineCapacity::freeComponentSlot(
@@ -1442,10 +1413,9 @@
int prevComponent = component.fetch_add(1, std::memory_order_relaxed);
ALOGV("[%s] %s -- PipelineCapacity::freeComponentSlot(): "
"pipeline availability +1 component ==> "
- "input = %d (-%d), component = %d, output = %d",
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
input.load(std::memory_order_relaxed),
- lentInput.load(std::memory_order_relaxed),
prevComponent + 1,
output.load(std::memory_order_relaxed));
return prevComponent + 1;
@@ -1456,10 +1426,9 @@
int prevOutput = output.fetch_add(1, std::memory_order_relaxed);
ALOGV("[%s] %s -- PipelineCapacity::freeOutputSlot(): "
"pipeline availability +1 output ==> "
- "input = %d (-%d), component = %d, output = %d",
+ "input = %d, component = %d, output = %d",
mName, callerTag ? callerTag : "*",
input.load(std::memory_order_relaxed),
- lentInput.load(std::memory_order_relaxed),
component.load(std::memory_order_relaxed),
prevOutput + 1);
return prevOutput + 1;
@@ -1859,8 +1828,10 @@
bool released = false;
{
Mutexed<std::unique_ptr<InputBuffers>>::Locked buffers(mInputBuffers);
- if (*buffers) {
- released = (*buffers)->releaseBuffer(buffer, nullptr);
+ if (*buffers && (*buffers)->releaseBuffer(buffer, nullptr)) {
+ buffers.unlock();
+ released = true;
+ mAvailablePipelineCapacity.freeInputSlots(1, "discardBuffer");
}
}
{
@@ -2233,7 +2204,8 @@
}
for (const sp<MediaCodecBuffer> &buffer : toBeQueued) {
if (queueInputBufferInternal(buffer) != OK) {
- mAvailablePipelineCapacity.free("requestInitialInputBuffers");
+ mAvailablePipelineCapacity.freeComponentSlot("requestInitialInputBuffers");
+ mAvailablePipelineCapacity.freeOutputSlot("requestInitialInputBuffers");
}
}
return OK;
@@ -2283,8 +2255,10 @@
void CCodecBufferChannel::onWorkDone(
std::unique_ptr<C2Work> work, const sp<AMessage> &outputFormat,
- const C2StreamInitDataInfo::output *initData) {
- mAvailablePipelineCapacity.freeInputSlot("onWorkDone");
+ const C2StreamInitDataInfo::output *initData,
+ size_t numDiscardedInputBuffers) {
+ mAvailablePipelineCapacity.freeInputSlots(numDiscardedInputBuffers,
+ "onWorkDone");
mAvailablePipelineCapacity.freeComponentSlot("onWorkDone");
if (handleWork(std::move(work), outputFormat, initData)) {
mAvailablePipelineCapacity.freeOutputSlot("onWorkDone");
@@ -2300,7 +2274,7 @@
newInputSlotAvailable = (*buffers)->expireComponentBuffer(buffer);
}
if (newInputSlotAvailable) {
- mAvailablePipelineCapacity.lendInputSlot("onInputBufferDone");
+ mAvailablePipelineCapacity.freeInputSlots(1, "onInputBufferDone");
feedInputBufferIfAvailable();
}
}
diff --git a/media/sfplugin/CCodecBufferChannel.h b/media/sfplugin/CCodecBufferChannel.h
index f9e086f..8631235 100644
--- a/media/sfplugin/CCodecBufferChannel.h
+++ b/media/sfplugin/CCodecBufferChannel.h
@@ -125,10 +125,14 @@
* @param workItems finished work item.
* @param outputFormat new output format if it has changed, otherwise nullptr
* @param initData new init data (CSD) if it has changed, otherwise nullptr
+ * @param numDiscardedInputBuffers the number of input buffers that are
+ * returned for the first time (not previously returned by
+ * onInputBufferDone()).
*/
void onWorkDone(
std::unique_ptr<C2Work> work, const sp<AMessage> &outputFormat,
- const C2StreamInitDataInfo::output *initData);
+ const C2StreamInitDataInfo::output *initData,
+ size_t numDiscardedInputBuffers);
/**
* Make an input buffer available for the client as it is no longer needed
@@ -283,11 +287,6 @@
struct PipelineCapacity {
// The number of available input capacity.
std::atomic_int input;
- // The number of input buffers that have been released by
- // onInputBufferDone() but not onWorkDone(). Once onWorkDone() is
- // called, this number will decrease unless it is already zero; in
- // which case #input will increase instead.
- std::atomic_int lentInput;
// The number of available component capacity.
std::atomic_int component;
// The number of available output capacity.
@@ -318,41 +317,31 @@
// essentially undoes an allocate() call.
void free(const char* callerTag = nullptr);
- // Increase #input and #lentInput by 1.
+ // Increase #input by @p numDiscardedInputBuffers.
//
// callerTag is used for logging only.
//
- // lendInputSlot() is called by CCodecBufferChannel when
- // onInputBufferDone() is called. This means an input buffer has been
- // freed, but a subsequent call to onWorkDone() will not free an input
- // buffer.
- int lendInputSlot(const char* callerTag = nullptr);
-
- // Increase #input by one if #lentInput is 0; otherwise, decrease
- // #lentInput by 1.
- //
- // callerTag is used for logging only.
- //
- // freeInputSlot() is called by CCodecBufferChannel when onWorkDone() is
- // called. If #lentInput is not zero, that means the input buffer for
- // the returned work has already been freed, so #input will not
- // increase.
- int freeInputSlot(const char* callerTag = nullptr);
+ // freeInputSlots() is called by CCodecBufferChannel when onWorkDone()
+ // or onInputBufferDone() is called. @p numDiscardedInputBuffers is
+ // provided in onWorkDone(), and is 1 in onInputBufferDone().
+ int freeInputSlots(size_t numDiscardedInputBuffers,
+ const char* callerTag = nullptr);
// Increase #component by one and return the updated value.
//
// callerTag is used for logging only.
//
- // freeComponentSlot() is called by CCodecBufferChannel when onWorkDone() is
- // called.
+ // freeComponentSlot() is called by CCodecBufferChannel when
+ // onWorkDone() is called.
int freeComponentSlot(const char* callerTag = nullptr);
// Increase #output by one and return the updated value.
//
// callerTag is used for logging only.
//
- // freeOutputSlot() is called by CCodecBufferChannel when discardBuffer() is
- // called on an output buffer or when renderOutputBuffer() is called.
+ // freeOutputSlot() is called by CCodecBufferChannel when
+ // discardBuffer() is called on an output buffer or when
+ // renderOutputBuffer() is called.
int freeOutputSlot(const char* callerTag = nullptr);
private:
