Merge changes from topic 'vts-audio-fix' into oc-dev
* changes:
Audio HAL: Destroy EventFlag on failed prepareTo{write,read}
Audio HAL: Check for buffer size overflow
Audio HAL: Detect buffer memory allocation failure
Audio HAL: Detect openDevice failure
Audio HAL VTS: Fix documentation
diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp
index 2745607..0798bbe 100644
--- a/audio/2.0/default/StreamIn.cpp
+++ b/audio/2.0/default/StreamIn.cpp
@@ -20,6 +20,7 @@
#include <android/log.h>
#include <hardware/audio.h>
+#include <memory>
#include <utils/Trace.h>
#include "StreamIn.h"
@@ -52,7 +53,11 @@
mDataMQ(dataMQ),
mStatusMQ(statusMQ),
mEfGroup(efGroup),
- mBuffer(new uint8_t[dataMQ->getQuantumCount()]) {
+ mBuffer(nullptr) {
+ }
+ bool init() {
+ mBuffer.reset(new(std::nothrow) uint8_t[mDataMQ->getQuantumCount()]);
+ return mBuffer != nullptr;
}
virtual ~ReadThread() {}
@@ -308,8 +313,14 @@
return Void();
}
std::unique_ptr<CommandMQ> tempCommandMQ(new CommandMQ(1));
- std::unique_ptr<DataMQ> tempDataMQ(
- new DataMQ(frameSize * framesCount, true /* EventFlag */));
+ if (frameSize > std::numeric_limits<size_t>::max() / framesCount) {
+ ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", frameSize, framesCount);
+ _hidl_cb(Result::INVALID_ARGUMENTS,
+ CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
+ return Void();
+ }
+ std::unique_ptr<DataMQ> tempDataMQ(new DataMQ(frameSize * framesCount, true /* EventFlag */));
+
std::unique_ptr<StatusMQ> tempStatusMQ(new StatusMQ(1));
if (!tempCommandMQ->isValid() || !tempDataMQ->isValid() || !tempStatusMQ->isValid()) {
ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid");
@@ -319,8 +330,11 @@
CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
return Void();
}
- status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), &mEfGroup);
- if (status != OK || !mEfGroup) {
+ EventFlag* tempRawEfGroup{};
+ status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), &tempRawEfGroup);
+ std::unique_ptr<EventFlag, void(*)(EventFlag*)> tempElfGroup(tempRawEfGroup, [](auto *ef) {
+ EventFlag::deleteEventFlag(&ef); });
+ if (status != OK || !tempElfGroup) {
ALOGE("failed creating event flag for data MQ: %s", strerror(-status));
_hidl_cb(Result::INVALID_ARGUMENTS,
CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
@@ -328,13 +342,18 @@
}
// Create and launch the thread.
- mReadThread = new ReadThread(
+ auto tempReadThread = std::make_unique<ReadThread>(
&mStopReadThread,
mStream,
tempCommandMQ.get(),
tempDataMQ.get(),
tempStatusMQ.get(),
- mEfGroup);
+ tempElfGroup.get());
+ if (!tempReadThread->init()) {
+ _hidl_cb(Result::INVALID_ARGUMENTS,
+ CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
+ return Void();
+ }
status = mReadThread->run("reader", PRIORITY_URGENT_AUDIO);
if (status != OK) {
ALOGW("failed to start reader thread: %s", strerror(-status));
@@ -346,6 +365,8 @@
mCommandMQ = std::move(tempCommandMQ);
mDataMQ = std::move(tempDataMQ);
mStatusMQ = std::move(tempStatusMQ);
+ mReadThread = tempReadThread.release();
+ mEfGroup = tempElfGroup.release();
threadInfo.pid = getpid();
threadInfo.tid = mReadThread->getTid();
_hidl_cb(Result::OK,
diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp
index 88045a0..3339b63 100644
--- a/audio/2.0/default/StreamOut.cpp
+++ b/audio/2.0/default/StreamOut.cpp
@@ -18,6 +18,8 @@
//#define LOG_NDEBUG 0
#define ATRACE_TAG ATRACE_TAG_AUDIO
+#include <memory>
+
#include <android/log.h>
#include <hardware/audio.h>
#include <utils/Trace.h>
@@ -50,7 +52,11 @@
mDataMQ(dataMQ),
mStatusMQ(statusMQ),
mEfGroup(efGroup),
- mBuffer(new uint8_t[dataMQ->getQuantumCount()]) {
+ mBuffer(nullptr) {
+ }
+ bool init() {
+ mBuffer.reset(new(std::nothrow) uint8_t[mDataMQ->getQuantumCount()]);
+ return mBuffer != nullptr;
}
virtual ~WriteThread() {}
@@ -291,8 +297,15 @@
return Void();
}
std::unique_ptr<CommandMQ> tempCommandMQ(new CommandMQ(1));
- std::unique_ptr<DataMQ> tempDataMQ(
- new DataMQ(frameSize * framesCount, true /* EventFlag */));
+
+ if (frameSize > std::numeric_limits<size_t>::max() / framesCount) {
+ ALOGE("Requested buffer is too big, %d*%d can not fit in size_t", frameSize, framesCount);
+ _hidl_cb(Result::INVALID_ARGUMENTS,
+ CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
+ return Void();
+ }
+ std::unique_ptr<DataMQ> tempDataMQ(new DataMQ(frameSize * framesCount, true /* EventFlag */));
+
std::unique_ptr<StatusMQ> tempStatusMQ(new StatusMQ(1));
if (!tempCommandMQ->isValid() || !tempDataMQ->isValid() || !tempStatusMQ->isValid()) {
ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid");
@@ -302,8 +315,11 @@
CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
return Void();
}
- status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), &mEfGroup);
- if (status != OK || !mEfGroup) {
+ EventFlag* tempRawEfGroup{};
+ status = EventFlag::createEventFlag(tempDataMQ->getEventFlagWord(), &tempRawEfGroup);
+ std::unique_ptr<EventFlag, void(*)(EventFlag*)> tempElfGroup(tempRawEfGroup,[](auto *ef) {
+ EventFlag::deleteEventFlag(&ef); });
+ if (status != OK || !tempElfGroup) {
ALOGE("failed creating event flag for data MQ: %s", strerror(-status));
_hidl_cb(Result::INVALID_ARGUMENTS,
CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
@@ -311,14 +327,19 @@
}
// Create and launch the thread.
- mWriteThread = new WriteThread(
+ auto tempWriteThread = std::make_unique<WriteThread>(
&mStopWriteThread,
mStream,
tempCommandMQ.get(),
tempDataMQ.get(),
tempStatusMQ.get(),
- mEfGroup);
- status = mWriteThread->run("writer", PRIORITY_URGENT_AUDIO);
+ tempElfGroup.get());
+ if (!tempWriteThread->init()) {
+ _hidl_cb(Result::INVALID_ARGUMENTS,
+ CommandMQ::Descriptor(), DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo);
+ return Void();
+ }
+ status = tempWriteThread->run("writer", PRIORITY_URGENT_AUDIO);
if (status != OK) {
ALOGW("failed to start writer thread: %s", strerror(-status));
_hidl_cb(Result::INVALID_ARGUMENTS,
@@ -329,6 +350,8 @@
mCommandMQ = std::move(tempCommandMQ);
mDataMQ = std::move(tempDataMQ);
mStatusMQ = std::move(tempStatusMQ);
+ mWriteThread = tempWriteThread.release();
+ mEfGroup = tempElfGroup.release();
threadInfo.pid = getpid();
threadInfo.tid = mWriteThread->getTid();
_hidl_cb(Result::OK,
diff --git a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
index 1021569..4b00f35 100644
--- a/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
+++ b/audio/2.0/vts/functional/AudioPrimaryHidlHalTest.cpp
@@ -171,6 +171,7 @@
sp<IDevice> baseDevice;
ASSERT_OK(devicesFactory->openDevice(IDevicesFactory::Device::PRIMARY,
returnIn(result, baseDevice)));
+ ASSERT_OK(result);
ASSERT_TRUE(baseDevice != nullptr);
environment->registerTearDown([]{ device.clear(); });
@@ -990,10 +991,12 @@
};
TEST_P(OutputStreamTest, SupportsPauseAndResumeAndDrain) {
+ doc::test("Implementation must expose pause, resume and drain capabilities");
Capability(stream.get());
}
TEST_P(OutputStreamTest, GetRenderPosition) {
+ doc::test("The render position should be 0 on a not started");
uint32_t dspFrames;
ASSERT_OK(stream->getRenderPosition(returnIn(res, dspFrames)));
if (res == Result::NOT_SUPPORTED) {
@@ -1005,6 +1008,7 @@
}
TEST_P(OutputStreamTest, GetNextWriteTimestamp) {
+ doc::test("The render position of a stream just created should be 0");
uint64_t timestampUs;
ASSERT_OK(stream->getNextWriteTimestamp(returnIn(res, timestampUs)));
if (res == Result::NOT_SUPPORTED) {
@@ -1030,6 +1034,7 @@
}
TEST_P(OutputStreamTest, SetCallback) {
+ doc::test("If supported, registering callback for async operation should never fail");
if (!isAsyncModeSupported(stream.get())) {
doc::partialTest("The stream does not support async operations");
return;
@@ -1039,6 +1044,7 @@
}
TEST_P(OutputStreamTest, clearCallback) {
+ doc::test("If supported, clearing a callback to go back to sync operation should not fail");
if (!isAsyncModeSupported(stream.get())) {
doc::partialTest("The stream does not support async operations");
return;
@@ -1049,6 +1055,7 @@
}
TEST_P(OutputStreamTest, Resume) {
+ doc::test("If supported, a stream should fail to resume if not previously paused");
if (!Capability(stream.get()).resume) {
doc::partialTest("The output stream does not support resume");
return;
@@ -1057,6 +1064,7 @@
}
TEST_P(OutputStreamTest, Pause) {
+ doc::test("If supported, a stream should fail to pause if not previously started");
if (!Capability(stream.get()).pause) {
doc::partialTest("The output stream does not support pause");
return;
@@ -1066,21 +1074,24 @@
static void testDrain(IStreamOut *stream, AudioDrain type) {
if (!Capability(stream).drain) {
- doc::partialTest("The output stream does not support pause");
+ doc::partialTest("The output stream does not support drain");
return;
}
ASSERT_RESULT(Result::OK, stream->drain(type));
}
TEST_P(OutputStreamTest, DrainAll) {
+ doc::test("If supported, a stream should always succeed to drain");
testDrain(stream.get(), AudioDrain::ALL);
}
TEST_P(OutputStreamTest, DrainEarlyNotify) {
+ doc::test("If supported, a stream should always succeed to drain");
testDrain(stream.get(), AudioDrain::EARLY_NOTIFY);
}
TEST_P(OutputStreamTest, FlushStop) {
+ doc::test("If supported, a stream should always succeed to flush");
auto ret = stream->flush();
ASSERT_TRUE(ret.isOk());
if (ret == Result::NOT_SUPPORTED) {
@@ -1091,6 +1102,7 @@
}
TEST_P(OutputStreamTest, GetPresentationPositionStop) {
+ doc::test("If supported, a stream should always succeed to retrieve the presentation position");
uint64_t frames;
TimeSpec mesureTS;
ASSERT_OK(stream->getPresentationPosition(returnIn(res, frames, mesureTS)));