CodecAsyncHandler: Fix race issue in async mode
Modified await on input/output queues. In earlier design, in a scenario,
async handler thread waits on input/output queues even if the queue is
not empty. This is because of lock-release-relock between isEmpty() and
mCondition.await().
Bug: 149031058
Test: atest android.mediav2.cts
Change-Id: Iee592e94704c4011d2cd336be5f5fd3a3a03fb78
diff --git a/tests/media/src/android/mediav2/cts/CodecDecoderTest.java b/tests/media/src/android/mediav2/cts/CodecDecoderTest.java
index cc58caf..1aeb375 100644
--- a/tests/media/src/android/mediav2/cts/CodecDecoderTest.java
+++ b/tests/media/src/android/mediav2/cts/CodecDecoderTest.java
@@ -793,7 +793,6 @@
* Test Decoder by Queuing CSD separately
*/
@LargeTest
- @Ignore("TODO(b/149031058)")
@Test(timeout = PER_TEST_TIMEOUT_LARGE_TEST_MS)
public void testSimpleDecodeQueueCSD() throws IOException, InterruptedException {
MediaFormat format = setUpSource(mTestFile);
diff --git a/tests/media/src/android/mediav2/cts/CodecTestBase.java b/tests/media/src/android/mediav2/cts/CodecTestBase.java
index a09cf5a..739b60a 100644
--- a/tests/media/src/android/mediav2/cts/CodecTestBase.java
+++ b/tests/media/src/android/mediav2/cts/CodecTestBase.java
@@ -45,22 +45,24 @@
private static final String LOG_TAG = CodecAsyncHandler.class.getSimpleName();
private final Lock mLock = new ReentrantLock();
private final Condition mCondition = mLock.newCondition();
- private CallBackQueue mCbInputQueue;
- private CallBackQueue mCbOutputQueue;
+ private final LinkedList<Pair<Integer, MediaCodec.BufferInfo>> mCbInputQueue;
+ private final LinkedList<Pair<Integer, MediaCodec.BufferInfo>> mCbOutputQueue;
private MediaFormat mOutFormat;
private boolean mSignalledOutFormatChanged;
private volatile boolean mSignalledError;
CodecAsyncHandler() {
- mCbInputQueue = new CallBackQueue();
- mCbOutputQueue = new CallBackQueue();
+ mCbInputQueue = new LinkedList<>();
+ mCbOutputQueue = new LinkedList<>();
mSignalledError = false;
mSignalledOutFormatChanged = false;
}
void clearQueues() {
+ mLock.lock();
mCbInputQueue.clear();
mCbOutputQueue.clear();
+ mLock.unlock();
}
void resetContext() {
@@ -73,14 +75,20 @@
@Override
public void onInputBufferAvailable(@NonNull MediaCodec codec, int bufferIndex) {
assertTrue(bufferIndex >= 0);
- mCbInputQueue.push(new Pair<>(bufferIndex, (MediaCodec.BufferInfo) null));
+ mLock.lock();
+ mCbInputQueue.add(new Pair<>(bufferIndex, (MediaCodec.BufferInfo) null));
+ mCondition.signalAll();
+ mLock.unlock();
}
@Override
public void onOutputBufferAvailable(@NonNull MediaCodec codec, int bufferIndex,
@NonNull MediaCodec.BufferInfo info) {
assertTrue(bufferIndex >= 0);
- mCbOutputQueue.push(new Pair<>(bufferIndex, info));
+ mLock.lock();
+ mCbOutputQueue.add(new Pair<>(bufferIndex, info));
+ mCondition.signalAll();
+ mLock.unlock();
}
@Override
@@ -109,51 +117,52 @@
Pair<Integer, MediaCodec.BufferInfo> getInput() throws InterruptedException {
Pair<Integer, MediaCodec.BufferInfo> element = null;
+ mLock.lock();
while (!mSignalledError) {
if (mCbInputQueue.isEmpty()) {
- mLock.lock();
mCondition.await();
- mLock.unlock();
} else {
- element = mCbInputQueue.pop();
+ element = mCbInputQueue.remove(0);
break;
}
}
+ mLock.unlock();
return element;
}
Pair<Integer, MediaCodec.BufferInfo> getOutput() throws InterruptedException {
Pair<Integer, MediaCodec.BufferInfo> element = null;
+ mLock.lock();
while (!mSignalledError) {
if (mCbOutputQueue.isEmpty()) {
- mLock.lock();
mCondition.await();
- mLock.unlock();
} else {
- element = mCbOutputQueue.pop();
+ element = mCbOutputQueue.remove(0);
break;
}
}
+ mLock.unlock();
return element;
}
Pair<Integer, MediaCodec.BufferInfo> getWork() throws InterruptedException {
Pair<Integer, MediaCodec.BufferInfo> element = null;
+ mLock.lock();
while (!mSignalledError) {
if (mCbInputQueue.isEmpty() && mCbOutputQueue.isEmpty()) {
- mLock.lock();
mCondition.await();
- mLock.unlock();
- }
- if (!mCbOutputQueue.isEmpty()) {
- element = mCbOutputQueue.pop();
- break;
- }
- if (!mCbInputQueue.isEmpty()) {
- element = mCbInputQueue.pop();
- break;
+ } else {
+ if (!mCbOutputQueue.isEmpty()) {
+ element = mCbOutputQueue.remove(0);
+ break;
+ }
+ if (!mCbInputQueue.isEmpty()) {
+ element = mCbInputQueue.remove(0);
+ break;
+ }
}
}
+ mLock.unlock();
return element;
}
@@ -168,38 +177,6 @@
MediaFormat getOutputFormat() {
return mOutFormat;
}
-
- private class CallBackQueue {
- private final LinkedList<Pair<Integer, MediaCodec.BufferInfo>> mQueue = new LinkedList<>();
-
- void push(Pair<Integer, MediaCodec.BufferInfo> element) {
- mLock.lock();
- mQueue.add(element);
- mCondition.signalAll();
- mLock.unlock();
- }
-
- Pair<Integer, MediaCodec.BufferInfo> pop() {
- Pair<Integer, MediaCodec.BufferInfo> element = null;
- mLock.lock();
- if (mQueue.size() != 0) element = mQueue.remove(0);
- mLock.unlock();
- return element;
- }
-
- boolean isEmpty() {
- mLock.lock();
- boolean isEmpty = (mQueue.size() == 0);
- mLock.unlock();
- return isEmpty;
- }
-
- void clear() {
- mLock.lock();
- mQueue.clear();
- mLock.unlock();
- }
- }
}
class OutputManager {
@@ -461,8 +438,10 @@
int bufferID = element.first;
MediaCodec.BufferInfo info = element.second;
if (info != null) {
+ // <id, info> corresponds to output callback. Handle it accordingly
dequeueOutput(bufferID, info);
} else {
+ // <id, null> corresponds to input callback. Handle it accordingly
enqueueInput(bufferID);
frameCount++;
}
