From c07df49e455ef67f880a3ca29ce585a213bccdde Mon Sep 17 00:00:00 2001 From: Kevin Rocard Date: Tue, 2 May 2017 18:31:24 -0700 Subject: [PATCH] Audio HAL VTS: refactor prepareFor{Reading,Writing} Those functions had lots of copy paste on errors and the following patch will even add more error detections. Refactor the hidl_cb call to avoid all duplication. Note that both function should be refactored as 99% identical. Test: vts-tradefed run vts --module VtsHalAudioV2_0Target Test: call/play music/record/video... Bug: 36311550 Change-Id: I40d6926b4f9f5e3aba51e878f55fb013f4ca09c1 Signed-off-by: Kevin Rocard --- audio/2.0/default/StreamIn.cpp | 27 +++++++++++++++------------ audio/2.0/default/StreamOut.cpp | 24 ++++++++++++++---------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/audio/2.0/default/StreamIn.cpp b/audio/2.0/default/StreamIn.cpp index 2262697cde..59029be522 100644 --- a/audio/2.0/default/StreamIn.cpp +++ b/audio/2.0/default/StreamIn.cpp @@ -319,19 +319,25 @@ Return StreamIn::prepareForReading(uint32_t frameSize, prepareForReading_cb _hidl_cb) { status_t status; ThreadInfo threadInfo = {0, 0}; + + // Wrap the _hidl_cb to return an error + auto sendError = [this, &threadInfo, &_hidl_cb](Result result) { + _hidl_cb(result, CommandMQ::Descriptor(), DataMQ::Descriptor(), + StatusMQ::Descriptor(), threadInfo); + + }; + // Create message queues. if (mDataMQ) { ALOGE("the client attempts to call prepareForReading twice"); - _hidl_cb(Result::INVALID_STATE, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_STATE); return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); if (frameSize > std::numeric_limits::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); + sendError(Result::INVALID_ARGUMENTS); return Void(); } std::unique_ptr tempDataMQ( @@ -343,8 +349,7 @@ Return StreamIn::prepareForReading(uint32_t frameSize, ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid"); ALOGE_IF(!tempDataMQ->isValid(), "data MQ is invalid"); ALOGE_IF(!tempStatusMQ->isValid(), "status MQ is invalid"); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } EventFlag* tempRawEfGroup{}; @@ -354,8 +359,7 @@ Return StreamIn::prepareForReading(uint32_t frameSize, 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); + sendError(Result::INVALID_ARGUMENTS); return Void(); } @@ -364,15 +368,14 @@ Return StreamIn::prepareForReading(uint32_t frameSize, &mStopReadThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); if (!tempReadThread->init()) { - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + ALOGW("failed to start reader thread: %s", strerror(-status)); + sendError(Result::INVALID_ARGUMENTS); return Void(); } status = tempReadThread->run("reader", PRIORITY_URGENT_AUDIO); if (status != OK) { ALOGW("failed to start reader thread: %s", strerror(-status)); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } diff --git a/audio/2.0/default/StreamOut.cpp b/audio/2.0/default/StreamOut.cpp index 63b9ae352c..fc055be546 100644 --- a/audio/2.0/default/StreamOut.cpp +++ b/audio/2.0/default/StreamOut.cpp @@ -295,11 +295,18 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, prepareForWriting_cb _hidl_cb) { status_t status; ThreadInfo threadInfo = {0, 0}; + + // Wrap the _hidl_cb to return an error + auto sendError = [this, &threadInfo, &_hidl_cb](Result result) { + _hidl_cb(result, CommandMQ::Descriptor(), DataMQ::Descriptor(), + StatusMQ::Descriptor(), threadInfo); + + }; + // Create message queues. if (mDataMQ) { ALOGE("the client attempts to call prepareForWriting twice"); - _hidl_cb(Result::INVALID_STATE, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_STATE); return Void(); } std::unique_ptr tempCommandMQ(new CommandMQ(1)); @@ -320,8 +327,7 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, ALOGE_IF(!tempCommandMQ->isValid(), "command MQ is invalid"); ALOGE_IF(!tempDataMQ->isValid(), "data MQ is invalid"); ALOGE_IF(!tempStatusMQ->isValid(), "status MQ is invalid"); - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); } EventFlag* tempRawEfGroup{}; @@ -331,8 +337,7 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, 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); + sendError(Result::INVALID_ARGUMENTS); return Void(); } @@ -341,15 +346,14 @@ Return StreamOut::prepareForWriting(uint32_t frameSize, &mStopWriteThread, mStream, tempCommandMQ.get(), tempDataMQ.get(), tempStatusMQ.get(), tempElfGroup.get()); if (!tempWriteThread->init()) { - _hidl_cb(Result::INVALID_ARGUMENTS, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + ALOGW("failed to start writer thread: %s", strerror(-status)); + sendError(Result::INVALID_ARGUMENTS); 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, CommandMQ::Descriptor(), - DataMQ::Descriptor(), StatusMQ::Descriptor(), threadInfo); + sendError(Result::INVALID_ARGUMENTS); return Void(); }