SimpleDecodingSource:Prevent OOB write in heap mem am: f3590a1b18 am: b240660c2e am: 247199f370 am: e3f218d634 am: 2708f51a28 am: efc5d1676f
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/av/+/16187628
Change-Id: Ia7c5b994cd9c361514ec9395c7972d6f4c77b3af
diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index 200e92d..bd6db55 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -53,6 +53,83 @@
//EL_FIXME 20 seconds may not be enough and must be reconciled with new obtainBuffer implementation
#define MAX_RUN_OFFLOADED_TIMEOUT_MS 20000 // assuming up to a maximum of 20 seconds of offloaded
+// for audio_track_cblk_t::mState, to match TrackBase.h
+static inline constexpr int CBLK_STATE_IDLE = 0;
+static inline constexpr int CBLK_STATE_PAUSING = 7;
+
+/**
+ * MirroredVariable is a local variable which simultaneously updates
+ * a mirrored storage location. This is useful for server side variables
+ * where a local copy is kept, but a client visible copy is offered through shared memory.
+ *
+ * We use std::atomic as the default container class to access this memory.
+ */
+template <typename T, template <typename> class Container = std::atomic>
+class MirroredVariable {
+ template <typename C>
+ struct Constraints {
+ // If setMirror is used with a different type U != T passed in,
+ // as a general rule, the Container must issue a memcpy to read or write
+ // (or its equivalent) to avoid possible strict aliasing issues.
+ // The memcpy also avoids gaps in structs and alignment issues with different types.
+ static constexpr bool ok_ = false; // Containers must specify constraints.
+ };
+ template <typename X>
+ struct Constraints<std::atomic<X>> {
+ // Atomics force read and write to memory.
+ static constexpr bool ok = std::is_same_v<X, T> ||
+ (std::atomic<X>::is_always_lock_free // no additional locking
+ && sizeof(std::atomic<X>) == sizeof(X) // layout identical to X.
+ && (std::is_arithmetic_v<X> || std::is_enum_v<X>)); // No gaps in the layout.
+ };
+
+static_assert(Constraints<Container<T>>::ok);
+public:
+ explicit MirroredVariable(const T& t) : t_{t} {}
+
+ // implicit conversion operator
+ operator T() const {
+ return t_;
+ }
+
+ MirroredVariable& operator=(const T& t) {
+ t_ = t;
+ if (mirror_ != nullptr) {
+ *mirror_ = t;
+ }
+ return *this;
+ }
+
+ template <typename U>
+ void setMirror(Container<U> *other_mirror) {
+ // Much of the concern is with T != U, however there are additional concerns
+ // when storage uses shared memory between processes. For atomics, it must be
+ // lock free.
+ static_assert(sizeof(U) == sizeof(T));
+ static_assert(alignof(U) == alignof(T));
+ static_assert(Constraints<Container<U>>::ok);
+ static_assert(sizeof(Container<U>) == sizeof(Container<T>));
+ static_assert(alignof(Container<U>) == alignof(Container<T>));
+ auto mirror = reinterpret_cast<Container<T>*>(other_mirror);
+ if (mirror_ != mirror) {
+ mirror_ = mirror;
+ if (mirror != nullptr) {
+ *mirror = t_;
+ }
+ }
+ }
+
+ void clear() {
+ mirror_ = nullptr;
+ }
+
+ MirroredVariable& operator&() const = delete;
+
+protected:
+ T t_{};
+ Container<T>* mirror_ = nullptr;
+};
+
struct AudioTrackSharedStreaming {
// similar to NBAIO MonoPipe
// in continuously incrementing frame units, take modulo buffer size, which must be a power of 2
@@ -188,6 +265,8 @@
volatile int32_t mFlags; // combinations of CBLK_*
+ std::atomic<int32_t> mState; // current TrackBase state.
+
public:
union {
AudioTrackSharedStreaming mStreaming;
@@ -198,6 +277,9 @@
// Cache line boundary (32 bytes)
};
+// TODO: ensure standard layout.
+// static_assert(std::is_standard_layout_v<audio_track_cblk_t>);
+
// ----------------------------------------------------------------------------
// Proxy for shared memory control block, to isolate callers from needing to know the details.
@@ -323,6 +405,7 @@
return mEpoch;
}
+ int32_t getState() const { return mCblk->mState; }
uint32_t getBufferSizeInFrames() const { return mBufferSizeInFrames; }
// See documentation for AudioTrack::setBufferSizeInFrames()
uint32_t setBufferSizeInFrames(uint32_t requestedSize);
diff --git a/media/codec2/components/aac/C2SoftAacDec.cpp b/media/codec2/components/aac/C2SoftAacDec.cpp
index 342d771..57cdcd0 100644
--- a/media/codec2/components/aac/C2SoftAacDec.cpp
+++ b/media/codec2/components/aac/C2SoftAacDec.cpp
@@ -287,6 +287,7 @@
mOutputDelayRingBufferWritePos = 0;
mOutputDelayRingBufferReadPos = 0;
mOutputDelayRingBufferFilled = 0;
+ mOutputDelayRingBuffer.reset();
mBuffersInfo.clear();
status_t status = UNKNOWN_ERROR;
@@ -308,10 +309,7 @@
aacDecoder_Close(mAACDecoder);
mAACDecoder = nullptr;
}
- if (mOutputDelayRingBuffer) {
- delete[] mOutputDelayRingBuffer;
- mOutputDelayRingBuffer = nullptr;
- }
+ mOutputDelayRingBuffer.reset();
}
status_t C2SoftAacDec::initDecoder() {
@@ -327,7 +325,7 @@
mOutputDelayCompensated = 0;
mOutputDelayRingBufferSize = 2048 * MAX_CHANNEL_COUNT * kNumDelayBlocksMax;
- mOutputDelayRingBuffer = new short[mOutputDelayRingBufferSize];
+ mOutputDelayRingBuffer.reset(new short[mOutputDelayRingBufferSize]);
mOutputDelayRingBufferWritePos = 0;
mOutputDelayRingBufferReadPos = 0;
mOutputDelayRingBufferFilled = 0;
diff --git a/media/codec2/components/aac/C2SoftAacDec.h b/media/codec2/components/aac/C2SoftAacDec.h
index 986187c..a03fc70 100644
--- a/media/codec2/components/aac/C2SoftAacDec.h
+++ b/media/codec2/components/aac/C2SoftAacDec.h
@@ -93,7 +93,7 @@
bool mEndOfOutput;
int32_t mOutputDelayCompensated;
int32_t mOutputDelayRingBufferSize;
- short *mOutputDelayRingBuffer;
+ std::unique_ptr<short[]> mOutputDelayRingBuffer;
int32_t mOutputDelayRingBufferWritePos;
int32_t mOutputDelayRingBufferReadPos;
int32_t mOutputDelayRingBufferFilled;
diff --git a/media/codec2/sfplugin/CCodecBufferChannel.cpp b/media/codec2/sfplugin/CCodecBufferChannel.cpp
index e9adfc9..307396b 100644
--- a/media/codec2/sfplugin/CCodecBufferChannel.cpp
+++ b/media/codec2/sfplugin/CCodecBufferChannel.cpp
@@ -19,6 +19,7 @@
#include <utils/Log.h>
#include <algorithm>
+#include <atomic>
#include <list>
#include <numeric>
@@ -155,6 +156,7 @@
input->pipelineDelay = 0u;
input->numSlots = kSmoothnessFactor;
input->numExtraSlots = 0u;
+ input->lastFlushIndex = 0u;
}
{
Mutexed<Output>::Locked output(mOutput);
@@ -1116,6 +1118,7 @@
input->numSlots = numInputSlots;
input->extraBuffers.flush();
input->numExtraSlots = 0u;
+ input->lastFlushIndex = mFrameIndex.load(std::memory_order_relaxed);
if (audioEncoder && encoderFrameSize && sampleRate && channelCount) {
input->frameReassembler.init(
pool,
@@ -1523,6 +1526,7 @@
ALOGV("[%s] flush", mName);
std::vector<uint64_t> indices;
std::list<std::unique_ptr<C2Work>> configs;
+ mInput.lock()->lastFlushIndex = mFrameIndex.load(std::memory_order_relaxed);
for (const std::unique_ptr<C2Work> &work : flushedWork) {
indices.push_back(work->input.ordinal.frameIndex.peeku());
if (!(work->input.flags & C2FrameData::FLAG_CODEC_CONFIG)) {
@@ -1589,12 +1593,18 @@
}
std::shared_ptr<C2Buffer> buffer =
mPipelineWatcher.lock()->onInputBufferReleased(frameIndex, arrayIndex);
- bool newInputSlotAvailable;
+ bool newInputSlotAvailable = false;
{
Mutexed<Input>::Locked input(mInput);
- newInputSlotAvailable = input->buffers->expireComponentBuffer(buffer);
- if (!newInputSlotAvailable) {
- (void)input->extraBuffers.expireComponentBuffer(buffer);
+ if (input->lastFlushIndex >= frameIndex) {
+ ALOGD("[%s] Ignoring stale input buffer done callback: "
+ "last flush index = %lld, frameIndex = %lld",
+ mName, input->lastFlushIndex.peekll(), (long long)frameIndex);
+ } else {
+ newInputSlotAvailable = input->buffers->expireComponentBuffer(buffer);
+ if (!newInputSlotAvailable) {
+ (void)input->extraBuffers.expireComponentBuffer(buffer);
+ }
}
}
if (newInputSlotAvailable) {
diff --git a/media/codec2/sfplugin/CCodecBufferChannel.h b/media/codec2/sfplugin/CCodecBufferChannel.h
index 5a2aca2..4db69cb 100644
--- a/media/codec2/sfplugin/CCodecBufferChannel.h
+++ b/media/codec2/sfplugin/CCodecBufferChannel.h
@@ -273,6 +273,7 @@
size_t numExtraSlots;
uint32_t inputDelay;
uint32_t pipelineDelay;
+ c2_cntr64_t lastFlushIndex;
FrameReassembler frameReassembler;
};
diff --git a/media/codec2/vndk/C2PlatformStorePluginLoader.cpp b/media/codec2/vndk/C2PlatformStorePluginLoader.cpp
index bee028a..2a888a8 100644
--- a/media/codec2/vndk/C2PlatformStorePluginLoader.cpp
+++ b/media/codec2/vndk/C2PlatformStorePluginLoader.cpp
@@ -59,13 +59,14 @@
c2_status_t C2PlatformStorePluginLoader::createBlockPool(
::C2Allocator::id_t allocatorId, ::C2BlockPool::local_id_t blockPoolId,
- std::shared_ptr<C2BlockPool>* pool) {
+ std::shared_ptr<C2BlockPool>* pool,
+ std::function<void(C2BlockPool *)> deleter) {
if (mCreateBlockPool == nullptr) {
ALOGD("Handle or CreateBlockPool symbol is null");
return C2_NOT_FOUND;
}
- std::shared_ptr<::C2BlockPool> ptr(mCreateBlockPool(allocatorId, blockPoolId));
+ std::shared_ptr<::C2BlockPool> ptr(mCreateBlockPool(allocatorId, blockPoolId), deleter);
if (ptr) {
*pool = ptr;
return C2_OK;
@@ -75,14 +76,16 @@
}
c2_status_t C2PlatformStorePluginLoader::createAllocator(
- ::C2Allocator::id_t allocatorId, std::shared_ptr<C2Allocator>* const allocator) {
+ ::C2Allocator::id_t allocatorId,
+ std::shared_ptr<C2Allocator>* const allocator,
+ std::function<void(C2Allocator *)> deleter) {
if (mCreateAllocator == nullptr) {
ALOGD("Handle or CreateAllocator symbol is null");
return C2_NOT_FOUND;
}
c2_status_t res = C2_CORRUPTED;
- allocator->reset(mCreateAllocator(allocatorId, &res));
+ allocator->reset(mCreateAllocator(allocatorId, &res), deleter);
if (res != C2_OK) {
ALOGD("Failed to CreateAllocator by id: %u, res: %d", allocatorId, res);
allocator->reset();
diff --git a/media/codec2/vndk/C2Store.cpp b/media/codec2/vndk/C2Store.cpp
index c07c09e..1660c38 100644
--- a/media/codec2/vndk/C2Store.cpp
+++ b/media/codec2/vndk/C2Store.cpp
@@ -443,6 +443,7 @@
public:
_C2BlockPoolCache() : mBlockPoolSeqId(C2BlockPool::PLATFORM_START + 1) {}
+private:
c2_status_t _createBlockPool(
C2PlatformAllocatorStore::id_t allocatorId,
std::vector<std::shared_ptr<const C2Component>> components,
@@ -456,14 +457,19 @@
if (allocatorId == C2AllocatorStore::DEFAULT_LINEAR) {
allocatorId = GetPreferredLinearAllocatorId(GetCodec2PoolMask());
}
+ auto deleter = [this, poolId](C2BlockPool *pool) {
+ std::unique_lock lock(mMutex);
+ mBlockPools.erase(poolId);
+ mComponents.erase(poolId);
+ delete pool;
+ };
switch(allocatorId) {
case C2PlatformAllocatorStore::ION: /* also ::DMABUFHEAP */
res = allocatorStore->fetchAllocator(
C2PlatformAllocatorStore::ION, &allocator);
if (res == C2_OK) {
- std::shared_ptr<C2BlockPool> ptr =
- std::make_shared<C2PooledBlockPool>(
- allocator, poolId);
+ std::shared_ptr<C2BlockPool> ptr(
+ new C2PooledBlockPool(allocator, poolId), deleter);
*pool = ptr;
mBlockPools[poolId] = ptr;
mComponents[poolId].insert(
@@ -475,9 +481,8 @@
res = allocatorStore->fetchAllocator(
C2PlatformAllocatorStore::BLOB, &allocator);
if (res == C2_OK) {
- std::shared_ptr<C2BlockPool> ptr =
- std::make_shared<C2PooledBlockPool>(
- allocator, poolId);
+ std::shared_ptr<C2BlockPool> ptr(
+ new C2PooledBlockPool(allocator, poolId), deleter);
*pool = ptr;
mBlockPools[poolId] = ptr;
mComponents[poolId].insert(
@@ -490,8 +495,8 @@
res = allocatorStore->fetchAllocator(
C2AllocatorStore::DEFAULT_GRAPHIC, &allocator);
if (res == C2_OK) {
- std::shared_ptr<C2BlockPool> ptr =
- std::make_shared<C2PooledBlockPool>(allocator, poolId);
+ std::shared_ptr<C2BlockPool> ptr(
+ new C2PooledBlockPool(allocator, poolId), deleter);
*pool = ptr;
mBlockPools[poolId] = ptr;
mComponents[poolId].insert(
@@ -503,9 +508,8 @@
res = allocatorStore->fetchAllocator(
C2PlatformAllocatorStore::BUFFERQUEUE, &allocator);
if (res == C2_OK) {
- std::shared_ptr<C2BlockPool> ptr =
- std::make_shared<C2BufferQueueBlockPool>(
- allocator, poolId);
+ std::shared_ptr<C2BlockPool> ptr(
+ new C2BufferQueueBlockPool(allocator, poolId), deleter);
*pool = ptr;
mBlockPools[poolId] = ptr;
mComponents[poolId].insert(
@@ -517,7 +521,7 @@
// Try to create block pool from platform store plugins.
std::shared_ptr<C2BlockPool> ptr;
res = C2PlatformStorePluginLoader::GetInstance()->createBlockPool(
- allocatorId, poolId, &ptr);
+ allocatorId, poolId, &ptr, deleter);
if (res == C2_OK) {
*pool = ptr;
mBlockPools[poolId] = ptr;
@@ -530,17 +534,20 @@
return res;
}
+public:
c2_status_t createBlockPool(
C2PlatformAllocatorStore::id_t allocatorId,
std::vector<std::shared_ptr<const C2Component>> components,
std::shared_ptr<C2BlockPool> *pool) {
+ std::unique_lock lock(mMutex);
return _createBlockPool(allocatorId, components, mBlockPoolSeqId++, pool);
}
- bool getBlockPool(
+ c2_status_t getBlockPool(
C2BlockPool::local_id_t blockPoolId,
std::shared_ptr<const C2Component> component,
std::shared_ptr<C2BlockPool> *pool) {
+ std::unique_lock lock(mMutex);
// TODO: use one iterator for multiple blockpool type scalability.
std::shared_ptr<C2BlockPool> ptr;
auto it = mBlockPools.find(blockPoolId);
@@ -558,14 +565,22 @@
});
if (found != mComponents[blockPoolId].end()) {
*pool = ptr;
- return true;
+ return C2_OK;
}
}
}
- return false;
+ // TODO: remove this. this is temporary
+ if (blockPoolId == C2BlockPool::PLATFORM_START) {
+ return _createBlockPool(
+ C2PlatformAllocatorStore::BUFFERQUEUE, {component}, blockPoolId, pool);
+ }
+ return C2_NOT_FOUND;
}
private:
+ // Deleter needs to hold this mutex, and there is a small chance that deleter
+ // is invoked while the mutex is held.
+ std::recursive_mutex mMutex;
C2BlockPool::local_id_t mBlockPoolSeqId;
std::map<C2BlockPool::local_id_t, std::weak_ptr<C2BlockPool>> mBlockPools;
@@ -574,7 +589,6 @@
static std::unique_ptr<_C2BlockPoolCache> sBlockPoolCache =
std::make_unique<_C2BlockPoolCache>();
-static std::mutex sBlockPoolCacheMutex;
} // anynymous namespace
@@ -582,15 +596,12 @@
C2BlockPool::local_id_t id, std::shared_ptr<const C2Component> component,
std::shared_ptr<C2BlockPool> *pool) {
pool->reset();
- std::lock_guard<std::mutex> lock(sBlockPoolCacheMutex);
std::shared_ptr<C2AllocatorStore> allocatorStore = GetCodec2PlatformAllocatorStore();
std::shared_ptr<C2Allocator> allocator;
c2_status_t res = C2_NOT_FOUND;
if (id >= C2BlockPool::PLATFORM_START) {
- if (sBlockPoolCache->getBlockPool(id, component, pool)) {
- return C2_OK;
- }
+ return sBlockPoolCache->getBlockPool(id, component, pool);
}
switch (id) {
@@ -606,11 +617,6 @@
*pool = std::make_shared<C2BasicGraphicBlockPool>(allocator);
}
break;
- // TODO: remove this. this is temporary
- case C2BlockPool::PLATFORM_START:
- res = sBlockPoolCache->_createBlockPool(
- C2PlatformAllocatorStore::BUFFERQUEUE, {component}, id, pool);
- break;
default:
break;
}
@@ -623,7 +629,6 @@
std::shared_ptr<C2BlockPool> *pool) {
pool->reset();
- std::lock_guard<std::mutex> lock(sBlockPoolCacheMutex);
return sBlockPoolCache->createBlockPool(allocatorId, components, pool);
}
@@ -633,7 +638,6 @@
std::shared_ptr<C2BlockPool> *pool) {
pool->reset();
- std::lock_guard<std::mutex> lock(sBlockPoolCacheMutex);
return sBlockPoolCache->createBlockPool(allocatorId, {component}, pool);
}
diff --git a/media/codec2/vndk/include/C2PlatformStorePluginLoader.h b/media/codec2/vndk/include/C2PlatformStorePluginLoader.h
index 4c10643..73d1b5e 100644
--- a/media/codec2/vndk/include/C2PlatformStorePluginLoader.h
+++ b/media/codec2/vndk/include/C2PlatformStorePluginLoader.h
@@ -61,9 +61,11 @@
* \retval C2_NOT_FOUND the extension symbol was not found.
* \retval C2_BAD_INDEX the input allocatorId is not defined in platform store extension.
*/
- c2_status_t createBlockPool(::C2Allocator::id_t allocatorId,
- ::C2BlockPool::local_id_t blockPoolId,
- std::shared_ptr<C2BlockPool>* pool);
+ c2_status_t createBlockPool(
+ ::C2Allocator::id_t allocatorId,
+ ::C2BlockPool::local_id_t blockPoolId,
+ std::shared_ptr<C2BlockPool>* pool,
+ std::function<void(C2BlockPool *)> deleter = std::default_delete<C2BlockPool>());
/**
* Creates allocator from platform store extension.
@@ -81,8 +83,10 @@
* \retval C2_BAD_INDEX the input allocatorId is not defined in platform store extension.
* \retval C2_NO_MEMORY not enough memory to create the allocator
*/
- c2_status_t createAllocator(::C2Allocator::id_t allocatorId,
- std::shared_ptr<C2Allocator>* const allocator);
+ c2_status_t createAllocator(
+ ::C2Allocator::id_t allocatorId,
+ std::shared_ptr<C2Allocator>* const allocator,
+ std::function<void(C2Allocator *)> deleter = std::default_delete<C2Allocator>());
private:
explicit C2PlatformStorePluginLoader(const char *libPath);
diff --git a/media/codecs/m4v_h263/enc/src/mp4enc_api.cpp b/media/codecs/m4v_h263/enc/src/mp4enc_api.cpp
index 234faef..00b2ab6 100644
--- a/media/codecs/m4v_h263/enc/src/mp4enc_api.cpp
+++ b/media/codecs/m4v_h263/enc/src/mp4enc_api.cpp
@@ -610,18 +610,19 @@
/* Find the maximum width*height for memory allocation of the VOPs */
for (idx = 0; idx < nLayers; idx++)
{
- temp_w = video->encParams->LayerWidth[idx];
- temp_h = video->encParams->LayerHeight[idx];
+ temp_w = ((video->encParams->LayerWidth[idx] + 15) >> 4) << 4;
+ temp_h = ((video->encParams->LayerHeight[idx] + 15) >> 4) << 4;
+
+ if (temp_w > 2048 || temp_h > 2048) {
+ goto CLEAN_UP;
+ }
if ((temp_w*temp_h) > max)
{
max = temp_w * temp_h;
- max_width = ((temp_w + 15) >> 4) << 4;
- max_height = ((temp_h + 15) >> 4) << 4;
- if (((uint64_t)max_width * max_height) > (uint64_t)INT32_MAX
- || temp_w > INT32_MAX - 15 || temp_h > INT32_MAX - 15) {
- goto CLEAN_UP;
- }
+ max_width = temp_w;
+ max_height = temp_h;
+
nTotalMB = ((max_width * max_height) >> 8);
}
diff --git a/media/libaudioclient/AudioTrack.cpp b/media/libaudioclient/AudioTrack.cpp
index 5f802de..3bc666b 100644
--- a/media/libaudioclient/AudioTrack.cpp
+++ b/media/libaudioclient/AudioTrack.cpp
@@ -21,6 +21,7 @@
#include <inttypes.h>
#include <math.h>
#include <sys/resource.h>
+#include <thread>
#include <android/media/IAudioPolicyService.h>
#include <android-base/macros.h>
@@ -947,6 +948,44 @@
mAudioTrack->flush();
}
+bool AudioTrack::pauseAndWait(const std::chrono::milliseconds& timeout)
+{
+ using namespace std::chrono_literals;
+
+ pause();
+
+ AutoMutex lock(mLock);
+ // offload and direct tracks do not wait because pause volume ramp is handled by hardware.
+ if (isOffloadedOrDirect_l()) return true;
+
+ // Wait for the track state to be anything besides pausing.
+ // This ensures that the volume has ramped down.
+ constexpr auto SLEEP_INTERVAL_MS = 10ms;
+ auto begin = std::chrono::steady_clock::now();
+ while (true) {
+ // wait for state to change
+ const int state = mProxy->getState();
+
+ mLock.unlock(); // only local variables accessed until lock.
+ auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
+ std::chrono::steady_clock::now() - begin);
+ if (state != CBLK_STATE_PAUSING) {
+ ALOGV("%s: success state:%d after %lld ms", __func__, state, elapsed.count());
+ return true;
+ }
+ std::chrono::milliseconds remaining = timeout - elapsed;
+ if (remaining.count() <= 0) {
+ ALOGW("%s: timeout expired state:%d still pausing:%d after %lld ms",
+ __func__, state, CBLK_STATE_PAUSING, elapsed.count());
+ return false;
+ }
+ // It is conceivable that the track is restored while sleeping;
+ // as this logic is advisory, we allow that.
+ std::this_thread::sleep_for(std::min(remaining, SLEEP_INTERVAL_MS));
+ mLock.lock();
+ }
+}
+
void AudioTrack::pause()
{
const int64_t beginNs = systemTime();
diff --git a/media/libaudioclient/include/media/AudioTrack.h b/media/libaudioclient/include/media/AudioTrack.h
index cb00990..6b592cb 100644
--- a/media/libaudioclient/include/media/AudioTrack.h
+++ b/media/libaudioclient/include/media/AudioTrack.h
@@ -28,6 +28,7 @@
#include <utils/threads.h>
#include <android/content/AttributionSourceState.h>
+#include <chrono>
#include <string>
#include "android/media/BnAudioTrackCallback.h"
@@ -510,6 +511,14 @@
*/
void pause();
+ /* Pause and wait (with timeout) for the audio track to ramp to silence.
+ *
+ * \param timeout is the time limit to wait before returning.
+ * A negative number is treated as 0.
+ * \return true if the track is ramped to silence, false if the timeout occurred.
+ */
+ bool pauseAndWait(const std::chrono::milliseconds& timeout);
+
/* Set volume for this track, mostly used for games' sound effects
* left and right volumes. Levels must be >= 0.0 and <= 1.0.
* This is the older API. New applications should use setVolume(float) when possible.
diff --git a/media/libeffects/lvm/lib/Eq/src/LVEQNB_Control.cpp b/media/libeffects/lvm/lib/Eq/src/LVEQNB_Control.cpp
index 7e5caed..1eadd27 100644
--- a/media/libeffects/lvm/lib/Eq/src/LVEQNB_Control.cpp
+++ b/media/libeffects/lvm/lib/Eq/src/LVEQNB_Control.cpp
@@ -313,9 +313,9 @@
*/
pInstance->eqBiquad.resize(pParams->NBands,
android::audio_utils::BiquadFilter<LVM_FLOAT>(pParams->NrChannels));
- LVEQNB_ClearFilterHistory(pInstance);
if (bChange || modeChange) {
+ LVEQNB_ClearFilterHistory(pInstance);
/*
* If the sample rate has changed clear the history
*/
diff --git a/media/libeffects/lvm/tests/EffectBundleTest.cpp b/media/libeffects/lvm/tests/EffectBundleTest.cpp
index 881ffb1..018cb7c 100644
--- a/media/libeffects/lvm/tests/EffectBundleTest.cpp
+++ b/media/libeffects/lvm/tests/EffectBundleTest.cpp
@@ -14,29 +14,39 @@
* limitations under the License.
*/
+#include <system/audio_effects/effect_bassboost.h>
+#include <system/audio_effects/effect_equalizer.h>
+#include <system/audio_effects/effect_virtualizer.h>
#include "EffectTestHelper.h"
-using namespace android;
-// Update isBassBoost, if the order of effects is updated
-constexpr effect_uuid_t kEffectUuids[] = {
+using namespace android;
+typedef enum {
+ EFFECT_BASS_BOOST,
+ EFFECT_EQUALIZER,
+ EFFECT_VIRTUALIZER,
+ EFFECT_VOLUME
+} effect_type_t;
+
+const std::map<effect_type_t, effect_uuid_t> kEffectUuids = {
// NXP SW BassBoost
- {0x8631f300, 0x72e2, 0x11df, 0xb57e, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}},
- // NXP SW Virtualizer
- {0x1d4033c0, 0x8557, 0x11df, 0x9f2d, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}},
+ {EFFECT_BASS_BOOST,
+ {0x8631f300, 0x72e2, 0x11df, 0xb57e, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}},
// NXP SW Equalizer
- {0xce772f20, 0x847d, 0x11df, 0xbb17, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}},
+ {EFFECT_EQUALIZER,
+ {0xce772f20, 0x847d, 0x11df, 0xbb17, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}},
+ // NXP SW Virtualizer
+ {EFFECT_VIRTUALIZER,
+ {0x1d4033c0, 0x8557, 0x11df, 0x9f2d, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}},
// NXP SW Volume
- {0x119341a0, 0x8469, 0x11df, 0x81f9, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}},
+ {EFFECT_VOLUME, {0x119341a0, 0x8469, 0x11df, 0x81f9, {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}}},
};
-static bool isBassBoost(const effect_uuid_t* uuid) {
- // Update this, if the order of effects in kEffectUuids is updated
- return uuid == &kEffectUuids[0];
-}
+const size_t kNumEffectUuids = std::size(kEffectUuids);
-constexpr size_t kNumEffectUuids = std::size(kEffectUuids);
+constexpr float kMinAmplitude = -1.0f;
+constexpr float kMaxAmplitude = 1.0f;
-typedef std::tuple<int, int, int, int, int> SingleEffectTestParam;
+using SingleEffectTestParam = std::tuple<int, int, int, int, int>;
class SingleEffectTest : public ::testing::TestWithParam<SingleEffectTestParam> {
public:
SingleEffectTest()
@@ -46,7 +56,8 @@
mFrameCount(EffectTestHelper::kFrameCounts[std::get<2>(GetParam())]),
mLoopCount(EffectTestHelper::kLoopCounts[std::get<3>(GetParam())]),
mTotalFrameCount(mFrameCount * mLoopCount),
- mUuid(&kEffectUuids[std::get<4>(GetParam())]) {}
+ mEffectType((effect_type_t)std::get<4>(GetParam())),
+ mUuid(kEffectUuids.at(mEffectType)) {}
const size_t mChMask;
const size_t mChannelCount;
@@ -54,7 +65,8 @@
const size_t mFrameCount;
const size_t mLoopCount;
const size_t mTotalFrameCount;
- const effect_uuid_t* mUuid;
+ const effect_type_t mEffectType;
+ const effect_uuid_t mUuid;
};
// Tests applying a single effect
@@ -63,7 +75,7 @@
<< "chMask: " << mChMask << " sampleRate: " << mSampleRate
<< " frameCount: " << mFrameCount << " loopCount: " << mLoopCount);
- EffectTestHelper effect(mUuid, mChMask, mChMask, mSampleRate, mFrameCount, mLoopCount);
+ EffectTestHelper effect(&mUuid, mChMask, mChMask, mSampleRate, mFrameCount, mLoopCount);
ASSERT_NO_FATAL_FAILURE(effect.createEffect());
ASSERT_NO_FATAL_FAILURE(effect.setConfig());
@@ -72,7 +84,7 @@
std::vector<float> input(mTotalFrameCount * mChannelCount);
std::vector<float> output(mTotalFrameCount * mChannelCount);
std::minstd_rand gen(mChMask);
- std::uniform_real_distribution<> dis(-1.0f, 1.0f);
+ std::uniform_real_distribution<> dis(kMinAmplitude, kMaxAmplitude);
for (auto& in : input) {
in = dis(gen);
}
@@ -88,7 +100,7 @@
::testing::Range(0, (int)EffectTestHelper::kNumLoopCounts),
::testing::Range(0, (int)kNumEffectUuids)));
-typedef std::tuple<int, int, int, int> SingleEffectComparisonTestParam;
+using SingleEffectComparisonTestParam = std::tuple<int, int, int, int>;
class SingleEffectComparisonTest
: public ::testing::TestWithParam<SingleEffectComparisonTestParam> {
public:
@@ -97,13 +109,15 @@
mFrameCount(EffectTestHelper::kFrameCounts[std::get<1>(GetParam())]),
mLoopCount(EffectTestHelper::kLoopCounts[std::get<2>(GetParam())]),
mTotalFrameCount(mFrameCount * mLoopCount),
- mUuid(&kEffectUuids[std::get<3>(GetParam())]) {}
+ mEffectType((effect_type_t)std::get<3>(GetParam())),
+ mUuid(kEffectUuids.at(mEffectType)) {}
const size_t mSampleRate;
const size_t mFrameCount;
const size_t mLoopCount;
const size_t mTotalFrameCount;
- const effect_uuid_t* mUuid;
+ const effect_type_t mEffectType;
+ const effect_uuid_t mUuid;
};
// Compares first two channels in multi-channel output to stereo output when same effect is applied
@@ -115,7 +129,7 @@
std::vector<float> monoInput(mTotalFrameCount);
std::minstd_rand gen(mSampleRate);
- std::uniform_real_distribution<> dis(-1.0f, 1.0f);
+ std::uniform_real_distribution<> dis(kMinAmplitude, kMaxAmplitude);
for (auto& in : monoInput) {
in = dis(gen);
}
@@ -126,7 +140,7 @@
mTotalFrameCount * sizeof(float) * FCC_1);
// Apply effect on stereo channels
- EffectTestHelper stereoEffect(mUuid, AUDIO_CHANNEL_OUT_STEREO, AUDIO_CHANNEL_OUT_STEREO,
+ EffectTestHelper stereoEffect(&mUuid, AUDIO_CHANNEL_OUT_STEREO, AUDIO_CHANNEL_OUT_STEREO,
mSampleRate, mFrameCount, mLoopCount);
ASSERT_NO_FATAL_FAILURE(stereoEffect.createEffect());
@@ -142,7 +156,7 @@
for (size_t chMask : EffectTestHelper::kChMasks) {
size_t channelCount = audio_channel_count_from_out_mask(chMask);
- EffectTestHelper testEffect(mUuid, chMask, chMask, mSampleRate, mFrameCount, mLoopCount);
+ EffectTestHelper testEffect(&mUuid, chMask, chMask, mSampleRate, mFrameCount, mLoopCount);
ASSERT_NO_FATAL_FAILURE(testEffect.createEffect());
ASSERT_NO_FATAL_FAILURE(testEffect.setConfig());
@@ -170,7 +184,7 @@
memcpy_to_i16_from_float(stereoTestI16.data(), stereoTestOutput.data(),
mTotalFrameCount * FCC_2);
- if (isBassBoost(mUuid)) {
+ if (EFFECT_BASS_BOOST == mEffectType) {
// SNR must be above the threshold
float snr = computeSnr<int16_t>(stereoRefI16.data(), stereoTestI16.data(),
mTotalFrameCount * FCC_2);
@@ -191,6 +205,135 @@
::testing::Range(0, (int)EffectTestHelper::kNumLoopCounts),
::testing::Range(0, (int)kNumEffectUuids)));
+using SingleEffectDefaultSetParamTestParam = std::tuple<int, int, int>;
+class SingleEffectDefaultSetParamTest
+ : public ::testing::TestWithParam<SingleEffectDefaultSetParamTestParam> {
+ public:
+ SingleEffectDefaultSetParamTest()
+ : mChMask(EffectTestHelper::kChMasks[std::get<0>(GetParam())]),
+ mChannelCount(audio_channel_count_from_out_mask(mChMask)),
+ mSampleRate(16000),
+ mFrameCount(EffectTestHelper::kFrameCounts[std::get<1>(GetParam())]),
+ mLoopCount(1),
+ mTotalFrameCount(mFrameCount * mLoopCount),
+ mEffectType((effect_type_t)std::get<2>(GetParam())),
+ mUuid(kEffectUuids.at(mEffectType)) {}
+
+ const size_t mChMask;
+ const size_t mChannelCount;
+ const size_t mSampleRate;
+ const size_t mFrameCount;
+ const size_t mLoopCount;
+ const size_t mTotalFrameCount;
+ const effect_type_t mEffectType;
+ const effect_uuid_t mUuid;
+};
+
+// Tests verifying that redundant setParam calls do not alter output
+TEST_P(SingleEffectDefaultSetParamTest, SimpleProcess) {
+ SCOPED_TRACE(testing::Message()
+ << "chMask: " << mChMask << " sampleRate: " << mSampleRate
+ << " frameCount: " << mFrameCount << " loopCount: " << mLoopCount);
+ // effect.process() handles mTotalFrameCount * mChannelCount samples in each call.
+ // This test calls process() twice per effect, hence total samples when allocating
+ // input and output vectors is twice the number of samples processed in one call.
+ size_t totalNumSamples = 2 * mTotalFrameCount * mChannelCount;
+ // Initialize input buffer with deterministic pseudo-random values
+ std::vector<float> input(totalNumSamples);
+ std::minstd_rand gen(mChMask);
+ std::uniform_real_distribution<> dis(kMinAmplitude, kMaxAmplitude);
+ for (auto& in : input) {
+ in = dis(gen);
+ }
+
+ uint32_t key;
+ int32_t value1, value2;
+ switch (mEffectType) {
+ case EFFECT_BASS_BOOST:
+ key = BASSBOOST_PARAM_STRENGTH;
+ value1 = 1;
+ value2 = 14;
+ break;
+ case EFFECT_VIRTUALIZER:
+ key = VIRTUALIZER_PARAM_STRENGTH;
+ value1 = 0;
+ value2 = 100;
+ break;
+ case EFFECT_EQUALIZER:
+ key = EQ_PARAM_CUR_PRESET;
+ value1 = 0;
+ value2 = 1;
+ break;
+ case EFFECT_VOLUME:
+ key = 0 /* VOLUME_PARAM_LEVEL */;
+ value1 = 0;
+ value2 = -100;
+ break;
+ default:
+ FAIL() << "Unsupported effect type : " << mEffectType;
+ }
+
+ EffectTestHelper refEffect(&mUuid, mChMask, mChMask, mSampleRate, mFrameCount, mLoopCount);
+
+ ASSERT_NO_FATAL_FAILURE(refEffect.createEffect());
+ ASSERT_NO_FATAL_FAILURE(refEffect.setConfig());
+
+ if (EFFECT_BASS_BOOST == mEffectType) {
+ ASSERT_NO_FATAL_FAILURE(refEffect.setParam<int16_t>(key, value1));
+ } else {
+ ASSERT_NO_FATAL_FAILURE(refEffect.setParam<int32_t>(key, value1));
+ }
+ std::vector<float> refOutput(totalNumSamples);
+ float* pInput = input.data();
+ float* pOutput = refOutput.data();
+ ASSERT_NO_FATAL_FAILURE(refEffect.process(pInput, pOutput));
+
+ pInput += totalNumSamples / 2;
+ pOutput += totalNumSamples / 2;
+ ASSERT_NO_FATAL_FAILURE(refEffect.process(pInput, pOutput));
+ ASSERT_NO_FATAL_FAILURE(refEffect.releaseEffect());
+
+ EffectTestHelper testEffect(&mUuid, mChMask, mChMask, mSampleRate, mFrameCount, mLoopCount);
+
+ ASSERT_NO_FATAL_FAILURE(testEffect.createEffect());
+ ASSERT_NO_FATAL_FAILURE(testEffect.setConfig());
+
+ if (EFFECT_BASS_BOOST == mEffectType) {
+ ASSERT_NO_FATAL_FAILURE(testEffect.setParam<int16_t>(key, value1));
+ } else {
+ ASSERT_NO_FATAL_FAILURE(testEffect.setParam<int32_t>(key, value1));
+ }
+
+ std::vector<float> testOutput(totalNumSamples);
+ pInput = input.data();
+ pOutput = testOutput.data();
+ ASSERT_NO_FATAL_FAILURE(testEffect.process(pInput, pOutput));
+
+ // Call setParam once to change the parameters, and then call setParam again
+ // to restore the parameters to the initial state, making the first setParam
+ // call redundant
+ if (EFFECT_BASS_BOOST == mEffectType) {
+ ASSERT_NO_FATAL_FAILURE(testEffect.setParam<int16_t>(key, value2));
+ ASSERT_NO_FATAL_FAILURE(testEffect.setParam<int16_t>(key, value1));
+ } else {
+ ASSERT_NO_FATAL_FAILURE(testEffect.setParam<int32_t>(key, value2));
+ ASSERT_NO_FATAL_FAILURE(testEffect.setParam<int32_t>(key, value1));
+ }
+
+ pInput += totalNumSamples / 2;
+ pOutput += totalNumSamples / 2;
+ ASSERT_NO_FATAL_FAILURE(testEffect.process(pInput, pOutput));
+ ASSERT_NO_FATAL_FAILURE(testEffect.releaseEffect());
+ ASSERT_TRUE(areNearlySame(refOutput.data(), testOutput.data(), totalNumSamples))
+ << "Outputs do not match with default setParam calls";
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ EffectBundleTestAll, SingleEffectDefaultSetParamTest,
+ ::testing::Combine(::testing::Range(0, (int)EffectTestHelper::kNumChMasks),
+ ::testing::Range(0, (int)EffectTestHelper::kNumFrameCounts),
+ ::testing::Range(0, (int)kNumEffectUuids)));
+
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
int status = RUN_ALL_TESTS();
diff --git a/media/libeffects/lvm/tests/EffectTestHelper.cpp b/media/libeffects/lvm/tests/EffectTestHelper.cpp
index 625c15a..ec727c7 100644
--- a/media/libeffects/lvm/tests/EffectTestHelper.cpp
+++ b/media/libeffects/lvm/tests/EffectTestHelper.cpp
@@ -50,23 +50,6 @@
ASSERT_EQ(reply, 0) << "cmd_enable reply non zero " << reply;
}
-void EffectTestHelper::setParam(uint32_t type, uint32_t value) {
- int reply = 0;
- uint32_t replySize = sizeof(reply);
- uint32_t paramData[2] = {type, value};
- auto effectParam = new effect_param_t[sizeof(effect_param_t) + sizeof(paramData)];
- memcpy(&effectParam->data[0], ¶mData[0], sizeof(paramData));
- effectParam->psize = sizeof(paramData[0]);
- effectParam->vsize = sizeof(paramData[1]);
- int status = (*mEffectHandle)
- ->command(mEffectHandle, EFFECT_CMD_SET_PARAM,
- sizeof(effect_param_t) + sizeof(paramData), effectParam,
- &replySize, &reply);
- delete[] effectParam;
- ASSERT_EQ(status, 0) << "set_param returned an error " << status;
- ASSERT_EQ(reply, 0) << "set_param reply non zero " << reply;
-}
-
void EffectTestHelper::process(float* input, float* output) {
audio_buffer_t inBuffer = {.frameCount = mFrameCount, .f32 = input};
audio_buffer_t outBuffer = {.frameCount = mFrameCount, .f32 = output};
diff --git a/media/libeffects/lvm/tests/EffectTestHelper.h b/media/libeffects/lvm/tests/EffectTestHelper.h
index 3854d46..bcee84e 100644
--- a/media/libeffects/lvm/tests/EffectTestHelper.h
+++ b/media/libeffects/lvm/tests/EffectTestHelper.h
@@ -50,6 +50,23 @@
return snr;
}
+template <typename T>
+static float areNearlySame(const T* ref, const T* tst, size_t count) {
+ T delta;
+ if constexpr (std::is_floating_point_v<T>) {
+ delta = std::numeric_limits<T>::epsilon();
+ } else {
+ delta = 1;
+ }
+ for (size_t i = 0; i < count; ++i) {
+ const double diff(tst[i] - ref[i]);
+ if (abs(diff) > delta) {
+ return false;
+ }
+ }
+ return true;
+}
+
class EffectTestHelper {
public:
EffectTestHelper(const effect_uuid_t* uuid, size_t inChMask, size_t outChMask,
@@ -65,7 +82,25 @@
void createEffect();
void releaseEffect();
void setConfig();
- void setParam(uint32_t type, uint32_t val);
+ template <typename VALUE_DTYPE>
+ void setParam(uint32_t type, VALUE_DTYPE const value) {
+ int reply = 0;
+ uint32_t replySize = sizeof(reply);
+
+ uint8_t paramData[sizeof(effect_param_t) + sizeof(type) + sizeof(value)];
+ auto effectParam = (effect_param_t*)paramData;
+
+ memcpy(&effectParam->data[0], &type, sizeof(type));
+ memcpy(&effectParam->data[sizeof(type)], &value, sizeof(value));
+ effectParam->psize = sizeof(type);
+ effectParam->vsize = sizeof(value);
+ int status = (*mEffectHandle)
+ ->command(mEffectHandle, EFFECT_CMD_SET_PARAM,
+ sizeof(effect_param_t) + sizeof(type) + sizeof(value),
+ effectParam, &replySize, &reply);
+ ASSERT_EQ(status, 0) << "set_param returned an error " << status;
+ ASSERT_EQ(reply, 0) << "set_param reply non zero " << reply;
+ };
void process(float* input, float* output);
// Corresponds to SNR for 1 bit difference between two int16_t signals
diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index d278a01..f85887e 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -21,6 +21,7 @@
#define LOG_TAG "MediaPlayerService"
#include <utils/Log.h>
+#include <chrono>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/time.h>
@@ -2467,8 +2468,13 @@
void MediaPlayerService::AudioOutput::pause()
{
ALOGV("pause");
+ // We use pauseAndWait() instead of pause() to ensure tracks ramp to silence before
+ // any flush. We choose 40 ms timeout to allow 1 deep buffer mixer period
+ // to occur. Often waiting is 0 - 20 ms.
+ using namespace std::chrono_literals;
+ constexpr auto TIMEOUT_MS = 40ms;
Mutex::Autolock lock(mLock);
- if (mTrack != 0) mTrack->pause();
+ if (mTrack != 0) mTrack->pauseAndWait(TIMEOUT_MS);
}
void MediaPlayerService::AudioOutput::close()
diff --git a/media/libstagefright/FrameDecoder.cpp b/media/libstagefright/FrameDecoder.cpp
index efd4070..b513e45 100644
--- a/media/libstagefright/FrameDecoder.cpp
+++ b/media/libstagefright/FrameDecoder.cpp
@@ -883,9 +883,18 @@
}
int32_t width, height, stride;
- CHECK(outputFormat->findInt32("width", &width));
- CHECK(outputFormat->findInt32("height", &height));
- CHECK(outputFormat->findInt32("stride", &stride));
+ if (outputFormat->findInt32("width", &width) == false) {
+ ALOGE("MediaImageDecoder::onOutputReceived:width is missing in outputFormat");
+ return ERROR_MALFORMED;
+ }
+ if (outputFormat->findInt32("height", &height) == false) {
+ ALOGE("MediaImageDecoder::onOutputReceived:height is missing in outputFormat");
+ return ERROR_MALFORMED;
+ }
+ if (outputFormat->findInt32("stride", &stride) == false) {
+ ALOGE("MediaImageDecoder::onOutputReceived:stride is missing in outputFormat");
+ return ERROR_MALFORMED;
+ }
if (mFrame == NULL) {
sp<IMemory> frameMem = allocVideoFrame(
diff --git a/media/libstagefright/MediaAppender.cpp b/media/libstagefright/MediaAppender.cpp
index 5d80b30..21dcfa1 100644
--- a/media/libstagefright/MediaAppender.cpp
+++ b/media/libstagefright/MediaAppender.cpp
@@ -75,10 +75,21 @@
return status;
}
- if (strcmp("MPEG4Extractor", mExtractor->getName()) == 0) {
+ sp<AMessage> fileFormat;
+ status = mExtractor->getFileFormat(&fileFormat);
+ if (status != OK) {
+ ALOGE("extractor_getFileFormat failed, status :%d", status);
+ return status;
+ }
+
+ AString fileMime;
+ fileFormat->findString("mime", &fileMime);
+ // only compare the end of the file MIME type to allow for vendor customized mime type
+ if (fileMime.endsWith("mp4")){
mFormat = MediaMuxer::OUTPUT_FORMAT_MPEG_4;
} else {
- ALOGE("Unsupported format, extractor name:%s", mExtractor->getName());
+ ALOGE("Unsupported file format, extractor name:%s, fileformat %s",
+ mExtractor->getName(), fileMime.c_str());
return ERROR_UNSUPPORTED;
}
diff --git a/media/libstagefright/rtsp/JitterCalculator.cpp b/media/libstagefright/rtsp/JitterCalculator.cpp
index 93b5a83..7e60be2 100644
--- a/media/libstagefright/rtsp/JitterCalculator.cpp
+++ b/media/libstagefright/rtsp/JitterCalculator.cpp
@@ -38,14 +38,13 @@
mInterArrivalJitterUs = inter;
}
-void JitterCalc::putBaseData(int64_t rtpTime, int64_t arrivalTimeUs) {
- // A RTP time wraps around after UINT32_MAX. We must consider this case.
- const int64_t UINT32_MSB = 0x80000000;
- int64_t overflowMask = (mFirstTimeStamp & UINT32_MSB & ~rtpTime) << 1;
- int64_t tempRtpTime = overflowMask | rtpTime;
+void JitterCalc::putBaseData(uint32_t rtpTime, int64_t arrivalTimeUs) {
+ // A RTP time wraps around after UINT32_MAX. Overflow can present.
+ uint32_t diff = 0;
+ __builtin_usub_overflow(rtpTime, mFirstTimeStamp, &diff);
// Base jitter implementation can be various
- int64_t scheduledTimeUs = (tempRtpTime - (int64_t)mFirstTimeStamp) * 1000000ll / mClockRate;
+ int64_t scheduledTimeUs = ((int32_t)diff) * 1000000ll / mClockRate;
int64_t elapsedTimeUs = arrivalTimeUs - mFirstArrivalTimeUs;
int64_t correctionTimeUs = elapsedTimeUs - scheduledTimeUs; // additional propagation delay;
mBaseJitterUs = (mBaseJitterUs * 15 + correctionTimeUs) / 16;
@@ -53,18 +52,13 @@
(long long)mBaseJitterUs, (long long)correctionTimeUs);
}
-void JitterCalc::putInterArrivalData(int64_t rtpTime, int64_t arrivalTimeUs) {
- const int64_t UINT32_MSB = 0x80000000;
- int64_t tempRtpTime = rtpTime;
- int64_t tempLastTimeStamp = mLastTimeStamp;
-
- // A RTP time wraps around after UINT32_MAX. We must consider this case.
- int64_t overflowMask = (mLastTimeStamp ^ rtpTime) & UINT32_MSB;
- tempRtpTime |= ((overflowMask & ~rtpTime) << 1);
- tempLastTimeStamp |= ((overflowMask & ~mLastTimeStamp) << 1);
+void JitterCalc::putInterArrivalData(uint32_t rtpTime, int64_t arrivalTimeUs) {
+ // A RTP time wraps around after UINT32_MAX. Overflow can present.
+ uint32_t diff = 0;
+ __builtin_usub_overflow(rtpTime, mLastTimeStamp, &diff);
// 6.4.1 of RFC3550 defines this interarrival jitter value.
- int64_t diffTimeStampUs = abs(tempRtpTime - tempLastTimeStamp) * 1000000ll / mClockRate;
+ int64_t diffTimeStampUs = abs((int32_t)diff) * 1000000ll / mClockRate;
int64_t diffArrivalUs = arrivalTimeUs - mLastArrivalTimeUs; // Can't be minus
ALOGV("diffTimeStampUs %lld \t\t diffArrivalUs %lld",
(long long)diffTimeStampUs, (long long)diffArrivalUs);
@@ -72,7 +66,7 @@
int64_t varianceUs = diffArrivalUs - diffTimeStampUs;
mInterArrivalJitterUs = (mInterArrivalJitterUs * 15 + abs(varianceUs)) / 16;
- mLastTimeStamp = (uint32_t)rtpTime;
+ mLastTimeStamp = rtpTime;
mLastArrivalTimeUs = arrivalTimeUs;
}
diff --git a/media/libstagefright/rtsp/JitterCalculator.h b/media/libstagefright/rtsp/JitterCalculator.h
index ff36f1f..4f3b761 100644
--- a/media/libstagefright/rtsp/JitterCalculator.h
+++ b/media/libstagefright/rtsp/JitterCalculator.h
@@ -40,8 +40,8 @@
JitterCalc(int32_t clockRate);
void init(uint32_t rtpTime, int64_t arrivalTimeUs, int32_t base, int32_t inter);
- void putInterArrivalData(int64_t rtpTime, int64_t arrivalTime);
- void putBaseData(int64_t rtpTime, int64_t arrivalTimeUs);
+ void putInterArrivalData(uint32_t rtpTime, int64_t arrivalTime);
+ void putBaseData(uint32_t rtpTime, int64_t arrivalTimeUs);
int32_t getBaseJitterMs();
int32_t getInterArrivalJitterMs();
};
diff --git a/media/ndk/NdkMediaFormat.cpp b/media/ndk/NdkMediaFormat.cpp
index c1793ce..51f6c78 100644
--- a/media/ndk/NdkMediaFormat.cpp
+++ b/media/ndk/NdkMediaFormat.cpp
@@ -200,8 +200,11 @@
AString tmp;
if (mData->mFormat->findString(name, &tmp)) {
String8 ret(tmp.c_str());
- mData->mStringCache.add(String8(name), ret);
- *out = ret.string();
+ ssize_t i = mData->mStringCache.add(String8(name), ret);
+ if (i < 0) {
+ return false;
+ }
+ *out = mData->mStringCache.valueAt(i).string();
return true;
}
return false;
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index b9cdab8..746d875 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -5089,7 +5089,7 @@
break;
case TrackBase::IDLE:
default:
- LOG_ALWAYS_FATAL("unexpected track state %d", track->mState);
+ LOG_ALWAYS_FATAL("unexpected track state %d", (int)track->mState);
}
if (isActive) {
@@ -5148,7 +5148,7 @@
// TODO Remove the ALOGW when this theory is confirmed.
ALOGW("fast track %d should have been active; "
"mState=%d, mTrackMask=%#x, recentUnderruns=%u, isShared=%d",
- j, track->mState, state->mTrackMask, recentUnderruns,
+ j, (int)track->mState, state->mTrackMask, recentUnderruns,
track->sharedBuffer() != 0);
// Since the FastMixer state already has the track inactive, do nothing here.
}
@@ -8041,7 +8041,7 @@
ALOGV("active record track PAUSING -> ACTIVE");
recordTrack->mState = TrackBase::ACTIVE;
} else {
- ALOGV("active record track state %d", recordTrack->mState);
+ ALOGV("active record track state %d", (int)recordTrack->mState);
}
return status;
}
@@ -8067,7 +8067,7 @@
}
if (recordTrack->mState != TrackBase::STARTING_1) {
ALOGW("%s(%d): unsynchronized mState:%d change",
- __func__, recordTrack->id(), recordTrack->mState);
+ __func__, recordTrack->id(), (int)recordTrack->mState);
// Someone else has changed state, let them take over,
// leave mState in the new state.
recordTrack->clearSyncStartEvent();
diff --git a/services/audioflinger/TrackBase.h b/services/audioflinger/TrackBase.h
index 92f129c..5311fe2 100644
--- a/services/audioflinger/TrackBase.h
+++ b/services/audioflinger/TrackBase.h
@@ -23,7 +23,7 @@
class TrackBase : public ExtendedAudioBufferProvider, public RefBase {
public:
- enum track_state {
+ enum track_state : int32_t {
IDLE,
FLUSHED, // for PlaybackTracks only
STOPPED,
@@ -271,6 +271,7 @@
void releaseCblk() {
if (mCblk != nullptr) {
+ mState.clear();
mCblk->~audio_track_cblk_t(); // destroy our shared-structure.
if (mClient == 0) {
free(mCblk);
@@ -355,7 +356,7 @@
// except for OutputTrack when it is in local memory
size_t mBufferSize; // size of mBuffer in bytes
// we don't really need a lock for these
- track_state mState;
+ MirroredVariable<track_state> mState;
const audio_attributes_t mAttr;
const uint32_t mSampleRate; // initial sample rate only; for tracks which
// support dynamic rates, the current value is in control block
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index d2a30b1..e0c5fa5 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -234,7 +234,11 @@
#ifdef TEE_SINK
mTee.set(sampleRate, mChannelCount, format, NBAIO_Tee::TEE_FLAG_TRACK);
#endif
-
+ // mState is mirrored for the client to read.
+ mState.setMirror(&mCblk->mState);
+ // ensure our state matches up until we consolidate the enumeration.
+ static_assert(CBLK_STATE_IDLE == IDLE);
+ static_assert(CBLK_STATE_PAUSING == PAUSING);
}
}
@@ -933,7 +937,7 @@
buffer->raw = buf.mRaw;
if (buf.mFrameCount == 0 && !isStopping() && !isStopped() && !isPaused() && !isOffloaded()) {
ALOGV("%s(%d): underrun, framesReady(%zu) < framesDesired(%zd), state: %d",
- __func__, mId, buf.mFrameCount, desiredFrames, mState);
+ __func__, mId, buf.mFrameCount, desiredFrames, (int)mState);
mAudioTrackServerProxy->tallyUnderrunFrames(desiredFrames);
} else {
mAudioTrackServerProxy->tallyUnderrunFrames(0);
@@ -1590,7 +1594,7 @@
(mState == STOPPED)))) {
ALOGW("%s(%d): in invalid state %d on session %d %s mode, framesReady %zu",
__func__, mId,
- mState, mSessionId, (mSharedBuffer != 0) ? "static" : "stream", framesReady());
+ (int)mState, mSessionId, (mSharedBuffer != 0) ? "static" : "stream", framesReady());
event->cancel();
return INVALID_OPERATION;
}
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index cc2d8e8..cb9d700 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -6548,7 +6548,7 @@
outputDesc->setVolume(
volumeDb, volumeSource, curves.getStreamTypes(), deviceTypes, delayMs, force);
- if (isVoiceVolSrc || isBtScoVolSrc) {
+ if (outputDesc == mPrimaryOutput && (isVoiceVolSrc || isBtScoVolSrc)) {
float voiceVolume;
// Force voice volume to max or mute for Bluetooth SCO as other attenuations are managed by the headset
if (isVoiceVolSrc) {
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index d0d3a9d..04b5604 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -2054,6 +2054,11 @@
id.string());
errorCode = ERROR_ILLEGAL_ARGUMENT;
break;
+ case -EBUSY:
+ msg = String8::format("Camera \"%s\" is in use",
+ id.string());
+ errorCode = ERROR_CAMERA_IN_USE;
+ break;
default:
msg = String8::format(
"Setting torch mode of camera \"%s\" to %d failed: %s (%d)",
diff --git a/services/camera/libcameraservice/api1/client2/Parameters.h b/services/camera/libcameraservice/api1/client2/Parameters.h
index e2f8d011..1b2ceda 100644
--- a/services/camera/libcameraservice/api1/client2/Parameters.h
+++ b/services/camera/libcameraservice/api1/client2/Parameters.h
@@ -205,7 +205,7 @@
static const int MAX_INITIAL_PREVIEW_WIDTH = 1920;
static const int MAX_INITIAL_PREVIEW_HEIGHT = 1080;
// Aspect ratio tolerance
- static const CONSTEXPR float ASPECT_RATIO_TOLERANCE = 0.001;
+ static const CONSTEXPR float ASPECT_RATIO_TOLERANCE = 0.01;
// Threshold for slow jpeg mode
static const int64_t kSlowJpegModeThreshold = 33400000LL; // 33.4 ms
// Margin for checking FPS
diff --git a/services/camera/libcameraservice/device3/BufferUtils.h b/services/camera/libcameraservice/device3/BufferUtils.h
index 1e1cd60..03112ec 100644
--- a/services/camera/libcameraservice/device3/BufferUtils.h
+++ b/services/camera/libcameraservice/device3/BufferUtils.h
@@ -104,7 +104,7 @@
// Return the removed buffer ID if input cache is found.
// Otherwise return BUFFER_ID_NO_BUFFER
- uint64_t removeOneBufferCache(int streamId, const native_handle_t* handle);
+ uint64_t removeOneBufferCache(int streamId, const native_handle_t* handle) override;
// Clear all caches for input stream, but do not remove the stream
// Removed buffers' ID are returned
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index fd645c7..87c1c75 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -4132,6 +4132,11 @@
return mBufferRecords.getBufferId(buf, streamId);
}
+uint64_t Camera3Device::HalInterface::removeOneBufferCache(int streamId,
+ const native_handle_t* handle) {
+ return mBufferRecords.removeOneBufferCache(streamId, handle);
+}
+
void Camera3Device::HalInterface::onBufferFreed(
int streamId, const native_handle_t* handle) {
uint32_t bufferId = mBufferRecords.removeOneBufferCache(streamId, handle);
@@ -4778,6 +4783,26 @@
return submitRequestSuccess;
}
+status_t Camera3Device::removeFwkOnlyRegionKeys(CameraMetadata *request) {
+ static const std::array<uint32_t, 4> kFwkOnlyRegionKeys = {ANDROID_CONTROL_AF_REGIONS_SET,
+ ANDROID_CONTROL_AE_REGIONS_SET, ANDROID_CONTROL_AWB_REGIONS_SET,
+ ANDROID_SCALER_CROP_REGION_SET};
+ if (request == nullptr) {
+ ALOGE("%s request metadata nullptr", __FUNCTION__);
+ return BAD_VALUE;
+ }
+ status_t res = OK;
+ for (const auto &key : kFwkOnlyRegionKeys) {
+ if (request->exists(key)) {
+ res = request->erase(key);
+ if (res != OK) {
+ return res;
+ }
+ }
+ }
+ return OK;
+}
+
status_t Camera3Device::RequestThread::prepareHalRequests() {
ATRACE_CALL();
@@ -4837,6 +4862,12 @@
it != captureRequest->mSettingsList.end(); it++) {
if (parent->mUHRCropAndMeteringRegionMappers.find(it->cameraId) ==
parent->mUHRCropAndMeteringRegionMappers.end()) {
+ if (removeFwkOnlyRegionKeys(&(it->metadata)) != OK) {
+ SET_ERR("RequestThread: Unable to remove fwk-only keys from request"
+ "%d: %s (%d)", halRequest->frame_number, strerror(-res),
+ res);
+ return INVALID_OPERATION;
+ }
continue;
}
@@ -4851,6 +4882,12 @@
return INVALID_OPERATION;
}
captureRequest->mUHRCropAndMeteringRegionsUpdated = true;
+ if (removeFwkOnlyRegionKeys(&(it->metadata)) != OK) {
+ SET_ERR("RequestThread: Unable to remove fwk-only keys from request"
+ "%d: %s (%d)", halRequest->frame_number, strerror(-res),
+ res);
+ return INVALID_OPERATION;
+ }
}
}
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index 39714f0..df941b2 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -297,6 +297,7 @@
private:
status_t disconnectImpl();
+ static status_t removeFwkOnlyRegionKeys(CameraMetadata *request);
// internal typedefs
using RequestMetadataQueue = hardware::MessageQueue<uint8_t, hardware::kSynchronizedReadWrite>;
@@ -412,6 +413,8 @@
std::pair<bool, uint64_t> getBufferId(
const buffer_handle_t& buf, int streamId) override;
+ uint64_t removeOneBufferCache(int streamId, const native_handle_t* handle) override;
+
status_t popInflightBuffer(int32_t frameNumber, int32_t streamId,
/*out*/ buffer_handle_t **buffer) override;
diff --git a/services/camera/libcameraservice/device3/Camera3OutputInterface.h b/services/camera/libcameraservice/device3/Camera3OutputInterface.h
index 8817833..40eef1d 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputInterface.h
+++ b/services/camera/libcameraservice/device3/Camera3OutputInterface.h
@@ -50,6 +50,10 @@
// return pair of (newlySeenBuffer?, bufferId)
virtual std::pair<bool, uint64_t> getBufferId(const buffer_handle_t& buf, int streamId) = 0;
+ // Return the removed buffer ID if input cache is found.
+ // Otherwise return BUFFER_ID_NO_BUFFER
+ virtual uint64_t removeOneBufferCache(int streamId, const native_handle_t* handle) = 0;
+
// Find a buffer_handle_t based on frame number and stream ID
virtual status_t popInflightBuffer(int32_t frameNumber, int32_t streamId,
/*out*/ buffer_handle_t **buffer) = 0;
diff --git a/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp b/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp
index 9f225d0..5a97f4b 100644
--- a/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp
+++ b/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp
@@ -1305,6 +1305,7 @@
hardware::hidl_vec<StreamBuffer> tmpRetBuffers(numBuffersRequested);
bool currentReqSucceeds = true;
std::vector<camera_stream_buffer_t> streamBuffers(numBuffersRequested);
+ std::vector<buffer_handle_t> newBuffers;
size_t numAllocatedBuffers = 0;
size_t numPushedInflightBuffers = 0;
for (size_t b = 0; b < numBuffersRequested; b++) {
@@ -1344,6 +1345,9 @@
hBuf.buffer = (isNewBuffer) ? *buffer : nullptr;
hBuf.status = BufferStatus::OK;
hBuf.releaseFence = nullptr;
+ if (isNewBuffer) {
+ newBuffers.push_back(*buffer);
+ }
native_handle_t *acquireFence = nullptr;
if (sb.acquire_fence != -1) {
@@ -1386,6 +1390,9 @@
returnOutputBuffers(states.useHalBufManager, /*listener*/nullptr,
streamBuffers.data(), numAllocatedBuffers, 0, /*requested*/false,
/*requestTimeNs*/0, states.sessionStatsBuilder);
+ for (auto buf : newBuffers) {
+ states.bufferRecordsIntf.removeOneBufferCache(streamId, buf);
+ }
}
}
diff --git a/services/camera/libcameraservice/tests/ExifUtilsTest.cpp b/services/camera/libcameraservice/tests/ExifUtilsTest.cpp
new file mode 100644
index 0000000..3de4bf2
--- /dev/null
+++ b/services/camera/libcameraservice/tests/ExifUtilsTest.cpp
@@ -0,0 +1,55 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+//#define LOG_NDEBUG 0
+#define LOG_TAG "ExifUtilsTest"
+
+#include <camera/CameraMetadata.h>
+#include "../utils/ExifUtils.h"
+#include <gtest/gtest.h>
+
+using android::camera3::ExifUtils;
+using android::camera3::ExifOrientation;
+using android::CameraMetadata;
+
+uint32_t kImageWidth = 1920;
+uint32_t kImageHeight = 1440;
+ExifOrientation kExifOrientation = ExifOrientation::ORIENTATION_0_DEGREES;
+
+// Test that setFromMetadata works correctly, without errors.
+TEST(ExifUtilsTest, SetFromMetadataTest) {
+ std::unique_ptr<ExifUtils> utils(ExifUtils::create());
+ uint8_t invalidSensorPixelMode = 2;
+ uint8_t validSensorPixelMode = ANDROID_SENSOR_PIXEL_MODE_DEFAULT;
+ CameraMetadata metadata;
+ // Empty staticInfo
+ CameraMetadata staticInfo;
+ ASSERT_TRUE(utils->initializeEmpty());
+ ASSERT_TRUE(
+ metadata.update(ANDROID_SENSOR_PIXEL_MODE, &invalidSensorPixelMode, 1) == android::OK);
+ ASSERT_FALSE(utils->setFromMetadata(metadata, staticInfo, kImageWidth, kImageHeight));
+ ASSERT_TRUE(
+ metadata.update(ANDROID_SENSOR_PIXEL_MODE, &validSensorPixelMode, 1) == android::OK);
+ ASSERT_TRUE(utils->setFromMetadata(metadata, staticInfo, kImageWidth, kImageHeight));
+ ASSERT_TRUE(utils->setImageWidth(kImageWidth));
+ ASSERT_TRUE(utils->setImageHeight(kImageHeight));
+ ASSERT_TRUE(utils->setOrientationValue(kExifOrientation));
+ ASSERT_TRUE(utils->generateApp1());
+ const uint8_t* exifBuffer = utils->getApp1Buffer();
+ ASSERT_NE(exifBuffer, nullptr);
+ size_t exifBufferSize = utils->getApp1Length();
+ ASSERT_TRUE(exifBufferSize != 0);
+}
diff --git a/services/camera/libcameraservice/utils/ExifUtils.cpp b/services/camera/libcameraservice/utils/ExifUtils.cpp
index 485705c..21f02db 100644
--- a/services/camera/libcameraservice/utils/ExifUtils.cpp
+++ b/services/camera/libcameraservice/utils/ExifUtils.cpp
@@ -920,7 +920,7 @@
camera_metadata_ro_entry sensorPixelModeEntry = metadata.find(ANDROID_SENSOR_PIXEL_MODE);
if (sensorPixelModeEntry.count != 0) {
sensorPixelMode = sensorPixelModeEntry.data.u8[0];
- if (sensorPixelMode != ANDROID_SENSOR_PIXEL_MODE_DEFAULT ||
+ if (sensorPixelMode != ANDROID_SENSOR_PIXEL_MODE_DEFAULT &&
sensorPixelMode != ANDROID_SENSOR_PIXEL_MODE_MAXIMUM_RESOLUTION) {
ALOGE("%s: Request sensor pixel mode is not one of the valid values %d",
__FUNCTION__, sensorPixelMode);