Merge "Fix few issues in virtual camera service:" into main
diff --git a/media/audio/aconfig/audio_framework.aconfig b/media/audio/aconfig/audio_framework.aconfig
index c6d0036..294e67d 100644
--- a/media/audio/aconfig/audio_framework.aconfig
+++ b/media/audio/aconfig/audio_framework.aconfig
@@ -12,6 +12,15 @@
bug: "302751899"
}
+flag {
+ name: "automatic_bt_device_type"
+ namespace: "media_audio"
+ description:
+ "Enable the automatic Bluetooth audio device type "
+ "categorization based on BluetoothDevice class metadata."
+ bug: "302323921"
+}
+
# TODO remove
flag {
name: "focus_freeze_test_api"
diff --git a/media/codec2/hal/client/client.cpp b/media/codec2/hal/client/client.cpp
index 597852a..e8e7cb2 100644
--- a/media/codec2/hal/client/client.cpp
+++ b/media/codec2/hal/client/client.cpp
@@ -2372,7 +2372,7 @@
void Codec2Client::Component::onBufferReleasedFromOutputSurface(
uint32_t generation) {
- (void) generation;
+ mOutputBufferQueue->onBufferReleased(generation);
}
c2_status_t Codec2Client::Component::connectToInputSurface(
diff --git a/media/codec2/hal/client/include/codec2/hidl/output.h b/media/codec2/hal/client/include/codec2/hidl/output.h
index 2e89c3b..fda34a8 100644
--- a/media/codec2/hal/client/include/codec2/hidl/output.h
+++ b/media/codec2/hal/client/include/codec2/hidl/output.h
@@ -65,6 +65,10 @@
const BnGraphicBufferProducer::QueueBufferInput& input,
BnGraphicBufferProducer::QueueBufferOutput* output);
+ // Nofify a buffer is released from the output surface. If HAL ver is 1.2
+ // update the number of dequeueable/allocatable buffers.
+ void onBufferReleased(uint32_t generation);
+
// Retrieve frame event history from the output surface.
void pollForRenderedFrames(FrameEventHistoryDelta* delta);
diff --git a/media/codec2/hal/client/output.cpp b/media/codec2/hal/client/output.cpp
index 7f4f86b..36322f5 100644
--- a/media/codec2/hal/client/output.cpp
+++ b/media/codec2/hal/client/output.cpp
@@ -441,9 +441,11 @@
status = outputIgbp->queueBuffer(static_cast<int>(bqSlot),
input, output);
if (status == OK) {
- syncVar->lock();
- syncVar->notifyQueuedLocked();
- syncVar->unlock();
+ if (output->bufferReplaced) {
+ syncVar->lock();
+ syncVar->notifyQueuedLocked();
+ syncVar->unlock();
+ }
}
} else {
status = outputIgbp->queueBuffer(static_cast<int>(bqSlot),
@@ -496,9 +498,11 @@
status = outputIgbp->queueBuffer(static_cast<int>(bqSlot),
input, output);
if (status == OK) {
- syncVar->lock();
- syncVar->notifyQueuedLocked();
- syncVar->unlock();
+ if (output->bufferReplaced) {
+ syncVar->lock();
+ syncVar->notifyQueuedLocked();
+ syncVar->unlock();
+ }
}
} else {
status = outputIgbp->queueBuffer(static_cast<int>(bqSlot),
@@ -514,6 +518,30 @@
return OK;
}
+void OutputBufferQueue::onBufferReleased(uint32_t generation) {
+ std::shared_ptr<C2SurfaceSyncMemory> syncMem;
+ sp<IGraphicBufferProducer> outputIgbp;
+ uint32_t outputGeneration = 0;
+ {
+ std::unique_lock<std::mutex> l(mMutex);
+ if (mStopped) {
+ return;
+ }
+ outputIgbp = mIgbp;
+ outputGeneration = mGeneration;
+ syncMem = mSyncMem;
+ }
+
+ if (outputIgbp && generation == outputGeneration) {
+ auto syncVar = syncMem ? syncMem->mem() : nullptr;
+ if (syncVar) {
+ syncVar->lock();
+ syncVar->notifyQueuedLocked();
+ syncVar->unlock();
+ }
+ }
+}
+
void OutputBufferQueue::pollForRenderedFrames(FrameEventHistoryDelta* delta) {
if (mIgbp) {
mIgbp->getFrameTimestamps(delta);
diff --git a/media/codec2/sfplugin/CCodecBufferChannel.cpp b/media/codec2/sfplugin/CCodecBufferChannel.cpp
index af15736..6b45e0e 100644
--- a/media/codec2/sfplugin/CCodecBufferChannel.cpp
+++ b/media/codec2/sfplugin/CCodecBufferChannel.cpp
@@ -1133,7 +1133,14 @@
}
void CCodecBufferChannel::onBufferReleasedFromOutputSurface(uint32_t generation) {
- mComponent->onBufferReleasedFromOutputSurface(generation);
+ // Note: Since this is called asynchronously from IProducerListener not
+ // knowing the internal state of CCodec/CCodecBufferChannel,
+ // prevent mComponent from being destroyed by holding the shared reference
+ // during this interface being executed.
+ std::shared_ptr<Codec2Client::Component> comp = mComponent;
+ if (comp) {
+ comp->onBufferReleasedFromOutputSurface(generation);
+ }
}
status_t CCodecBufferChannel::discardBuffer(const sp<MediaCodecBuffer> &buffer) {
diff --git a/media/codec2/vndk/platform/C2BqBuffer.cpp b/media/codec2/vndk/platform/C2BqBuffer.cpp
index 960fa79..62b0ab5 100644
--- a/media/codec2/vndk/platform/C2BqBuffer.cpp
+++ b/media/codec2/vndk/platform/C2BqBuffer.cpp
@@ -35,6 +35,7 @@
#include <C2FenceFactory.h>
#include <C2SurfaceSyncObj.h>
+#include <atomic>
#include <list>
#include <map>
#include <mutex>
@@ -753,8 +754,8 @@
}
void invalidate() {
- std::scoped_lock<std::mutex> lock(mMutex);
mInvalidated = true;
+ mIgbpValidityToken.reset();
}
private:
@@ -794,7 +795,7 @@
// if the token has been expired, the buffers will not call IGBP::cancelBuffer()
// when they are no longer used.
std::shared_ptr<int> mIgbpValidityToken;
- bool mInvalidated{false};
+ std::atomic<bool> mInvalidated{false};
};
C2BufferQueueBlockPoolData::C2BufferQueueBlockPoolData(
diff --git a/media/libaudioclient/tests/Android.bp b/media/libaudioclient/tests/Android.bp
index 5b90158..f72ac89 100644
--- a/media/libaudioclient/tests/Android.bp
+++ b/media/libaudioclient/tests/Android.bp
@@ -104,6 +104,7 @@
shared_libs: [
"capture_state_listener-aidl-cpp",
"framework-permission-aidl-cpp",
+ "libaudioutils",
"libbase",
"libcgrouprc",
"libdl",
@@ -130,7 +131,6 @@
"libaudiofoundation",
"libaudiomanager",
"libaudiopolicy",
- "libaudioutils",
],
data: ["bbb*.raw"],
test_config_template: "audio_test_template.xml",
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp
index 18d0d95..e4ec2ba 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp
@@ -169,8 +169,8 @@
const auto& measure = VALUE_OR_RETURN_STATUS(GET_PARAMETER_SPECIFIC_FIELD(
aidlParam, Visualizer, visualizer, Visualizer::measurement, Visualizer::Measurement));
int32_t* reply = (int32_t *) pReplyData;
- *reply++ = measure.rms;
- *reply = measure.peak;
+ *reply++ = measure.peak;
+ *reply = measure.rms;
return OK;
}
diff --git a/media/libeffects/visualizer/aidl/VisualizerContext.cpp b/media/libeffects/visualizer/aidl/VisualizerContext.cpp
index a1726ad..5d2bb3a 100644
--- a/media/libeffects/visualizer/aidl/VisualizerContext.cpp
+++ b/media/libeffects/visualizer/aidl/VisualizerContext.cpp
@@ -61,6 +61,7 @@
#endif
mChannelCount = channelCount;
mCommon = common;
+ std::fill(mCaptureBuf.begin(), mCaptureBuf.end(), 0x80);
return RetCode::SUCCESS;
}
@@ -84,7 +85,7 @@
void VisualizerContext::reset() {
std::lock_guard lg(mMutex);
- std::fill_n(mCaptureBuf.begin(), kMaxCaptureBufSize, 0x80);
+ std::fill(mCaptureBuf.begin(), mCaptureBuf.end(), 0x80);
}
RetCode VisualizerContext::setCaptureSamples(int samples) {
@@ -190,13 +191,12 @@
}
std::vector<uint8_t> VisualizerContext::capture() {
- std::vector<uint8_t> result;
std::lock_guard lg(mMutex);
+ uint32_t captureSamples = mCaptureSamples;
+ std::vector<uint8_t> result(captureSamples, 0x80);
// cts android.media.audio.cts.VisualizerTest expecting silence data when effect not running
// RETURN_VALUE_IF(mState != State::ACTIVE, result, "illegalState");
if (mState != State::ACTIVE) {
- result.resize(mCaptureSamples);
- memset(result.data(), 0x80, mCaptureSamples);
return result;
}
@@ -214,7 +214,7 @@
if (latencyMs < 0) {
latencyMs = 0;
}
- uint32_t deltaSamples = mCaptureSamples + mCommon.input.base.sampleRate * latencyMs / 1000;
+ uint32_t deltaSamples = captureSamples + mCommon.input.base.sampleRate * latencyMs / 1000;
// large sample rate, latency, or capture size, could cause overflow.
// do not offset more than the size of buffer.
@@ -223,22 +223,22 @@
deltaSamples = kMaxCaptureBufSize;
}
- int32_t capturePoint, captureSamples = mCaptureSamples;
+ int32_t capturePoint;
__builtin_sub_overflow((int32_t) mCaptureIdx, deltaSamples, &capturePoint);
// a negative capturePoint means we wrap the buffer.
if (capturePoint < 0) {
uint32_t size = -capturePoint;
- if (size > mCaptureSamples) {
- size = mCaptureSamples;
+ if (size > captureSamples) {
+ size = captureSamples;
}
- // first part of two stages copy, capture to the end of buffer and reset the size/point
- result.insert(result.end(), &mCaptureBuf[kMaxCaptureBufSize + capturePoint],
- &mCaptureBuf[kMaxCaptureBufSize + capturePoint + size]);
+ std::copy(std::begin(mCaptureBuf) + kMaxCaptureBufSize - size,
+ std::begin(mCaptureBuf) + kMaxCaptureBufSize, result.begin());
captureSamples -= size;
capturePoint = 0;
}
- result.insert(result.end(), &mCaptureBuf[capturePoint],
- &mCaptureBuf[capturePoint + captureSamples]);
+ std::copy(std::begin(mCaptureBuf) + capturePoint,
+ std::begin(mCaptureBuf) + capturePoint + captureSamples,
+ result.begin() + mCaptureSamples - captureSamples);
mLastCaptureIdx = mCaptureIdx;
return result;
}
@@ -256,16 +256,15 @@
// find the peak and RMS squared for the new buffer
float rmsSqAcc = 0;
float maxSample = 0.f;
- for (size_t inIdx = 0; inIdx < (unsigned)samples; ++inIdx) {
+ for (size_t inIdx = 0; inIdx < (unsigned) samples; ++inIdx) {
maxSample = fmax(maxSample, fabs(in[inIdx]));
rmsSqAcc += in[inIdx] * in[inIdx];
}
maxSample *= 1 << 15; // scale to int16_t, with exactly 1 << 15 representing positive num.
rmsSqAcc *= 1 << 30; // scale to int16_t * 2
- mPastMeasurements[mMeasurementBufferIdx] = {
- .mPeakU16 = (uint16_t)maxSample,
- .mRmsSquared = rmsSqAcc / samples,
- .mIsValid = true };
+ mPastMeasurements[mMeasurementBufferIdx] = {.mIsValid = true,
+ .mPeakU16 = (uint16_t)maxSample,
+ .mRmsSquared = rmsSqAcc / samples};
if (++mMeasurementBufferIdx >= mMeasurementWindowSizeInBuffers) {
mMeasurementBufferIdx = 0;
}
diff --git a/media/utils/TimeCheck.cpp b/media/utils/TimeCheck.cpp
index a5378e6..ec68de7 100644
--- a/media/utils/TimeCheck.cpp
+++ b/media/utils/TimeCheck.cpp
@@ -287,6 +287,11 @@
.append(halPids).append("\n")
.append(snapshotAnalysis.toString());
+ // In many cases, the initial timeout stack differs from the abort backtrace because
+ // (1) the time difference between initial timeout and the final abort signal
+ // and (2) signalling the HAL audio service may cause
+ // the thread to unblock and continue.
+
// Note: LOG_ALWAYS_FATAL limits the size of the string - per log/log.h:
// Log message text may be truncated to less than an
// implementation-specific limit (1023 bytes).
diff --git a/media/utils/TimerThread.cpp b/media/utils/TimerThread.cpp
index 3966103..ad27af3 100644
--- a/media/utils/TimerThread.cpp
+++ b/media/utils/TimerThread.cpp
@@ -21,6 +21,7 @@
#include <unistd.h>
#include <vector>
+#include <audio_utils/mutex.h>
#include <mediautils/MediaUtilsDelayed.h>
#include <mediautils/TidWrapper.h>
#include <mediautils/TimerThread.h>
@@ -59,14 +60,14 @@
return true;
}
-
-std::string TimerThread::SnapshotAnalysis::toString() const {
+std::string TimerThread::SnapshotAnalysis::toString(bool showTimeoutStack) const {
// Note: These request queues are snapshot very close together but
// not at "identical" times as we don't use a class-wide lock.
std::string analysisSummary = std::string("\nanalysis [ ").append(description).append(" ]");
std::string timeoutStack;
std::string blockedStack;
- if (timeoutTid != -1) {
+ std::string mutexWaitChainStack;
+ if (showTimeoutStack && timeoutTid != -1) {
timeoutStack = std::string(suspectTid == timeoutTid ? "\ntimeout/blocked(" : "\ntimeout(")
.append(std::to_string(timeoutTid)).append(") callstack [\n")
.append(getCallStackStringForTid(timeoutTid)).append("]");
@@ -78,6 +79,29 @@
.append(getCallStackStringForTid(suspectTid)).append("]");
}
+ if (!mutexWaitChain.empty()) {
+ mutexWaitChainStack.append("\nmutex wait chain [\n");
+ // the wait chain omits the initial timeout tid (which is good as we don't
+ // need to suppress it).
+ for (size_t i = 0; i < mutexWaitChain.size(); ++i) {
+ const auto& [tid, name] = mutexWaitChain[i];
+ mutexWaitChainStack.append("{ tid: ").append(std::to_string(tid))
+ .append(" (holding ").append(name).append(")");
+ if (tid == timeoutTid) {
+ mutexWaitChainStack.append(" TIMEOUT_STACK }\n");
+ } else if (tid == suspectTid) {
+ mutexWaitChainStack.append(" BLOCKED_STACK }\n");
+ } else if (hasMutexCycle && i == mutexWaitChain.size() - 1) {
+ // for a cycle, the last pid in the chain is repeated.
+ mutexWaitChainStack.append(" CYCLE_STACK }\n");
+ } else {
+ mutexWaitChainStack.append(" callstack [\n")
+ .append(getCallStackStringForTid(tid)).append("] }\n");
+ }
+ }
+ mutexWaitChainStack.append("]");
+ }
+
return std::string("now ")
.append(formatTime(std::chrono::system_clock::now()))
.append("\nsecondChanceCount ")
@@ -91,7 +115,8 @@
.append(requestsToString(retiredRequests))
.append(" ]")
.append(timeoutStack)
- .append(blockedStack);
+ .append(blockedStack)
+ .append(mutexWaitChainStack);
}
// A HAL method is where the substring "Hidl" is in the class name.
@@ -121,13 +146,33 @@
analysis.pendingRequests = getPendingRequests();
analysis.secondChanceCount = mMonitorThread.getSecondChanceCount();
// No call has timed out, so there is no analysis to be done.
- if (analysis.timeoutRequests.empty())
+ if (analysis.timeoutRequests.empty()) {
return analysis;
+ }
+
// for now look at last timeout (in our case, the only timeout)
const std::shared_ptr<const Request> timeout = analysis.timeoutRequests.back();
analysis.timeoutTid = timeout->tid;
- if (analysis.pendingRequests.empty())
- return analysis;
+
+ std::string& description = analysis.description;
+
+ // audio mutex specific wait chain analysis
+ auto deadlockInfo = audio_utils::mutex::deadlock_detection(analysis.timeoutTid);
+ ALOGD("%s: deadlockInfo: %s", __func__, deadlockInfo.to_string().c_str());
+
+ if (!deadlockInfo.empty()) {
+ if (!description.empty()) description.append("\n");
+ description.append(deadlockInfo.to_string());
+ }
+
+ analysis.hasMutexCycle = deadlockInfo.has_cycle;
+ analysis.mutexWaitChain = std::move(deadlockInfo.chain);
+
+ // no pending requests, we don't attempt to use temporal correlation between a recent call.
+ if (analysis.pendingRequests.empty()) {
+ return analysis;
+ }
+
// pending Requests that are problematic.
std::vector<std::shared_ptr<const Request>> pendingExact;
std::vector<std::shared_ptr<const Request>> pendingPossible;
@@ -151,15 +196,15 @@
}
}
- std::string& description = analysis.description;
if (!pendingExact.empty()) {
const auto& request = pendingExact.front();
const bool hal = isRequestFromHal(request);
if (hal) {
- description = std::string("Blocked directly due to HAL call: ")
+ if (!description.empty()) description.append("\n");
+ description.append("Blocked directly due to HAL call: ")
.append(request->toString());
- analysis.suspectTid= request->tid;
+ analysis.suspectTid = request->tid;
}
}
if (description.empty() && !pendingPossible.empty()) {
diff --git a/media/utils/include/mediautils/TimerThread.h b/media/utils/include/mediautils/TimerThread.h
index d84d682..320567b 100644
--- a/media/utils/include/mediautils/TimerThread.h
+++ b/media/utils/include/mediautils/TimerThread.h
@@ -269,9 +269,12 @@
std::vector<std::shared_ptr<const Request>> timeoutRequests;
// List of retired requests
std::vector<std::shared_ptr<const Request>> retiredRequests;
+ // mutex deadlock / wait detection information.
+ bool hasMutexCycle = false;
+ std::vector<std::pair<pid_t, std::string>> mutexWaitChain;
// Dumps the information contained above as well as additional call
// stacks where applicable.
- std::string toString() const;
+ std::string toString(bool showTimeoutStack = true) const;
};
private:
diff --git a/services/camera/virtualcamera/Android.bp b/services/camera/virtualcamera/Android.bp
index 0375a7c..ff7b037 100644
--- a/services/camera/virtualcamera/Android.bp
+++ b/services/camera/virtualcamera/Android.bp
@@ -91,9 +91,5 @@
"libvirtualcamera",
"libvirtualcamera_utils",
],
- // Remove before flight.
- // We don't want the service to be started and discovered
- // yet - remove comments on the lines below in order to
- // test locally.
- // init_rc: ["virtual_camera.hal.rc"],
+ init_rc: ["virtual_camera.hal.rc"],
}
diff --git a/services/camera/virtualcamera/TEST_MAPPING b/services/camera/virtualcamera/TEST_MAPPING
index 04b4025..10ba61a 100644
--- a/services/camera/virtualcamera/TEST_MAPPING
+++ b/services/camera/virtualcamera/TEST_MAPPING
@@ -2,6 +2,14 @@
"postsubmit": [
{
"name": "virtual_camera_tests"
+ },
+ {
+ "name": "CtsVirtualDevicesCameraTestCases",
+ "options": [
+ {
+ "exclude-annotation": "androidx.test.filters.FlakyTest"
+ }
+ ]
}
]
}
diff --git a/services/camera/virtualcamera/VirtualCameraDevice.h b/services/camera/virtualcamera/VirtualCameraDevice.h
index 4c3cfc2..ee7a3a7 100644
--- a/services/camera/virtualcamera/VirtualCameraDevice.h
+++ b/services/camera/virtualcamera/VirtualCameraDevice.h
@@ -95,6 +95,8 @@
// "device@{major}.{minor}/virtual/{numerical_id}"
std::string getCameraName() const;
+ uint32_t getCameraId() const { return mCameraId; }
+
private:
const uint32_t mCameraId;
const std::shared_ptr<
diff --git a/services/camera/virtualcamera/VirtualCameraService.cc b/services/camera/virtualcamera/VirtualCameraService.cc
index 8afd901..a2054da 100644
--- a/services/camera/virtualcamera/VirtualCameraService.cc
+++ b/services/camera/virtualcamera/VirtualCameraService.cc
@@ -143,6 +143,27 @@
return ndk::ScopedAStatus::ok();
}
+ndk::ScopedAStatus VirtualCameraService::getCameraId(
+ const ::ndk::SpAIBinder& token, int32_t* _aidl_return) {
+ if (_aidl_return == nullptr) {
+ return ndk::ScopedAStatus::fromServiceSpecificError(
+ Status::EX_ILLEGAL_ARGUMENT);
+ }
+
+ auto camera = getCamera(token);
+ if (camera == nullptr) {
+ ALOGE(
+ "Attempt to get camera id corresponding to unknown binder token: "
+ "0x%" PRIxPTR,
+ reinterpret_cast<uintptr_t>(token.get()));
+ return ndk::ScopedAStatus::ok();
+ }
+
+ *_aidl_return = camera->getCameraId();
+
+ return ndk::ScopedAStatus::ok();
+}
+
std::shared_ptr<VirtualCameraDevice> VirtualCameraService::getCamera(
const ::ndk::SpAIBinder& token) {
if (token == nullptr) {
diff --git a/services/camera/virtualcamera/VirtualCameraService.h b/services/camera/virtualcamera/VirtualCameraService.h
index cf68c1b..b68d43a 100644
--- a/services/camera/virtualcamera/VirtualCameraService.h
+++ b/services/camera/virtualcamera/VirtualCameraService.h
@@ -47,6 +47,10 @@
ndk::ScopedAStatus unregisterCamera(const ::ndk::SpAIBinder& token) override
EXCLUDES(mLock);
+ // Returns the camera id corresponding to the binder token.
+ ndk::ScopedAStatus getCameraId(
+ const ::ndk::SpAIBinder& token, int32_t* _aidl_return) override EXCLUDES(mLock);
+
// Returns VirtualCameraDevice corresponding to binder token or nullptr if
// there's no camera asociated with the token.
std::shared_ptr<VirtualCameraDevice> getCamera(const ::ndk::SpAIBinder& token)
diff --git a/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl b/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl
index af8324a..bb74f5c 100644
--- a/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl
+++ b/services/camera/virtualcamera/aidl/android/companion/virtualcamera/IVirtualCameraService.aidl
@@ -35,4 +35,10 @@
* be visible to the camera framework anymore.
*/
void unregisterCamera(in IBinder token);
+
+ /**
+ * Returns the camera id for a given binder token. Note that this id corresponds to the id of
+ * the camera device in the camera framework.
+ */
+ int getCameraId(in IBinder token);
}