Merge "CCodec: increase timeout for operations" into qt-dev
diff --git a/camera/cameraserver/Android.bp b/camera/cameraserver/Android.bp
index 92b06c2..ecaba3a 100644
--- a/camera/cameraserver/Android.bp
+++ b/camera/cameraserver/Android.bp
@@ -24,6 +24,7 @@
"libui",
"libgui",
"libbinder",
+ "libhidlbase",
"libhidltransport",
"android.hardware.camera.common@1.0",
"android.hardware.camera.provider@2.4",
diff --git a/media/audioserver/Android.mk b/media/audioserver/Android.mk
index 1f24413..33b36b8 100644
--- a/media/audioserver/Android.mk
+++ b/media/audioserver/Android.mk
@@ -12,6 +12,7 @@
libbinder \
libcutils \
liblog \
+ libhidlbase \
libhidltransport \
libhwbinder \
libmedia \
diff --git a/media/codec2/sfplugin/C2OMXNode.cpp b/media/codec2/sfplugin/C2OMXNode.cpp
index f0f62f6..3a93c2a 100644
--- a/media/codec2/sfplugin/C2OMXNode.cpp
+++ b/media/codec2/sfplugin/C2OMXNode.cpp
@@ -31,6 +31,7 @@
#include <OMX_Index.h>
#include <OMX_IndexExt.h>
+#include <android/fdsan.h>
#include <media/stagefright/omx/OMXUtils.h>
#include <media/stagefright/MediaErrors.h>
#include <ui/Fence.h>
@@ -52,10 +53,12 @@
C2OMXNode::C2OMXNode(const std::shared_ptr<Codec2Client::Component> &comp)
: mComp(comp), mFrameIndex(0), mWidth(0), mHeight(0), mUsage(0),
mAdjustTimestampGapUs(0), mFirstInputFrame(true) {
+ android_fdsan_set_error_level(ANDROID_FDSAN_ERROR_LEVEL_WARN_ALWAYS);
}
status_t C2OMXNode::freeNode() {
mComp.reset();
+ android_fdsan_set_error_level(ANDROID_FDSAN_ERROR_LEVEL_WARN_ONCE);
return OK;
}
@@ -227,6 +230,7 @@
? C2FrameData::FLAG_END_OF_STREAM : 0;
std::shared_ptr<C2GraphicBlock> block;
+ android::base::unique_fd fd0, fd1;
C2Handle *handle = nullptr;
if (omxBuf.mBufferType == OMXBuffer::kBufferTypeANWBuffer
&& omxBuf.mGraphicBuffer != nullptr) {
@@ -238,8 +242,19 @@
omxBuf.mGraphicBuffer->format,
omxBuf.mGraphicBuffer->usage,
omxBuf.mGraphicBuffer->stride);
+ if (handle != nullptr) {
+ // unique_fd takes ownership of the fds, we'll get warning if these
+ // fds get closed by somebody else. Onwership will be released before
+ // we return, so that the fds get closed as usually when this function
+ // goes out of scope (when both items and block are gone).
+ native_handle_t *nativeHandle = reinterpret_cast<native_handle_t*>(handle);
+ fd0.reset(nativeHandle->numFds > 0 ? nativeHandle->data[0] : -1);
+ fd1.reset(nativeHandle->numFds > 1 ? nativeHandle->data[1] : -1);
+ }
c2_status_t err = mAllocator->priorGraphicAllocation(handle, &alloc);
if (err != OK) {
+ (void)fd0.release();
+ (void)fd1.release();
return UNKNOWN_ERROR;
}
block = _C2BlockFactory::CreateGraphicBlock(alloc);
@@ -290,10 +305,17 @@
c2_status_t err = comp->queue(&items);
if (err != C2_OK) {
+ (void)fd0.release();
+ (void)fd1.release();
return UNKNOWN_ERROR;
}
mBufferIdsInUse.lock()->emplace(index, buffer);
+
+ // release ownership of the fds
+ (void)fd0.release();
+ (void)fd1.release();
+
return OK;
}
diff --git a/media/libaaudio/src/client/AudioStreamInternal.cpp b/media/libaaudio/src/client/AudioStreamInternal.cpp
index c7e8088..52eadd4 100644
--- a/media/libaaudio/src/client/AudioStreamInternal.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternal.cpp
@@ -241,22 +241,18 @@
return result;
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStreamInternal::close() {
aaudio_result_t result = AAUDIO_OK;
ALOGV("%s(): mServiceStreamHandle = 0x%08X", __func__, mServiceStreamHandle);
if (mServiceStreamHandle != AAUDIO_HANDLE_INVALID) {
// Don't close a stream while it is running.
aaudio_stream_state_t currentState = getState();
- if (isActive()) {
+ // Don't close a stream while it is running. Stop it first.
+ // If DISCONNECTED then we should still try to stop in case the
+ // error callback is still running.
+ if (isActive() || currentState == AAUDIO_STREAM_STATE_DISCONNECTED) {
requestStop();
- aaudio_stream_state_t nextState;
- int64_t timeoutNanoseconds = MIN_TIMEOUT_NANOS;
- result = waitForStateChange(currentState, &nextState,
- timeoutNanoseconds);
- if (result != AAUDIO_OK) {
- ALOGW("%s() waitForStateChange() returned %d %s",
- __func__, result, AAudio_convertResultToText(result));
- }
}
setState(AAUDIO_STREAM_STATE_CLOSING);
aaudio_handle_t serviceStreamHandle = mServiceStreamHandle;
@@ -357,21 +353,31 @@
return calculateReasonableTimeout(getFramesPerBurst());
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStreamInternal::stopCallback()
{
- if (isDataCallbackActive()) {
+ if (isDataCallbackSet()
+ && (isActive() || getState() == AAUDIO_STREAM_STATE_DISCONNECTED)) {
mCallbackEnabled.store(false);
- return joinThread(NULL);
+ return joinThread(NULL); // may temporarily unlock mStreamLock
} else {
return AAUDIO_OK;
}
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStreamInternal::requestStop() {
aaudio_result_t result = stopCallback();
if (result != AAUDIO_OK) {
return result;
}
+ // The stream may have been unlocked temporarily to let a callback finish
+ // and the callback may have stopped the stream.
+ // Check to make sure the stream still needs to be stopped.
+ // See also AudioStream::safeStop().
+ if (!(isActive() || getState() == AAUDIO_STREAM_STATE_DISCONNECTED)) {
+ return AAUDIO_OK;
+ }
if (mServiceStreamHandle == AAUDIO_HANDLE_INVALID) {
ALOGW("%s() mServiceStreamHandle invalid = 0x%08X",
@@ -728,6 +734,7 @@
return mFramesPerBurst;
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStreamInternal::joinThread(void** returnArg) {
return AudioStream::joinThread(returnArg, calculateReasonableTimeout(getFramesPerBurst()));
}
diff --git a/media/libaaudio/src/client/AudioStreamInternalPlay.cpp b/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
index 164ad2b..b8ef247 100644
--- a/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
@@ -58,6 +58,7 @@
return result;
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStreamInternalPlay::requestPause()
{
aaudio_result_t result = stopCallback();
diff --git a/media/libaaudio/src/core/AudioStream.cpp b/media/libaaudio/src/core/AudioStream.cpp
index 25669be..9b77223 100644
--- a/media/libaaudio/src/core/AudioStream.cpp
+++ b/media/libaaudio/src/core/AudioStream.cpp
@@ -211,6 +211,7 @@
return result;
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStream::safeStop() {
switch (getState()) {
@@ -247,6 +248,7 @@
}
aaudio_result_t AudioStream::safeClose() {
+ // This get temporarily unlocked in the close when joining callback threads.
std::lock_guard<std::mutex> lock(mStreamLock);
if (collidesWithCallback()) {
ALOGE("%s cannot be called from a callback!", __func__);
@@ -363,6 +365,7 @@
}
}
+// This must be called under mStreamLock.
aaudio_result_t AudioStream::joinThread(void** returnArg, int64_t timeoutNanoseconds __unused)
{
if (!mHasThread) {
@@ -374,6 +377,8 @@
// then we don't need to join(). The thread is already about to exit.
if (pthread_self() != mThread) {
// Called from an app thread. Not the callback.
+ // Unlock because the callback may be trying to stop the stream but is blocked.
+ mStreamLock.unlock();
#if 0
// TODO implement equivalent of pthread_timedjoin_np()
struct timespec abstime;
@@ -381,6 +386,7 @@
#else
int err = pthread_join(mThread, returnArg);
#endif
+ mStreamLock.lock();
if (err) {
ALOGE("%s() pthread_join() returns err = %d", __func__, err);
result = AAudioConvert_androidToAAudioResult(-err);
diff --git a/media/libaaudio/tests/Android.bp b/media/libaaudio/tests/Android.bp
index 958bb2e..6101e99 100644
--- a/media/libaaudio/tests/Android.bp
+++ b/media/libaaudio/tests/Android.bp
@@ -198,6 +198,18 @@
}
cc_test {
+ name: "test_stop_hang",
+ defaults: ["libaaudio_tests_defaults"],
+ srcs: ["test_stop_hang.cpp"],
+ shared_libs: [
+ "libaaudio",
+ "libbinder",
+ "libcutils",
+ "libutils",
+ ],
+}
+
+cc_test {
name: "test_full_queue",
defaults: ["libaaudio_tests_defaults"],
srcs: ["test_full_queue.cpp"],
diff --git a/media/libaaudio/tests/test_stop_hang.cpp b/media/libaaudio/tests/test_stop_hang.cpp
new file mode 100644
index 0000000..2397b6c
--- /dev/null
+++ b/media/libaaudio/tests/test_stop_hang.cpp
@@ -0,0 +1,159 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * Return stop from the callback
+ * and then close the stream immediately.
+ */
+
+#include <atomic>
+#include <mutex>
+#include <stdio.h>
+#include <thread>
+#include <unistd.h>
+
+#include <aaudio/AAudio.h>
+
+#define DURATION_SECONDS 5
+
+struct AudioEngine {
+ AAudioStreamBuilder *builder = nullptr;
+ AAudioStream *stream = nullptr;
+ std::thread *thread = nullptr;
+
+ std::atomic<bool> started{false};
+ std::mutex doneLock; // Use a mutex so we can sleep on it while join()ing.
+ std::atomic<bool> done{false};
+
+ aaudio_result_t join() {
+ aaudio_result_t result = AAUDIO_ERROR_INVALID_STATE;
+ if (stream != nullptr) {
+ while (true) {
+ {
+ // Will block if the thread is running.
+ // This mutex is used to close() immediately after the callback returns
+ // and before the requestStop() is called.
+ std::lock_guard<std::mutex> lock(doneLock);
+ if (done) break;
+ }
+ printf("join() got mutex but stream not done!");
+ usleep(10 * 1000); // sleep then check again
+ }
+ result = AAudioStream_close(stream);
+ stream = nullptr;
+ }
+ return result;
+ }
+};
+
+// Callback function that fills the audio output buffer.
+static aaudio_data_callback_result_t s_myDataCallbackProc(
+ AAudioStream *stream,
+ void *userData,
+ void *audioData,
+ int32_t numFrames
+) {
+ (void) stream;
+ (void) audioData;
+ (void) numFrames;
+ AudioEngine *engine = (struct AudioEngine *)userData;
+ std::lock_guard<std::mutex> lock(engine->doneLock);
+ engine->started = true;
+ usleep(DURATION_SECONDS * 1000 * 1000); // Mimic SynthMark procedure.
+ engine->done = true;
+ return AAUDIO_CALLBACK_RESULT_STOP;
+}
+
+static void s_myErrorCallbackProc(
+ AAudioStream *stream __unused,
+ void *userData __unused,
+ aaudio_result_t error) {
+ printf("%s() - error = %d\n", __func__, error);
+}
+
+static aaudio_result_t s_OpenAudioStream(struct AudioEngine *engine) {
+ // Use an AAudioStreamBuilder to contain requested parameters.
+ aaudio_result_t result = AAudio_createStreamBuilder(&engine->builder);
+ if (result != AAUDIO_OK) {
+ printf("AAudio_createStreamBuilder returned %s",
+ AAudio_convertResultToText(result));
+ return result;
+ }
+
+ // Request stream properties.
+ AAudioStreamBuilder_setPerformanceMode(engine->builder, AAUDIO_PERFORMANCE_MODE_LOW_LATENCY);
+ AAudioStreamBuilder_setDataCallback(engine->builder, s_myDataCallbackProc, engine);
+ AAudioStreamBuilder_setErrorCallback(engine->builder, s_myErrorCallbackProc, engine);
+
+ // Create an AAudioStream using the Builder.
+ result = AAudioStreamBuilder_openStream(engine->builder, &engine->stream);
+ if (result != AAUDIO_OK) {
+ printf("AAudioStreamBuilder_openStream returned %s",
+ AAudio_convertResultToText(result));
+ return result;
+ }
+
+ return result;
+}
+
+int main(int argc, char **argv) {
+ (void) argc;
+ (void) argv;
+ struct AudioEngine engine;
+ aaudio_result_t result = AAUDIO_OK;
+ int errorCount = 0;
+
+ // Make printf print immediately so that debug info is not stuck
+ // in a buffer if we hang or crash.
+ setvbuf(stdout, nullptr, _IONBF, (size_t) 0);
+
+ printf("Test Return Stop Hang V1.0\n");
+
+ result = s_OpenAudioStream(&engine);
+ if (result != AAUDIO_OK) {
+ printf("s_OpenAudioStream returned %s\n",
+ AAudio_convertResultToText(result));
+ errorCount++;
+ }
+
+ // Check to see what kind of stream we actually got.
+ int32_t deviceId = AAudioStream_getDeviceId(engine.stream);
+ aaudio_performance_mode_t actualPerfMode = AAudioStream_getPerformanceMode(engine.stream);
+ printf("-------- opened: deviceId = %3d, perfMode = %d\n", deviceId, actualPerfMode);
+
+ // Start stream.
+ result = AAudioStream_requestStart(engine.stream);
+ printf("AAudioStream_requestStart() returned %d >>>>>>>>>>>>>>>>>>>>>>\n", result);
+ if (result != AAUDIO_OK) {
+ errorCount++;
+ } else {
+ int counter = 0;
+ while (!engine.started) {
+ printf("Waiting for stream to start, %d\n", counter++);
+ usleep(5 * 1000);
+ }
+ printf("You should see more messages %d seconds after this. If not then the test failed!\n",
+ DURATION_SECONDS);
+ result = engine.join(); // This might hang!
+ AAudioStreamBuilder_delete(engine.builder);
+ engine.builder = nullptr;
+ }
+
+ printf("aaudio result = %d = %s\n", result, AAudio_convertResultToText(result));
+ printf("test %s\n", errorCount ? "FAILED" : "PASSED");
+
+ return errorCount ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/media/libeffects/visualizer/EffectVisualizer.cpp b/media/libeffects/visualizer/EffectVisualizer.cpp
index 00bc371..f838892 100644
--- a/media/libeffects/visualizer/EffectVisualizer.cpp
+++ b/media/libeffects/visualizer/EffectVisualizer.cpp
@@ -388,7 +388,7 @@
maxSample = fmax(maxSample, fabs(smp));
}
if (maxSample > 0.f) {
- fscale = 127.f / maxSample;
+ fscale = 0.99f / maxSample;
int exp; // unused
const float significand = frexp(fscale, &exp);
if (significand == 0.5f) {
diff --git a/media/libmedia/include/media/omx/1.0/Conversion.h b/media/libmedia/include/media/omx/1.0/Conversion.h
index 80e8f3a..6dc46b7 100644
--- a/media/libmedia/include/media/omx/1.0/Conversion.h
+++ b/media/libmedia/include/media/omx/1.0/Conversion.h
@@ -625,8 +625,18 @@
// convert: AnwBuffer -> GraphicBuffer
// Ref: frameworks/native/libs/ui/GraphicBuffer.cpp: GraphicBuffer::flatten
inline bool convertTo(GraphicBuffer* l, AnwBuffer const& t) {
- native_handle_t* handle = t.nativeHandle == nullptr ?
- nullptr : native_handle_clone(t.nativeHandle);
+ native_handle_t* handle = nullptr;
+
+ if (t.nativeHandle != nullptr) {
+ handle = native_handle_clone(t.nativeHandle);
+ if (handle == nullptr) {
+ ALOGE("Failed to clone handle: numFds=%d, data[0]=%d, data[1]=%d",
+ t.nativeHandle->numFds,
+ (t.nativeHandle->numFds > 0) ? t.nativeHandle->data[0] : -1,
+ (t.nativeHandle->numFds > 1) ? t.nativeHandle->data[1] : -1);
+ return false;
+ }
+ }
size_t const numInts = 12 + (handle ? handle->numInts : 0);
int32_t* ints = new int32_t[numInts];
@@ -756,7 +766,12 @@
return true;
}
AnwBuffer anwBuffer;
- anwBuffer.nativeHandle = t.nativeHandle;
+ // Explicitly get the native_handle_t* (in stead of assigning t.nativeHandle)
+ // so that we don't do an extra native_handle_clone() in this step, as the
+ // convertion to GraphicBuffer below will do a clone regardless.
+ // If we encounter an invalid handle, the convertTo() below would fail (while
+ // the assigning of hidl_handle would abort and cause a crash).
+ anwBuffer.nativeHandle = t.nativeHandle.getNativeHandle();
anwBuffer.attr = t.attr.anwBuffer;
sp<GraphicBuffer> graphicBuffer = new GraphicBuffer();
if (!convertTo(graphicBuffer.get(), anwBuffer)) {
diff --git a/media/libmediaplayerservice/nuplayer/GenericSource.cpp b/media/libmediaplayerservice/nuplayer/GenericSource.cpp
index 5c77e41..2cd920a 100644
--- a/media/libmediaplayerservice/nuplayer/GenericSource.cpp
+++ b/media/libmediaplayerservice/nuplayer/GenericSource.cpp
@@ -389,7 +389,6 @@
if (httpSource == NULL) {
ALOGE("Failed to create http source!");
notifyPreparedAndCleanup(UNKNOWN_ERROR);
- mDisconnectLock.lock();
return;
}
mDisconnectLock.lock();
@@ -405,8 +404,8 @@
sp<DataSource> dataSource = DataSourceFactory::CreateFromURI(
mHTTPService, uri, &mUriHeaders, &contentType,
static_cast<HTTPBase *>(mHttpSource.get()));
- mLock.lock();
mDisconnectLock.lock();
+ mLock.lock();
if (!mDisconnected) {
mDataSource = dataSource;
}
@@ -449,6 +448,7 @@
if (mDataSource == NULL) {
ALOGE("Failed to create data source!");
+ mDisconnectLock.unlock();
notifyPreparedAndCleanup(UNKNOWN_ERROR);
return;
}
diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp
index e56750b..f579e9d 100644
--- a/media/libstagefright/MediaCodec.cpp
+++ b/media/libstagefright/MediaCodec.cpp
@@ -1110,12 +1110,10 @@
reset();
}
if (!isResourceError(err)) {
- if (err == OK) {
- disableLegacyBufferDropPostQ(surface);
- }
break;
}
}
+
return err;
}
@@ -1178,11 +1176,7 @@
msg->setObject("surface", surface);
sp<AMessage> response;
- status_t result = PostAndAwaitResponse(msg, &response);
- if (result == OK) {
- disableLegacyBufferDropPostQ(surface);
- }
- return result;
+ return PostAndAwaitResponse(msg, &response);
}
status_t MediaCodec::createInputSurface(
@@ -2005,6 +1999,13 @@
CHECK(msg->findMessage("input-format", &mInputFormat));
CHECK(msg->findMessage("output-format", &mOutputFormat));
+
+ // limit to confirming the opt-in behavior to minimize any behavioral change
+ if (mSurface != nullptr && !mAllowFrameDroppingBySurface) {
+ // signal frame dropping mode in the input format as this may also be
+ // meaningful and confusing for an encoder in a transcoder scenario
+ mInputFormat->setInt32("allow-frame-drop", mAllowFrameDroppingBySurface);
+ }
ALOGV("[%s] configured as input format: %s, output format: %s",
mComponentName.c_str(),
mInputFormat->debugString(4).c_str(),
@@ -2437,6 +2438,11 @@
}
if (obj != NULL) {
+ if (!format->findInt32("allow-frame-drop", &mAllowFrameDroppingBySurface)) {
+ // allow frame dropping by surface by default
+ mAllowFrameDroppingBySurface = true;
+ }
+
format->setObject("native-window", obj);
status_t err = handleSetSurface(static_cast<Surface *>(obj.get()));
if (err != OK) {
@@ -2444,6 +2450,9 @@
break;
}
} else {
+ // we are not using surface so this variable is not used, but initialize sensibly anyway
+ mAllowFrameDroppingBySurface = false;
+
handleSetSurface(NULL);
}
@@ -3436,6 +3445,10 @@
if (err != OK) {
ALOGE("nativeWindowConnect returned an error: %s (%d)", strerror(-err), err);
+ } else {
+ if (!mAllowFrameDroppingBySurface) {
+ disableLegacyBufferDropPostQ(surface);
+ }
}
}
// do not return ALREADY_EXISTS unless surfaces are the same
diff --git a/media/libstagefright/include/media/stagefright/MediaCodec.h b/media/libstagefright/include/media/stagefright/MediaCodec.h
index 6cd4265..89cca63 100644
--- a/media/libstagefright/include/media/stagefright/MediaCodec.h
+++ b/media/libstagefright/include/media/stagefright/MediaCodec.h
@@ -340,6 +340,7 @@
int32_t mVideoWidth;
int32_t mVideoHeight;
int32_t mRotationDegrees;
+ int32_t mAllowFrameDroppingBySurface;
// initial create parameters
AString mInitName;
diff --git a/services/mediacodec/Android.bp b/services/mediacodec/Android.bp
index 32d2fde..99a6d6b 100644
--- a/services/mediacodec/Android.bp
+++ b/services/mediacodec/Android.bp
@@ -9,6 +9,7 @@
shared_libs: [
"libavservices_minijail",
"libbase",
+ "libhidlbase",
"libhidltransport",
"libhwbinder",
"liblog",
diff --git a/services/mediacodec/Android.mk b/services/mediacodec/Android.mk
index ecd437b..ecc8408 100644
--- a/services/mediacodec/Android.mk
+++ b/services/mediacodec/Android.mk
@@ -40,6 +40,7 @@
libavservices_minijail_vendor \
libcutils \
libhwbinder \
+ libhidlbase \
libhidltransport \
libstagefright_omx \
libstagefright_xmlparser \