Merge changes If6110034,Ia2ac55bd into main
* changes:
C2SoftGav1Dec: Add support for 12-bit decoding
C2SoftGav1Dec: Disallow bitdepth > 10
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 0db3eec..514d622 100644
--- a/media/codec2/sfplugin/CCodecBufferChannel.cpp
+++ b/media/codec2/sfplugin/CCodecBufferChannel.cpp
@@ -1131,7 +1131,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/audio_aidl_legacy_conversion_tests.cpp b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
index 9a46b20..7f55e48 100644
--- a/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
+++ b/media/libaudioclient/tests/audio_aidl_legacy_conversion_tests.cpp
@@ -43,7 +43,9 @@
using media::audio::common::AudioGain;
using media::audio::common::AudioGainConfig;
using media::audio::common::AudioGainMode;
+using media::audio::common::AudioInputFlags;
using media::audio::common::AudioIoFlags;
+using media::audio::common::AudioOutputFlags;
using media::audio::common::AudioPortDeviceExt;
using media::audio::common::AudioProfile;
using media::audio::common::AudioStandard;
@@ -831,3 +833,27 @@
EXPECT_EQ(MicrophoneDynamicInfo::ChannelMapping::UNUSED, aidl.dynamic.channelMapping[2]);
EXPECT_EQ(MicrophoneDynamicInfo::ChannelMapping::PROCESSED, aidl.dynamic.channelMapping[3]);
}
+
+TEST(AudioInputFlags, Aidl2Legacy2Aidl) {
+ for (auto flag : enum_range<AudioInputFlags>()) {
+ int32_t aidlMask = 1 << static_cast<int32_t>(flag);
+ auto convMask = aidl2legacy_int32_t_audio_input_flags_t_mask(aidlMask);
+ ASSERT_TRUE(convMask.ok());
+ ASSERT_EQ(1, __builtin_popcount(convMask.value()));
+ auto convFlag = legacy2aidl_audio_input_flags_t_AudioInputFlags(convMask.value());
+ ASSERT_TRUE(convFlag.ok());
+ EXPECT_EQ(flag, convFlag.value());
+ }
+}
+
+TEST(AudioOutputFlags, Aidl2Legacy2Aidl) {
+ for (auto flag : enum_range<AudioOutputFlags>()) {
+ int32_t aidlMask = 1 << static_cast<int32_t>(flag);
+ auto convMask = aidl2legacy_int32_t_audio_output_flags_t_mask(aidlMask);
+ ASSERT_TRUE(convMask.ok());
+ ASSERT_EQ(1, __builtin_popcount(convMask.value()));
+ auto convFlag = legacy2aidl_audio_output_flags_t_AudioOutputFlags(convMask.value());
+ ASSERT_TRUE(convFlag.ok());
+ EXPECT_EQ(flag, convFlag.value());
+ }
+}
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/downmix/aidl/DownmixContext.cpp b/media/libeffects/downmix/aidl/DownmixContext.cpp
index ac893d8..0e76d1d 100644
--- a/media/libeffects/downmix/aidl/DownmixContext.cpp
+++ b/media/libeffects/downmix/aidl/DownmixContext.cpp
@@ -66,8 +66,8 @@
LOG(DEBUG) << __func__ << " in " << in << " out " << out << " sample " << samples;
IEffect::Status status = {EX_ILLEGAL_ARGUMENT, 0, 0};
- if (in == nullptr || out == nullptr || getInputFrameSize() != getOutputFrameSize() ||
- getInputFrameSize() == 0) {
+ if (in == nullptr || out == nullptr ||
+ getCommon().input.frameCount != getCommon().output.frameCount || getInputFrameSize() == 0) {
return status;
}
@@ -112,7 +112,7 @@
void DownmixContext::init_params(const Parameter::Common& common) {
// when configuring the effect, do not allow a blank or unsupported channel mask
AudioChannelLayout channelMask = common.input.base.channelMask;
- if (isChannelMaskValid(channelMask)) {
+ if (!isChannelMaskValid(channelMask)) {
LOG(ERROR) << "Downmix_Configure error: input channel mask " << channelMask.toString()
<< " not supported";
} else {
@@ -123,7 +123,7 @@
}
bool DownmixContext::isChannelMaskValid(AudioChannelLayout channelMask) {
- if (channelMask.getTag() == AudioChannelLayout::layoutMask) return false;
+ if (channelMask.getTag() != AudioChannelLayout::layoutMask) return false;
int chMask = channelMask.get<AudioChannelLayout::layoutMask>();
// check against unsupported channels (up to FCC_26)
constexpr uint32_t MAXIMUM_CHANNEL_MASK = AudioChannelLayout::LAYOUT_22POINT2 |
diff --git a/media/libeffects/dynamicsproc/aidl/DynamicsProcessing.cpp b/media/libeffects/dynamicsproc/aidl/DynamicsProcessing.cpp
index 63cb48d..85ea53a 100644
--- a/media/libeffects/dynamicsproc/aidl/DynamicsProcessing.cpp
+++ b/media/libeffects/dynamicsproc/aidl/DynamicsProcessing.cpp
@@ -105,14 +105,14 @@
DynamicsProcessing::EqBandConfig({.channel = 0,
.band = 0,
.enable = false,
- .cutoffFrequencyHz = 20,
+ .cutoffFrequencyHz = 0,
.gainDb = -200});
static const DynamicsProcessing::EqBandConfig kEqBandConfigMax =
DynamicsProcessing::EqBandConfig({.channel = std::numeric_limits<int>::max(),
.band = std::numeric_limits<int>::max(),
.enable = true,
- .cutoffFrequencyHz = 20000.1,
+ .cutoffFrequencyHz = 192000,
.gainDb = 200});
static const Range::DynamicsProcessingRange kPreEqBandConfigRange = {
@@ -129,7 +129,7 @@
{.channel = 0,
.band = 0,
.enable = false,
- .cutoffFrequencyHz = 20,
+ .cutoffFrequencyHz = 0,
.attackTimeMs = 0,
.releaseTimeMs = 0,
.ratio = 1,
@@ -144,7 +144,7 @@
{.channel = std::numeric_limits<int>::max(),
.band = std::numeric_limits<int>::max(),
.enable = true,
- .cutoffFrequencyHz = 20000.1,
+ .cutoffFrequencyHz = 192000,
.attackTimeMs = 60000,
.releaseTimeMs = 60000,
.ratio = 50,
@@ -443,7 +443,7 @@
IEffect::Status DynamicsProcessingImpl::effectProcessImpl(float* in, float* out, int samples) {
IEffect::Status status = {EX_NULL_POINTER, 0, 0};
RETURN_VALUE_IF(!mContext, status, "nullContext");
- return mContext->lvmProcess(in, out, samples);
+ return mContext->dpeProcess(in, out, samples);
}
} // namespace aidl::android::hardware::audio::effect
diff --git a/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.cpp b/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.cpp
index 57c873b..e5e5368 100644
--- a/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.cpp
+++ b/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.cpp
@@ -19,6 +19,7 @@
#include "DynamicsProcessingContext.h"
#include "DynamicsProcessing.h"
+#include <audio_utils/power.h>
#include <sys/param.h>
#include <functional>
#include <unordered_set>
@@ -68,6 +69,23 @@
return RetCode::SUCCESS;
}
+RetCode DynamicsProcessingContext::setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) {
+ std::lock_guard lg(mMutex);
+ dp_fx::DPChannel* leftChannel = mDpFreq->getChannel(0);
+ dp_fx::DPChannel* rightChannel = mDpFreq->getChannel(1);
+ if (leftChannel != nullptr) {
+ leftChannel->setOutputGain(audio_utils_power_from_amplitude(volumeStereo.left));
+ }
+ if (rightChannel != nullptr) {
+ rightChannel->setOutputGain(audio_utils_power_from_amplitude(volumeStereo.right));
+ }
+ return RetCode::SUCCESS;
+}
+
+Parameter::VolumeStereo DynamicsProcessingContext::getVolumeStereo() {
+ return {1.0f, 1.0f};
+}
+
void DynamicsProcessingContext::dpSetFreqDomainVariant_l(
const DynamicsProcessing::EngineArchitecture& engine) {
mDpFreq.reset(new dp_fx::DPFrequency());
@@ -273,7 +291,7 @@
return ret;
}
-IEffect::Status DynamicsProcessingContext::lvmProcess(float* in, float* out, int samples) {
+IEffect::Status DynamicsProcessingContext::dpeProcess(float* in, float* out, int samples) {
LOG(DEBUG) << __func__ << " in " << in << " out " << out << " sample " << samples;
IEffect::Status status = {EX_NULL_POINTER, 0, 0};
@@ -460,6 +478,11 @@
RetCode ret = RetCode::SUCCESS;
std::unordered_set<int> channelSet;
+ if (!stageInUse) {
+ LOG(WARNING) << __func__ << " not in use " << ::android::internal::ToString(channels);
+ return RetCode::SUCCESS;
+ }
+
RETURN_VALUE_IF(!stageInUse, RetCode::ERROR_ILLEGAL_PARAMETER, "stageNotInUse");
for (auto& it : channels) {
if (0 != channelSet.count(it.channel)) {
diff --git a/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.h b/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.h
index b8539f6..ced7f19 100644
--- a/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.h
+++ b/media/libeffects/dynamicsproc/aidl/DynamicsProcessingContext.h
@@ -45,6 +45,8 @@
// override EffectContext::setCommon to update mChannelCount
RetCode setCommon(const Parameter::Common& common) override;
+ RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) override;
+ Parameter::VolumeStereo getVolumeStereo() override;
RetCode setEngineArchitecture(const DynamicsProcessing::EngineArchitecture& engineArchitecture);
RetCode setPreEq(const std::vector<DynamicsProcessing::ChannelConfig>& eqChannels);
@@ -66,7 +68,7 @@
std::vector<DynamicsProcessing::LimiterConfig> getLimiter();
std::vector<DynamicsProcessing::InputGain> getInputGain();
- IEffect::Status lvmProcess(float* in, float* out, int samples);
+ IEffect::Status dpeProcess(float* in, float* out, int samples);
private:
static constexpr float kPreferredProcessingDurationMs = 10.0f;
diff --git a/media/libeffects/lvm/wrapper/Aidl/BundleContext.h b/media/libeffects/lvm/wrapper/Aidl/BundleContext.h
index 62bb6e4..3f31b8b 100644
--- a/media/libeffects/lvm/wrapper/Aidl/BundleContext.h
+++ b/media/libeffects/lvm/wrapper/Aidl/BundleContext.h
@@ -98,7 +98,7 @@
const Virtualizer::SpeakerAnglesPayload payload);
RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) override;
- Parameter::VolumeStereo getVolumeStereo() override { return mVolumeStereo; }
+ Parameter::VolumeStereo getVolumeStereo() override { return {1.0f, 1.0f}; }
IEffect::Status lvmProcess(float* in, float* out, int samples);
diff --git a/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h b/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h
index 9bb0b1a..d11a081 100644
--- a/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h
+++ b/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h
@@ -81,7 +81,12 @@
bool getEnvironmentalReverbBypass() const { return mBypass; }
RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) override;
- Parameter::VolumeStereo getVolumeStereo() override { return mVolumeStereo; }
+ Parameter::VolumeStereo getVolumeStereo() override {
+ if (isAuxiliary()) {
+ return mVolumeStereo;
+ }
+ return {1.0f, 1.0f};
+ }
RetCode setReflectionsDelay(int delay) {
mReflectionsDelayMs = delay;
diff --git a/media/libeffects/visualizer/aidl/Visualizer.cpp b/media/libeffects/visualizer/aidl/Visualizer.cpp
index 53bfb41..0303842 100644
--- a/media/libeffects/visualizer/aidl/Visualizer.cpp
+++ b/media/libeffects/visualizer/aidl/Visualizer.cpp
@@ -73,7 +73,7 @@
.proxy = std::nullopt},
.flags = {.type = Flags::Type::INSERT,
.insert = Flags::Insert::LAST,
- .volume = Flags::Volume::CTRL},
+ .volume = Flags::Volume::NONE},
.name = VisualizerImpl::kEffectName,
.implementor = "The Android Open Source Project"},
.capability = VisualizerImpl::kCapability};
diff --git a/media/libeffects/visualizer/aidl/VisualizerContext.cpp b/media/libeffects/visualizer/aidl/VisualizerContext.cpp
index 5d0d08d..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.
@@ -224,21 +224,21 @@
}
int32_t capturePoint;
- //capturePoint = (int32_t)mCaptureIdx - deltaSamples;
__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;
}
- result.insert(result.end(), &mCaptureBuf[kMaxCaptureBufSize + capturePoint],
- &mCaptureBuf[kMaxCaptureBufSize + capturePoint + size]);
- mCaptureSamples -= 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 + mCaptureSamples]);
+ 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/libmediaplayerservice/fuzzer/Android.bp b/media/libmediaplayerservice/fuzzer/Android.bp
index c3d6c89..b511372 100644
--- a/media/libmediaplayerservice/fuzzer/Android.bp
+++ b/media/libmediaplayerservice/fuzzer/Android.bp
@@ -174,6 +174,7 @@
"libnetd_client",
"libpowermanager",
"libstagefright_httplive",
+ "libaudiohal@7.0",
],
}
diff --git a/media/libstagefright/VideoRenderQualityTracker.cpp b/media/libstagefright/VideoRenderQualityTracker.cpp
index 3bd236a..eb9ac0f 100644
--- a/media/libstagefright/VideoRenderQualityTracker.cpp
+++ b/media/libstagefright/VideoRenderQualityTracker.cpp
@@ -28,6 +28,7 @@
#include <cmath>
#include <stdio.h>
#include <sys/time.h>
+#include <sys/wait.h>
#include <android-base/macros.h>
#include <android-base/parsebool.h>
@@ -161,7 +162,6 @@
getFlag(traceTriggerEnabled, "trace_trigger_enabled");
getFlag(traceTriggerThrottleMs, "trace_trigger_throttle_ms");
getFlag(traceMinFreezeDurationMs, "trace_minimum_freeze_duration_ms");
- getFlag(traceMaxFreezeDurationMs, "trace_maximum_freeze_duration_ms");
#undef getFlag
return c;
}
@@ -208,7 +208,6 @@
"ro.build.type", "user") != "user"; // Enabled for non-user builds for debugging.
traceTriggerThrottleMs = 5 * 60 * 1000; // 5 mins.
traceMinFreezeDurationMs = 400;
- traceMaxFreezeDurationMs = 1500;
}
VideoRenderQualityTracker::VideoRenderQualityTracker()
@@ -300,15 +299,7 @@
int64_t actualRenderTimeUs = actualRenderTimeNs / 1000;
if (mLastRenderTimeUs != -1) {
- int64_t frameRenderDurationMs = (actualRenderTimeUs - mLastRenderTimeUs) / 1000;
- mRenderDurationMs += frameRenderDurationMs;
- if (mConfiguration.traceTriggerEnabled
- // Threshold for visible video freeze.
- && frameRenderDurationMs >= mConfiguration.traceMinFreezeDurationMs
- // Threshold for removing long render durations which could be video pause.
- && frameRenderDurationMs < mConfiguration.traceMaxFreezeDurationMs) {
- triggerTraceWithThrottle(mTraceTriggerFn, mConfiguration, actualRenderTimeUs);
- }
+ mRenderDurationMs += (actualRenderTimeUs - mLastRenderTimeUs) / 1000;
}
// Now that a frame has been rendered, the previously skipped frames can be processed as skipped
@@ -549,7 +540,7 @@
}
if (!isLikelyCatchingUpAfterPause) {
processFreeze(actualRenderTimeUs, mLastRenderTimeUs, mLastFreezeEndTimeUs, mFreezeEvent,
- mMetrics, mConfiguration);
+ mMetrics, mConfiguration, mTraceTriggerFn);
mLastFreezeEndTimeUs = actualRenderTimeUs;
}
}
@@ -575,8 +566,8 @@
void VideoRenderQualityTracker::processFreeze(int64_t actualRenderTimeUs, int64_t lastRenderTimeUs,
int64_t lastFreezeEndTimeUs, FreezeEvent &e,
- VideoRenderQualityMetrics &m,
- const Configuration &c) {
+ VideoRenderQualityMetrics &m, const Configuration &c,
+ const TraceTriggerFn traceTriggerFn) {
int32_t durationMs = int32_t((actualRenderTimeUs - lastRenderTimeUs) / 1000);
m.freezeDurationMsHistogram.insert(durationMs);
int32_t distanceMs = -1;
@@ -612,6 +603,11 @@
e.details.distanceMs.push_back(distanceMs); // -1 for first detail in the first event
}
}
+
+ if (c.traceTriggerEnabled && durationMs >= c.traceMinFreezeDurationMs) {
+ ALOGI("Video freezed %lld ms", (long long) durationMs);
+ triggerTraceWithThrottle(traceTriggerFn, c, actualRenderTimeUs);
+ }
}
void VideoRenderQualityTracker::maybeCaptureFreezeEvent(int64_t actualRenderTimeUs,
@@ -818,36 +814,48 @@
static int64_t lastTriggerUs = -1;
static Mutex updateLastTriggerLock;
- Mutex::Autolock autoLock(updateLastTriggerLock);
- if (lastTriggerUs != -1) {
- int32_t sinceLastTriggerMs = int32_t((triggerTimeUs - lastTriggerUs) / 1000);
- // Throttle the trace trigger calls to reduce continuous PID fork calls in a short time
- // to impact device performance, and reduce spamming trace reports.
- if (sinceLastTriggerMs < c.traceTriggerThrottleMs) {
- ALOGI("Not triggering trace - not enough time since last trigger");
- return;
+ {
+ Mutex::Autolock autoLock(updateLastTriggerLock);
+ if (lastTriggerUs != -1) {
+ int32_t sinceLastTriggerMs = int32_t((triggerTimeUs - lastTriggerUs) / 1000);
+ // Throttle the trace trigger calls to reduce continuous PID fork calls in a short time
+ // to impact device performance, and reduce spamming trace reports.
+ if (sinceLastTriggerMs < c.traceTriggerThrottleMs) {
+ ALOGI("Not triggering trace - not enough time since last trigger");
+ return;
+ }
}
+ lastTriggerUs = triggerTimeUs;
}
- lastTriggerUs = triggerTimeUs;
+
(*traceTriggerFn)();
}
void VideoRenderQualityTracker::triggerTrace() {
// Trigger perfetto to stop always-on-tracing (AOT) to collect trace into a file for video
// freeze event, the collected trace categories are configured by AOT.
- const char* args[] = {"/system/bin/trigger_perfetto", "com.android.codec-video-freeze", NULL};
+ static const char* args[] = {"/system/bin/trigger_perfetto",
+ "com.android.codec-video-freeze", NULL};
+
pid_t pid = fork();
if (pid < 0) {
ALOGI("Failed to fork for triggering trace");
- return;
- }
- if (pid == 0) {
- // child process.
+ } else if (pid == 0) {
+ // Child process.
+ ALOGI("Trigger trace %s", args[1]);
execvp(args[0], const_cast<char**>(args));
ALOGW("Failed to trigger trace %s", args[1]);
_exit(1);
+ } else {
+ // Parent process.
+ int status;
+ // Wait for the child process (pid) gets terminated, and allow the system to release
+ // the resource associated with the child. Or the child process will remain in a
+ // zombie state and get killed by llkd to cause foreground app crash.
+ if (waitpid(pid, &status, 0) < 0) {
+ ALOGW("Failed to waitpid for triggering trace");
+ }
}
- ALOGI("Triggered trace %s", args[1]);
}
} // namespace android
diff --git a/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h b/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h
index d58dfad..7139deb 100644
--- a/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h
+++ b/media/libstagefright/include/media/stagefright/VideoRenderQualityTracker.h
@@ -220,11 +220,6 @@
//
// The minimum frame render duration to recognize video freeze event to collect trace.
int32_t traceMinFreezeDurationMs;
- //
- // The maximum frame render duration to recognize video freeze event. A frame render
- // duration that is larger than the max duration would not trigger trace collection for
- // video freeze because it's highly possible a video pause.
- int32_t traceMaxFreezeDurationMs;
};
struct FreezeEvent {
@@ -380,7 +375,8 @@
// Process a frame freeze.
static void processFreeze(int64_t actualRenderTimeUs, int64_t lastRenderTimeUs,
int64_t lastFreezeEndTimeUs, FreezeEvent &e,
- VideoRenderQualityMetrics &m, const Configuration &c);
+ VideoRenderQualityMetrics &m, const Configuration &c,
+ const TraceTriggerFn traceTriggerFn);
// Retrieve a freeze event if an event just finished.
static void maybeCaptureFreezeEvent(int64_t actualRenderTimeUs, int64_t lastFreezeEndTimeUs,
diff --git a/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp b/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp
index 9b6315c..16f8294 100644
--- a/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp
+++ b/media/libstagefright/tests/VideoRenderQualityTracker_test.cpp
@@ -156,7 +156,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withEmpty) {
@@ -188,7 +187,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withInvalid) {
@@ -220,7 +218,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withAlmostValid) {
@@ -297,7 +294,6 @@
EXPECT_EQ(c.traceTriggerEnabled, d.traceTriggerEnabled);
EXPECT_EQ(c.traceTriggerThrottleMs, d.traceTriggerThrottleMs);
EXPECT_EQ(c.traceMinFreezeDurationMs, d.traceMinFreezeDurationMs);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, d.traceMaxFreezeDurationMs);
}
TEST_F(VideoRenderQualityTrackerTest, getFromServerConfigurableFlags_withValid) {
@@ -412,7 +408,6 @@
EXPECT_EQ(c.traceTriggerEnabled, true);
EXPECT_EQ(c.traceTriggerThrottleMs, 50000);
EXPECT_EQ(c.traceMinFreezeDurationMs, 1000);
- EXPECT_EQ(c.traceMaxFreezeDurationMs, 5000);
}
TEST_F(VideoRenderQualityTrackerTest, countsReleasedFrames) {
@@ -1126,53 +1121,34 @@
c.enabled = true;
c.traceTriggerEnabled = true; // The trigger is enabled, so traces should be triggered.
// The value of traceTriggerThrottleMs must be larger than traceMinFreezeDurationMs. Otherwise,
- // the throttle does work.
+ // the throttle does not work.
c.traceTriggerThrottleMs = 200;
- c.traceMinFreezeDurationMs = 40;
- int32_t freeze = c.traceMinFreezeDurationMs;
+ c.traceMinFreezeDurationMs = 4 * 20; // 4 frames.
Helper h(20, c);
- // Freeze triggers separated by 80ms which is less than the threshold.
- h.render({
- freeze, // Freeze duration does not check trace trigger.
- 20, // Trace triggered.
- 20, // Throttle time: 20/200ms
- 20, // Throttle time: 40/200ms
- freeze, // Throttle time: 80/200ms
- 20, // Throttle time: 100/200ms (Trace not triggered)
- });
+ // Freeze triggers separated by 100ms which is less than the threshold.
+ h.render(1); // Video start.
+ h.drop(3); // Freeze.
+ h.render(1); // Trace triggered.
+ h.render(1); // Throttle time: 20/200ms
+ h.drop(3); // Throttle time: 80/200ms
+ h.render(1); // Throttle time: 100/200ms (Trace not triggered)
EXPECT_EQ(h.getTraceTriggeredCount(), 1);
// Next freeze trigger is separated by 200ms which breaks the throttle threshold.
- h.render({
- 20, // Throttle time: 120/200ms
- 20, // Throttle time: 140/200ms
- 20, // Throttle time: 160/200ms
- freeze, // Throttle time: 200/200ms
- 20, // Trace triggered.
- });
+ h.render(1); // Throttle time: 120/200ms
+ h.drop(3); // Throttle time: 180/200ms
+ h.render(1); // Throttle time: 200/200ms (Trace triggered)
EXPECT_EQ(h.getTraceTriggeredCount(), 2);
- // Next freeze trigger is separated by 80ms which is less than the threshold.
- h.render({
- 20, // Throttle time: 20/200ms
- 20, // Throttle time: 40/200ms
- freeze, // Throttle time: 80/200ms
- 20, // Throttle time: 100/200ms (Trace not triggered)
- });
+ // Next freeze trigger is separated by 100ms which is less than the threshold.
+ h.render(1); // Throttle time: 20/200ms
+ h.drop(3); // Throttle time: 80/200ms
+ h.render(1); // Throttle time: 100/200ms (Trace not triggered)
EXPECT_EQ(h.getTraceTriggeredCount(), 2);
-}
-
-TEST_F(VideoRenderQualityTrackerTest, freezeForTraceDuration_triggersTrace) {
- Configuration c;
- c.enabled = true;
- c.traceTriggerEnabled = true; // The trigger is enabled, so traces should be triggered.
- c.traceTriggerThrottleMs = 0; // Disable throttle in the test case.
- int32_t freeze1 = c.traceMinFreezeDurationMs;
- int32_t freeze2 = c.traceMaxFreezeDurationMs - 1;
- int32_t couldBeAPause = c.traceMaxFreezeDurationMs + 1;
-
- Helper h(20, c);
- h.render({freeze1, 20, freeze2, 20, couldBeAPause, 20});
-
+ // Freeze duration is less than traceMinFreezeDurationMs and throttle ends.
+ h.render(1); // Throttle time: 120/200ms
+ h.render(1); // Throttle time: 140/200ms
+ h.drop(2); // Throttle time: 180/200ms
+ h.render(1); // Throttle time: 200/200ms (Trace not triggered, freeze duration = 60ms)
EXPECT_EQ(h.getTraceTriggeredCount(), 2);
}
@@ -1182,12 +1158,14 @@
c.enabled = true;
c.traceTriggerEnabled = false; // The trigger is disabled, so no traces should be triggered.
c.traceTriggerThrottleMs = 0; // Disable throttle in the test case.
- int32_t freeze1 = c.traceMinFreezeDurationMs;
- int32_t freeze2 = c.traceMaxFreezeDurationMs - 1;
- int32_t couldBeAPause = c.traceMaxFreezeDurationMs + 1;
+ c.traceMinFreezeDurationMs = 4 * 20; // 4 frames.
Helper h(20, c);
- h.render({freeze1, 20, freeze2, 20, couldBeAPause, 20});
+ h.render(1);
+ h.drop(3);
+ h.render(1); // Render duration is 80 ms.
+ h.drop(4);
+ h.render(1); // Render duration is 100 ms.
EXPECT_EQ(h.getTraceTriggeredCount(), 0);
}
diff --git a/media/utils/include/mediautils/FixedString.h b/media/utils/include/mediautils/FixedString.h
index 047aa82..c316813 100644
--- a/media/utils/include/mediautils/FixedString.h
+++ b/media/utils/include/mediautils/FixedString.h
@@ -101,10 +101,15 @@
return strncmp(c_str(), s, capacity() + 1) == 0;
}
- bool operator==(std::string_view s) const {
+ bool operator==(const std::string_view s) const {
return size() == s.size() && memcmp(data(), s.data(), size()) == 0;
}
+ template <uint32_t N_>
+ bool operator==(const FixedString<N_>& s) const {
+ return operator==(s.asStringView());
+ }
+
// operator not-equals
template <typename T>
bool operator!=(const T& other) const {
diff --git a/media/utils/tests/Android.bp b/media/utils/tests/Android.bp
index 0d6a4c5..0689083 100644
--- a/media/utils/tests/Android.bp
+++ b/media/utils/tests/Android.bp
@@ -13,7 +13,6 @@
host_supported: true,
- cpp_std: "gnu++17",
cflags: [
"-Wall",
"-Werror",