Merge "audioflinger: Move async callback setting to onFirstRef"
diff --git a/media/audioaidlconversion/AidlConversionEffect.cpp b/media/audioaidlconversion/AidlConversionEffect.cpp
index 2df97d1..ec380e3 100644
--- a/media/audioaidlconversion/AidlConversionEffect.cpp
+++ b/media/audioaidlconversion/AidlConversionEffect.cpp
@@ -14,12 +14,16 @@
  * limitations under the License.
  */
 
+#include <cstdint>
+#include <inttypes.h>
 #include <utility>
 
 #define LOG_TAG "AidlConversionEffect"
 //#define LOG_NDEBUG 0
 #include <utils/Log.h>
 
+#include <aidl/android/hardware/audio/effect/DefaultExtension.h>
+#include <aidl/android/hardware/audio/effect/VendorExtension.h>
 #include <media/AidlConversionCppNdk.h>
 #include <media/AidlConversionEffect.h>
 
@@ -32,16 +36,21 @@
 using ::aidl::android::hardware::audio::effect::AcousticEchoCanceler;
 using ::aidl::android::hardware::audio::effect::AutomaticGainControlV2;
 using ::aidl::android::hardware::audio::effect::BassBoost;
+using ::aidl::android::hardware::audio::effect::DefaultExtension;
 using ::aidl::android::hardware::audio::effect::Descriptor;
 using ::aidl::android::hardware::audio::effect::Downmix;
 using ::aidl::android::hardware::audio::effect::DynamicsProcessing;
 using ::aidl::android::hardware::audio::effect::Flags;
 using ::aidl::android::hardware::audio::effect::Parameter;
 using ::aidl::android::hardware::audio::effect::PresetReverb;
+using ::aidl::android::hardware::audio::effect::VendorExtension;
+using ::aidl::android::hardware::audio::effect::Visualizer;
 using ::aidl::android::media::audio::common::AudioDeviceDescription;
 
 using ::android::BAD_VALUE;
 using ::android::base::unexpected;
+using ::android::effect::utils::EffectParamReader;
+using ::android::effect::utils::EffectParamWriter;
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////
 // Converters
@@ -349,5 +358,100 @@
     return static_cast<int32_t>(aidl);
 }
 
+ConversionResult<uint32_t> aidl2legacy_Parameter_Visualizer_ScalingMode_uint32(
+        Visualizer::ScalingMode aidl) {
+    switch (aidl) {
+        case Visualizer::ScalingMode::NORMALIZED: {
+            return 0;
+        }
+        case Visualizer::ScalingMode::AS_PLAYED: {
+            return 1;
+        }
+    }
+    return unexpected(BAD_VALUE);
+}
+
+ConversionResult<Visualizer::ScalingMode> legacy2aidl_Parameter_Visualizer_uint32_ScalingMode(
+        uint32_t legacy) {
+    if (legacy == 0) {
+        return Visualizer::ScalingMode::NORMALIZED;
+    } else if (legacy == 1) {
+        return Visualizer::ScalingMode::AS_PLAYED;
+    } else {
+        return unexpected(BAD_VALUE);
+    }
+}
+
+ConversionResult<uint32_t> aidl2legacy_Parameter_Visualizer_MeasurementMode_uint32(
+        Visualizer::MeasurementMode aidl) {
+    switch (aidl) {
+        case Visualizer::MeasurementMode::NONE: {
+            return 0;
+        }
+        case Visualizer::MeasurementMode::PEAK_RMS: {
+            return 1;
+        }
+    }
+    return unexpected(BAD_VALUE);
+}
+
+ConversionResult<Visualizer::MeasurementMode>
+legacy2aidl_Parameter_Visualizer_uint32_MeasurementMode(uint32_t legacy) {
+    if (legacy == 0) {
+        return Visualizer::MeasurementMode::NONE;
+    } else if (legacy == 1) {
+        return Visualizer::MeasurementMode::PEAK_RMS;
+    } else {
+        return unexpected(BAD_VALUE);
+    }
+}
+
+/**
+ * Copy the entire effect_param_t to DefaultExtension::bytes.
+ */
+ConversionResult<Parameter> legacy2aidl_EffectParameterReader_ParameterExtension(
+        EffectParamReader& param) {
+    size_t len = param.getTotalSize();
+    DefaultExtension ext;
+    ext.bytes.resize(len);
+    std::memcpy(ext.bytes.data(), &param.getEffectParam(), len);
+
+    VendorExtension effectParam;
+    effectParam.extension.setParcelable(ext);
+    return UNION_MAKE(Parameter, specific,
+                      UNION_MAKE(Parameter::Specific, vendorEffect, effectParam));
+}
+
+ConversionResult<std::vector<uint8_t>> aidl2legacy_ParameterExtension_vector_uint8(
+        const Parameter& param) {
+    VendorExtension effectParam = VALUE_OR_RETURN(
+            (::aidl::android::getParameterSpecific<Parameter, VendorExtension,
+                                                   Parameter::Specific::vendorEffect>(param)));
+    std::optional<DefaultExtension> ext;
+    if (STATUS_OK != effectParam.extension.getParcelable(&ext) || !ext.has_value()) {
+        return unexpected(BAD_VALUE);
+    }
+    return ext.value().bytes;
+}
+
+ConversionResult<::android::status_t> aidl2legacy_ParameterExtension_EffectParameterWriter(
+        const ::aidl::android::hardware::audio::effect::Parameter& aidl,
+        EffectParamWriter& legacy) {
+    const std::vector<uint8_t>& extBytes = VALUE_OR_RETURN_STATUS(
+            ::aidl::android::aidl2legacy_ParameterExtension_vector_uint8(aidl));
+    if (legacy.getTotalSize() < extBytes.size()) {
+        legacy.setStatus(BAD_VALUE);
+        return unexpected(BAD_VALUE);
+    }
+
+    // create a reader wrapper and read the content to legacy EffectParamWriter
+    EffectParamReader reader(*(effect_param_t*)extBytes.data());
+    if (STATUS_OK != legacy.writeToValue(reader.getValueAddress(), reader.getValueSize())) {
+        legacy.setStatus(BAD_VALUE);
+        return unexpected(BAD_VALUE);
+    }
+    return STATUS_OK;
+}
+
 }  // namespace android
 }  // aidl
diff --git a/media/audioaidlconversion/include/media/AidlConversionCppNdk.h b/media/audioaidlconversion/include/media/AidlConversionCppNdk.h
index abf0231..e1daf31 100644
--- a/media/audioaidlconversion/include/media/AidlConversionCppNdk.h
+++ b/media/audioaidlconversion/include/media/AidlConversionCppNdk.h
@@ -127,6 +127,11 @@
 ConversionResult<media::audio::common::AudioChannelLayout>
 legacy2aidl_audio_channel_mask_t_AudioChannelLayout(audio_channel_mask_t legacy, bool isInput);
 
+audio_channel_mask_t aidl2legacy_AudioChannelLayout_layout_audio_channel_mask_t_bits(
+        int aidlLayout, bool isInput);
+int legacy2aidl_audio_channel_mask_t_bits_AudioChannelLayout_layout(
+        audio_channel_mask_t legacy, bool isInput);
+
 enum class AudioPortDirection {
     INPUT, OUTPUT
 };
diff --git a/media/audioaidlconversion/include/media/AidlConversionEffect.h b/media/audioaidlconversion/include/media/AidlConversionEffect.h
index 83aa614..3aa9ac2 100644
--- a/media/audioaidlconversion/include/media/AidlConversionEffect.h
+++ b/media/audioaidlconversion/include/media/AidlConversionEffect.h
@@ -26,6 +26,7 @@
 #include <hardware/audio_effect.h>
 #include <media/AidlConversionUtil.h>
 #include <system/audio_effect.h>
+#include <system/audio_effects/audio_effects_utils.h>
 
 #include <aidl/android/hardware/audio/effect/IEffect.h>
 
@@ -126,5 +127,24 @@
 ConversionResult<int32_t> aidl2legacy_DynamicsProcessing_ResolutionPreference_int32(
         ::aidl::android::hardware::audio::effect::DynamicsProcessing::ResolutionPreference aidl);
 
+ConversionResult<uint32_t> aidl2legacy_Parameter_Visualizer_ScalingMode_uint32(
+        ::aidl::android::hardware::audio::effect::Visualizer::ScalingMode aidl);
+ConversionResult<::aidl::android::hardware::audio::effect::Visualizer::ScalingMode>
+legacy2aidl_Parameter_Visualizer_uint32_ScalingMode(uint32_t legacy);
+
+ConversionResult<uint32_t> aidl2legacy_Parameter_Visualizer_MeasurementMode_uint32(
+        ::aidl::android::hardware::audio::effect::Visualizer::MeasurementMode aidl);
+ConversionResult<::aidl::android::hardware::audio::effect::Visualizer::MeasurementMode>
+legacy2aidl_Parameter_Visualizer_uint32_MeasurementMode(uint32_t legacy);
+
+ConversionResult<::aidl::android::hardware::audio::effect::Parameter>
+legacy2aidl_EffectParameterReader_ParameterExtension(
+        ::android::effect::utils::EffectParamReader& param);
+ConversionResult<std::vector<uint8_t>> aidl2legacy_ParameterExtension_vector_uint8(
+        const ::aidl::android::hardware::audio::effect::Parameter& legacy);
+ConversionResult<::android::status_t> aidl2legacy_ParameterExtension_EffectParameterWriter(
+        const ::aidl::android::hardware::audio::effect::Parameter& aidl,
+        ::android::effect::utils::EffectParamWriter& legacy);
+
 }  // namespace android
 }  // namespace aidl
diff --git a/media/libaudiohal/impl/DeviceHalAidl.cpp b/media/libaudiohal/impl/DeviceHalAidl.cpp
index 32ebe36..697135f 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.cpp
+++ b/media/libaudiohal/impl/DeviceHalAidl.cpp
@@ -45,6 +45,7 @@
 using aidl::android::media::audio::common::AudioPort;
 using aidl::android::media::audio::common::AudioPortConfig;
 using aidl::android::media::audio::common::AudioPortDeviceExt;
+using aidl::android::media::audio::common::AudioPortMixExt;
 using aidl::android::media::audio::common::AudioPortExt;
 using aidl::android::media::audio::common::AudioSource;
 using aidl::android::media::audio::common::Int;
@@ -744,22 +745,27 @@
     return p.ext.get<AudioPortExt::Tag::device>().device == device;
 }
 
-status_t DeviceHalAidl::createPortConfig(const AudioPortConfig& requestedPortConfig,
-        AudioPortConfig* appliedPortConfig) {
+status_t DeviceHalAidl::createPortConfig(
+        const AudioPortConfig& requestedPortConfig, PortConfigs::iterator* result) {
     TIME_CHECK();
+    AudioPortConfig appliedPortConfig;
     bool applied = false;
     RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
-                            requestedPortConfig, appliedPortConfig, &applied)));
+                            requestedPortConfig, &appliedPortConfig, &applied)));
     if (!applied) {
         RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mModule->setAudioPortConfig(
-                                *appliedPortConfig, appliedPortConfig, &applied)));
+                                appliedPortConfig, &appliedPortConfig, &applied)));
         if (!applied) {
             ALOGE("%s: module %s did not apply suggested config %s",
-                    __func__, mInstance.c_str(), appliedPortConfig->toString().c_str());
+                    __func__, mInstance.c_str(), appliedPortConfig.toString().c_str());
             return NO_INIT;
         }
     }
-    mPortConfigs.emplace(appliedPortConfig->id, *appliedPortConfig);
+    auto id = appliedPortConfig.id;
+    auto [it, inserted] = mPortConfigs.emplace(std::move(id), std::move(appliedPortConfig));
+    LOG_ALWAYS_FATAL_IF(!inserted, "%s: port config with id %d already exists",
+            __func__, it->first);
+    *result = it;
     return OK;
 }
 
@@ -806,10 +812,7 @@
         }
         AudioPortConfig requestedPortConfig;
         requestedPortConfig.portId = portsIt->first;
-        AudioPortConfig appliedPortConfig;
-        RETURN_STATUS_IF_ERROR(createPortConfig(requestedPortConfig, &appliedPortConfig));
-        portConfigIt = mPortConfigs.insert(
-                mPortConfigs.end(), std::make_pair(appliedPortConfig.id, appliedPortConfig));
+        RETURN_STATUS_IF_ERROR(createPortConfig(requestedPortConfig, &portConfigIt));
         *created = true;
     } else {
         *created = false;
@@ -821,23 +824,40 @@
 status_t DeviceHalAidl::findOrCreatePortConfig(
         const AudioConfig& config, const std::optional<AudioIoFlags>& flags, int32_t ioHandle,
         AudioPortConfig* portConfig, bool* created) {
+    // These flags get removed one by one in this order when retrying port finding.
+    static const std::vector<AudioInputFlags> kOptionalInputFlags{
+        AudioInputFlags::FAST, AudioInputFlags::RAW };
     auto portConfigIt = findPortConfig(config, flags, ioHandle);
     if (portConfigIt == mPortConfigs.end() && flags.has_value()) {
-        auto portsIt = findPort(config, flags.value());
+        auto optionalInputFlagsIt = kOptionalInputFlags.begin();
+        AudioIoFlags matchFlags = flags.value();
+        auto portsIt = findPort(config, matchFlags);
+        while (portsIt == mPorts.end() && matchFlags.getTag() == AudioIoFlags::Tag::input
+                && optionalInputFlagsIt != kOptionalInputFlags.end()) {
+            if (!isBitPositionFlagSet(
+                            matchFlags.get<AudioIoFlags::Tag::input>(), *optionalInputFlagsIt)) {
+                ++optionalInputFlagsIt;
+                continue;
+            }
+            matchFlags.set<AudioIoFlags::Tag::input>(matchFlags.get<AudioIoFlags::Tag::input>() &
+                    ~makeBitPositionFlagMask(*optionalInputFlagsIt++));
+            portsIt = findPort(config, matchFlags);
+            ALOGI("%s: mix port for config %s, flags %s was not found in the module %s, "
+                    "retried with flags %s", __func__, config.toString().c_str(),
+                    flags.value().toString().c_str(), mInstance.c_str(),
+                    matchFlags.toString().c_str());
+        }
         if (portsIt == mPorts.end()) {
             ALOGE("%s: mix port for config %s, flags %s is not found in the module %s",
-                    __func__, config.toString().c_str(), flags.value().toString().c_str(),
+                    __func__, config.toString().c_str(), matchFlags.toString().c_str(),
                     mInstance.c_str());
             return BAD_VALUE;
         }
         AudioPortConfig requestedPortConfig;
         requestedPortConfig.portId = portsIt->first;
         setPortConfigFromConfig(&requestedPortConfig, config);
-        AudioPortConfig appliedPortConfig;
-        RETURN_STATUS_IF_ERROR(createPortConfig(requestedPortConfig, &appliedPortConfig));
-        appliedPortConfig.ext.get<AudioPortExt::Tag::mix>().handle = ioHandle;
-        portConfigIt = mPortConfigs.insert(
-                mPortConfigs.end(), std::make_pair(appliedPortConfig.id, appliedPortConfig));
+        requestedPortConfig.ext = AudioPortMixExt{ .handle = ioHandle };
+        RETURN_STATUS_IF_ERROR(createPortConfig(requestedPortConfig, &portConfigIt));
         *created = true;
     } else if (!flags.has_value()) {
         ALOGW("%s: mix port config for %s, handle %d not found in the module %s, "
@@ -899,12 +919,10 @@
 
 DeviceHalAidl::Ports::iterator DeviceHalAidl::findPort(
             const AudioConfig& config, const AudioIoFlags& flags) {
-    using Tag = AudioPortExt::Tag;
-    AudioIoFlags matchFlags = flags;
     auto matcher = [&](const auto& pair) {
         const auto& p = pair.second;
-        return p.ext.getTag() == Tag::mix &&
-                p.flags == matchFlags &&
+        return p.ext.getTag() == AudioPortExt::Tag::mix &&
+                p.flags == flags &&
                 std::find_if(p.profiles.begin(), p.profiles.end(),
                         [&](const auto& prof) {
                             return prof.format == config.base.format &&
@@ -913,15 +931,7 @@
                                     std::find(prof.sampleRates.begin(), prof.sampleRates.end(),
                                             config.base.sampleRate) != prof.sampleRates.end();
                         }) != p.profiles.end(); };
-    auto it = std::find_if(mPorts.begin(), mPorts.end(), matcher);
-    if (it == mPorts.end() && flags.getTag() == AudioIoFlags::Tag::input &&
-            isBitPositionFlagSet(flags.get<AudioIoFlags::Tag::input>(), AudioInputFlags::FAST)) {
-        // "Fast" input is not a mandatory flag, try without it.
-        matchFlags.set<AudioIoFlags::Tag::input>(flags.get<AudioIoFlags::Tag::input>() &
-                ~makeBitPositionFlagMask(AudioInputFlags::FAST));
-        it = std::find_if(mPorts.begin(), mPorts.end(), matcher);
-    }
-    return it;
+    return std::find_if(mPorts.begin(), mPorts.end(), matcher);
 }
 
 DeviceHalAidl::PortConfigs::iterator DeviceHalAidl::findPortConfig(const AudioDevice& device) {
diff --git a/media/libaudiohal/impl/DeviceHalAidl.h b/media/libaudiohal/impl/DeviceHalAidl.h
index 6f16daf..5e6f662 100644
--- a/media/libaudiohal/impl/DeviceHalAidl.h
+++ b/media/libaudiohal/impl/DeviceHalAidl.h
@@ -179,7 +179,7 @@
             const ::aidl::android::media::audio::common::AudioPortConfig& p);
     status_t createPortConfig(
             const ::aidl::android::media::audio::common::AudioPortConfig& requestedPortConfig,
-            ::aidl::android::media::audio::common::AudioPortConfig* appliedPortConfig);
+            PortConfigs::iterator* result);
     status_t findOrCreatePatch(
         const std::set<int32_t>& sourcePortConfigIds,
         const std::set<int32_t>& sinkPortConfigIds,
diff --git a/media/libaudiohal/impl/EffectConversionHelperAidl.cpp b/media/libaudiohal/impl/EffectConversionHelperAidl.cpp
index 7e25b04..dc47d67 100644
--- a/media/libaudiohal/impl/EffectConversionHelperAidl.cpp
+++ b/media/libaudiohal/impl/EffectConversionHelperAidl.cpp
@@ -24,6 +24,7 @@
 #include <media/AidlConversionCppNdk.h>
 #include <media/AidlConversionNdk.h>
 #include <media/AidlConversionEffect.h>
+#include <system/audio_effects/effect_visualizer.h>
 
 #include <utils/Log.h>
 
@@ -59,7 +60,10 @@
                 {EFFECT_CMD_SET_INPUT_DEVICE, &EffectConversionHelperAidl::handleSetDevice},
                 {EFFECT_CMD_SET_VOLUME, &EffectConversionHelperAidl::handleSetVolume},
                 {EFFECT_CMD_OFFLOAD, &EffectConversionHelperAidl::handleSetOffload},
-                {EFFECT_CMD_FIRST_PROPRIETARY, &EffectConversionHelperAidl::handleFirstPriority}};
+                {EFFECT_CMD_FIRST_PROPRIETARY, &EffectConversionHelperAidl::handleFirstPriority},
+                // Only visualizer support these commands
+                {VISUALIZER_CMD_CAPTURE, &EffectConversionHelperAidl::handleVisualizerCapture},
+                {VISUALIZER_CMD_MEASURE, &EffectConversionHelperAidl::handleVisualizerMeasure}};
 
 EffectConversionHelperAidl::EffectConversionHelperAidl(
         std::shared_ptr<::aidl::android::hardware::audio::effect::IEffect> effect,
@@ -286,5 +290,45 @@
     return OK;
 }
 
+status_t EffectConversionHelperAidl::handleVisualizerCapture(uint32_t cmdSize __unused,
+                                                             const void* pCmdData __unused,
+                                                             uint32_t* replySize,
+                                                             void* pReplyData) {
+    if (!replySize || !pReplyData) {
+        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+        return BAD_VALUE;
+    }
+
+    const auto& uuid = VALUE_OR_RETURN_STATUS(
+            ::aidl::android::aidl2legacy_AudioUuid_audio_uuid_t(mDesc.common.id.type));
+    if (0 != memcmp(&uuid, SL_IID_VISUALIZATION, sizeof(effect_uuid_t))) {
+        ALOGE("%s visualizer command not supported by %s", __func__,
+              mDesc.common.id.toString().c_str());
+        return BAD_VALUE;
+    }
+
+    return visualizerCapture(replySize, pReplyData);
+}
+
+status_t EffectConversionHelperAidl::handleVisualizerMeasure(uint32_t cmdSize __unused,
+                                                             const void* pCmdData __unused,
+                                                             uint32_t* replySize,
+                                                             void* pReplyData) {
+    if (!replySize || !pReplyData) {
+        ALOGE("%s parameter invalid %p %p", __func__, replySize, pReplyData);
+        return BAD_VALUE;
+    }
+
+    const auto& uuid = VALUE_OR_RETURN_STATUS(
+            ::aidl::android::aidl2legacy_AudioUuid_audio_uuid_t(mDesc.common.id.type));
+    if (0 != memcmp(&uuid, SL_IID_VISUALIZATION, sizeof(effect_uuid_t))) {
+        ALOGE("%s visualizer command not supported by %s", __func__,
+              mDesc.common.id.toString().c_str());
+        return BAD_VALUE;
+    }
+
+    return visualizerMeasure(replySize, pReplyData);
+}
+
 }  // namespace effect
 }  // namespace android
diff --git a/media/libaudiohal/impl/EffectConversionHelperAidl.h b/media/libaudiohal/impl/EffectConversionHelperAidl.h
index 94435c6..37ca75d 100644
--- a/media/libaudiohal/impl/EffectConversionHelperAidl.h
+++ b/media/libaudiohal/impl/EffectConversionHelperAidl.h
@@ -94,10 +94,20 @@
                               void* pReplyData);
     status_t handleFirstPriority(uint32_t cmdSize, const void* pCmdData, uint32_t* replySize,
                                  void* pReplyData);
+    status_t handleVisualizerCapture(uint32_t cmdSize, const void* pCmdData, uint32_t* replySize,
+                                     void* pReplyData);
+    status_t handleVisualizerMeasure(uint32_t cmdSize, const void* pCmdData, uint32_t* replySize,
+                                     void* pReplyData);
 
     // implemented by conversion of each effect
     virtual status_t setParameter(utils::EffectParamReader& param) = 0;
     virtual status_t getParameter(utils::EffectParamWriter& param) = 0;
+    virtual status_t visualizerCapture(uint32_t* replySize __unused, void* pReplyData __unused) {
+        return BAD_VALUE;
+    }
+    virtual status_t visualizerMeasure(uint32_t* replySize __unused, void* pReplyData __unused) {
+        return BAD_VALUE;
+    }
 };
 
 }  // namespace effect
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionBassBoost.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionBassBoost.cpp
index 91c3dea..9ec593f 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionBassBoost.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionBassBoost.cpp
@@ -82,7 +82,7 @@
     Parameter aidlParam;
     switch (type) {
         case BASSBOOST_PARAM_STRENGTH: {
-            uint32_t value;
+            uint16_t value;
             Parameter::Id id =
                     MAKE_SPECIFIC_PARAMETER_ID(BassBoost, bassBoostTag, BassBoost::strengthPm);
             RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
@@ -92,7 +92,7 @@
         }
         case BASSBOOST_PARAM_STRENGTH_SUPPORTED: {
             // an invalid range indicates not setting support for this parameter
-            uint16_t value =
+            uint32_t value =
                     ::aidl::android::hardware::audio::effect::isRangeValid<Range::Tag::bassBoost>(
                             BassBoost::strengthPm, mDesc.capability);
             return param.writeToValue(&value);
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionEnvReverb.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionEnvReverb.cpp
index 960273b..0544e3f 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionEnvReverb.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionEnvReverb.cpp
@@ -43,157 +43,208 @@
 using utils::EffectParamReader;
 using utils::EffectParamWriter;
 
-#define MAKE_AIDL_PARAMETER(aidlParam, param, value, tag)                            \
-    {                                                                                \
-        if (OK != param.readFromValue(&value)) {                                     \
-            ALOGE("%s invalid parameter %s %d", __func__, #tag, value);              \
-            return BAD_VALUE;                                                        \
-        }                                                                            \
-        aidlParam = MAKE_SPECIFIC_PARAMETER(                                         \
-                EnvironmentalReverb, environmentalReverb, tag,                       \
-                VALUE_OR_RETURN_STATUS(aidl::android::convertIntegral<int>(value))); \
+/**
+ * Macro to get a parameter from effect_param_t wrapper and set it to AIDL effect.
+ *
+ * Return if there is any error, otherwise continue execution.
+ *
+ * @param param EffectParamReader, a reader wrapper of effect_param_t.
+ * @param aidlType Type of the AIDL parameter field, used to construct AIDL Parameter union.
+ * @param valueType Type of the value get from effect_param_t.
+ * @param tag The AIDL parameter union field tag.
+ */
+#define SET_AIDL_PARAMETER(param, aidlType, valueType, tag)                                \
+    {                                                                                      \
+        Parameter aidlParam;                                                               \
+        valueType value;                                                                   \
+        if (status_t status = param.readFromValue(&value); status != OK) {                 \
+            ALOGE("%s  %s read from parameter failed, ret %d", __func__, #tag, status);    \
+            return status;                                                                 \
+        }                                                                                  \
+        aidlParam = MAKE_SPECIFIC_PARAMETER(                                               \
+                EnvironmentalReverb, environmentalReverb, tag,                             \
+                VALUE_OR_RETURN_STATUS(aidl::android::convertIntegral<aidlType>(value)));  \
+        RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->setParameter(aidlParam))); \
     }
 
-#define GET_AIDL_PARAMETER(tag, value, param)                                                      \
+/**
+ * Macro to get a parameter from AIDL effect and write the value to effect_param_t with wrapper.
+ *
+ * Return if there is any error, otherwise continue execution.
+ *
+ * @param param EffectParamWriter, a writer wrapper of effect_param_t.
+ * @param aidlType Type of the AIDL parameter field, used to construct AIDL Parameter union.
+ * @param valueType  Type of the value get from effect_param_t.
+ * @param tag The AIDL parameter union field tag.
+ */
+#define GET_AIDL_PARAMETER(param, aidltype, valueType, tag)                                        \
     {                                                                                              \
+        aidltype value;                                                                            \
         Parameter aidlParam;                                                                       \
         Parameter::Id id = MAKE_SPECIFIC_PARAMETER_ID(EnvironmentalReverb, environmentalReverbTag, \
                                                       EnvironmentalReverb::tag);                   \
         RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));    \
-        value = VALUE_OR_RETURN_STATUS(GET_PARAMETER_SPECIFIC_FIELD(                               \
-                aidlParam, EnvironmentalReverb, environmentalReverb, EnvironmentalReverb::tag,     \
-                std::decay_t<decltype(value)>));                                                   \
-        return param.writeToValue(&value);                                                         \
+        value = VALUE_OR_RETURN_STATUS(                                                            \
+                GET_PARAMETER_SPECIFIC_FIELD(aidlParam, EnvironmentalReverb, environmentalReverb,  \
+                                             EnvironmentalReverb::tag, std::decay_t<aidltype>));   \
+        if (status_t status = param.writeToValue((valueType*)&value); status != OK) {              \
+            param.setStatus(status);                                                               \
+            ALOGE("%s %s write to parameter failed %d, ret %d", __func__, #tag, value, status);    \
+            return status;                                                                         \
+        }                                                                                          \
     }
 
 status_t AidlConversionEnvReverb::setParameter(EffectParamReader& param) {
     uint32_t type = 0;
-    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint16_t)) ||
-        OK != param.readFromParameter(&type)) {
-        ALOGE("%s invalid param %s", __func__, param.toString().c_str());
+    if (status_t status = param.readFromParameter(&type); status != OK) {
+        ALOGE("%s failed to read type from %s, ret %d", __func__, param.toString().c_str(), status);
         return BAD_VALUE;
     }
-    Parameter aidlParam;
-    uint16_t value16;
-    uint32_t value32;
+
     switch (type) {
         case REVERB_PARAM_ROOM_LEVEL: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value16, roomLevelMb);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, roomLevelMb);
             break;
         }
         case REVERB_PARAM_ROOM_HF_LEVEL: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value16, roomHfLevelMb);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, roomHfLevelMb);
             break;
         }
         case REVERB_PARAM_DECAY_TIME: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value32, decayTimeMs);
+            SET_AIDL_PARAMETER(param, int32_t, uint32_t, decayTimeMs);
             break;
         }
         case REVERB_PARAM_DECAY_HF_RATIO: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value16, decayHfRatioPm);
-            break;
-        }
-        case REVERB_PARAM_REVERB_LEVEL: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value16, levelMb);
-            break;
-        }
-        case REVERB_PARAM_REVERB_DELAY: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value32, delayMs);
-            break;
-        }
-        case REVERB_PARAM_DIFFUSION: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value16, diffusionPm);
-            break;
-        }
-        case REVERB_PARAM_DENSITY: {
-            MAKE_AIDL_PARAMETER(aidlParam, param, value16, densityPm);
-            break;
-        }
-        case REVERB_PARAM_BYPASS: {
-            if (OK != param.readFromValue(&value32)) {
-                ALOGE("%s invalid bypass parameter %d", __func__, value32);
-                return BAD_VALUE;
-            }
-            bool isByPass = VALUE_OR_RETURN_STATUS(aidl::android::convertIntegral<bool>(value32));
-            aidlParam = MAKE_SPECIFIC_PARAMETER(EnvironmentalReverb, environmentalReverb, bypass,
-                                                isByPass);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, decayHfRatioPm);
             break;
         }
         case REVERB_PARAM_REFLECTIONS_LEVEL: {
-            // TODO
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, reflectionsLevelMb);
             break;
         }
         case REVERB_PARAM_REFLECTIONS_DELAY: {
-            // TODO
+            SET_AIDL_PARAMETER(param, int32_t, uint32_t, reflectionsDelayMs);
+            break;
+        }
+        case REVERB_PARAM_REVERB_LEVEL: {
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, levelMb);
+            break;
+        }
+        case REVERB_PARAM_REVERB_DELAY: {
+            SET_AIDL_PARAMETER(param, int32_t, uint32_t, delayMs);
+            break;
+        }
+        case REVERB_PARAM_DIFFUSION: {
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, diffusionPm);
+            break;
+        }
+        case REVERB_PARAM_DENSITY: {
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, densityPm);
+            break;
+        }
+        case REVERB_PARAM_BYPASS: {
+            SET_AIDL_PARAMETER(param, bool, int32_t, bypass);
             break;
         }
         case REVERB_PARAM_PROPERTIES: {
-            // TODO
+            if (sizeof(t_reverb_settings) > param.getValueSize()) {
+                ALOGE("%s vsize %zu less than t_reverb_settings size %zu", __func__,
+                      param.getValueSize(), sizeof(t_reverb_settings));
+                return BAD_VALUE;
+            }
+            // this sequency needs to be aligned with t_reverb_settings
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, roomLevelMb);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, roomHfLevelMb);
+            SET_AIDL_PARAMETER(param, int32_t, uint32_t, decayTimeMs);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, decayHfRatioPm);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, reflectionsLevelMb);
+            SET_AIDL_PARAMETER(param, int32_t, uint32_t, reflectionsDelayMs);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, levelMb);
+            SET_AIDL_PARAMETER(param, int32_t, uint32_t, delayMs);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, diffusionPm);
+            SET_AIDL_PARAMETER(param, int32_t, int16_t, densityPm);
             break;
         }
         default: {
             // TODO: handle with vendor extension
         }
     }
-    return statusTFromBinderStatus(mEffect->setParameter(aidlParam));
+    return OK;
 }
 
 status_t AidlConversionEnvReverb::getParameter(EffectParamWriter& param) {
     uint32_t type = 0;
-    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint32_t)) ||
-        OK != param.readFromParameter(&type)) {
-        ALOGE("%s invalid param %s", __func__, param.toString().c_str());
-        param.setStatus(BAD_VALUE);
-        return BAD_VALUE;
+    if (status_t status = param.readFromParameter(&type); status != OK) {
+        ALOGE("%s failed to read type from %s", __func__, param.toString().c_str());
+        param.setStatus(status);
+        return status;
     }
-    uint16_t value16;
-    uint32_t value32;
+
     switch (type) {
         case REVERB_PARAM_ROOM_LEVEL: {
-            GET_AIDL_PARAMETER(roomLevelMb, value16, param);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, roomLevelMb);
+            break;
         }
         case REVERB_PARAM_ROOM_HF_LEVEL: {
-            GET_AIDL_PARAMETER(roomHfLevelMb, value16, param);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, roomHfLevelMb);
+            break;
         }
         case REVERB_PARAM_DECAY_TIME: {
-            GET_AIDL_PARAMETER(decayTimeMs, value32, param);
+            GET_AIDL_PARAMETER(param, int32_t, uint32_t, decayTimeMs);
+            break;
         }
         case REVERB_PARAM_DECAY_HF_RATIO: {
-            GET_AIDL_PARAMETER(decayHfRatioPm, value16, param);
-        }
-        case REVERB_PARAM_REVERB_LEVEL: {
-            GET_AIDL_PARAMETER(levelMb, value16, param);
-        }
-        case REVERB_PARAM_REVERB_DELAY: {
-            GET_AIDL_PARAMETER(delayMs, value32, param);
-        }
-        case REVERB_PARAM_DIFFUSION: {
-            GET_AIDL_PARAMETER(diffusionPm, value16, param);
-        }
-        case REVERB_PARAM_DENSITY: {
-            GET_AIDL_PARAMETER(densityPm, value16, param);
-        }
-        case REVERB_PARAM_BYPASS: {
-            bool isByPass;
-            GET_AIDL_PARAMETER(bypass, isByPass, param);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, decayHfRatioPm);
+            break;
         }
         case REVERB_PARAM_REFLECTIONS_LEVEL: {
-            // TODO
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, reflectionsLevelMb);
             break;
         }
         case REVERB_PARAM_REFLECTIONS_DELAY: {
-            // TODO
+            GET_AIDL_PARAMETER(param, int32_t, uint32_t, reflectionsDelayMs);
+            break;
+        }
+        case REVERB_PARAM_REVERB_LEVEL: {
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, levelMb);
+            break;
+        }
+        case REVERB_PARAM_REVERB_DELAY: {
+            GET_AIDL_PARAMETER(param, int32_t, uint32_t, delayMs);
+            break;
+        }
+        case REVERB_PARAM_DIFFUSION: {
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, diffusionPm);
+            break;
+        }
+        case REVERB_PARAM_DENSITY: {
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, densityPm);
+            break;
+        }
+        case REVERB_PARAM_BYPASS: {
+            GET_AIDL_PARAMETER(param, bool, int32_t, bypass);
             break;
         }
         case REVERB_PARAM_PROPERTIES: {
-            // TODO
+            // this sequency needs to be aligned with t_reverb_settings
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, roomLevelMb);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, roomHfLevelMb);
+            GET_AIDL_PARAMETER(param, int32_t, uint32_t, decayTimeMs);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, decayHfRatioPm);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, reflectionsLevelMb);
+            GET_AIDL_PARAMETER(param, int32_t, uint32_t, reflectionsDelayMs);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, levelMb);
+            GET_AIDL_PARAMETER(param, int32_t, uint32_t, delayMs);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, diffusionPm);
+            GET_AIDL_PARAMETER(param, int32_t, int16_t, densityPm);
             break;
         }
         default: {
             // TODO: handle with vendor extension
+            return BAD_VALUE;
         }
     }
-    return BAD_VALUE;
+    return OK;
 }
 
 } // namespace effect
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionSpatializer.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionSpatializer.cpp
index 1dac479..d2a94e4 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionSpatializer.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionSpatializer.cpp
@@ -40,28 +40,24 @@
 using utils::EffectParamWriter;
 
 status_t AidlConversionSpatializer::setParameter(EffectParamReader& param) {
-    uint32_t type = 0;
-    uint16_t value = 0;
-    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint16_t)) ||
-        OK != param.readFromParameter(&type) || OK != param.readFromValue(&value)) {
-        ALOGE("%s invalid param %s", __func__, param.toString().c_str());
-        return BAD_VALUE;
-    }
-    Parameter aidlParam;
-    // TODO
+    Parameter aidlParam = VALUE_OR_RETURN_STATUS(
+            ::aidl::android::legacy2aidl_EffectParameterReader_ParameterExtension(param));
     return statusTFromBinderStatus(mEffect->setParameter(aidlParam));
 }
 
 status_t AidlConversionSpatializer::getParameter(EffectParamWriter& param) {
-    uint32_t type = 0, value = 0;
-    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint32_t)) ||
-        OK != param.readFromParameter(&type)) {
-        ALOGE("%s invalid param %s", __func__, param.toString().c_str());
+    Parameter aidlParam;
+    Parameter::Id id = UNION_MAKE(Parameter::Id, vendorEffectTag, 0 /* no tag */);
+    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+    const auto& extBytes = VALUE_OR_RETURN_STATUS(
+            ::aidl::android::aidl2legacy_ParameterExtension_vector_uint8(aidlParam));
+    if (param.getValueSize() < extBytes.size()) {
+        ALOGE("%s extension return data %zu exceed vsize %zu", __func__, extBytes.size(),
+              param.getValueSize());
         param.setStatus(BAD_VALUE);
         return BAD_VALUE;
     }
-    // TODO
-    return param.writeToValue(&value);
+    return param.writeToValue(extBytes.data(), extBytes.size());
 }
 
 } // namespace effect
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVendorExtension.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVendorExtension.cpp
index 3baf72e..584b60e 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVendorExtension.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVendorExtension.cpp
@@ -50,17 +50,8 @@
  * pass down in Parameter as is.
  */
 status_t AidlConversionVendorExtension::setParameter(EffectParamReader& param) {
-    size_t len = param.getValueSize();
-    DefaultExtension ext;
-    ext.bytes.resize(len);
-    if (OK != param.readFromValue(ext.bytes.data(), len)) {
-        ALOGE("%s read value from param %s failed", __func__, param.toString().c_str());
-        return BAD_VALUE;
-    }
-    VendorExtension effectParam;
-    effectParam.extension.setParcelable(ext);
-    Parameter aidlParam = UNION_MAKE(Parameter, specific,
-                                     UNION_MAKE(Parameter::Specific, vendorEffect, effectParam));
+    Parameter aidlParam = VALUE_OR_RETURN_STATUS(
+            ::aidl::android::legacy2aidl_EffectParameterReader_ParameterExtension(param));
     return statusTFromBinderStatus(mEffect->setParameter(aidlParam));
 }
 
@@ -75,23 +66,11 @@
     Parameter aidlParam;
     Parameter::Id id = UNION_MAKE(Parameter::Id, vendorEffectTag, tag /* parameter tag */);
     RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
-    VendorExtension effectParam = VALUE_OR_RETURN_STATUS(
-            (::aidl::android::getParameterSpecific<Parameter, VendorExtension,
-                                                   Parameter::Specific::vendorEffect>(aidlParam)));
-    std::optional<DefaultExtension> ext;
-    if (STATUS_OK != effectParam.extension.getParcelable(&ext) || !ext.has_value()) {
-        ALOGE("%s get extension parcelable failed", __func__);
-        param.setStatus(BAD_VALUE);
-        return BAD_VALUE;
-    }
-    const auto& extBytes = ext.value().bytes;
-    if (param.getValueSize() < extBytes.size()) {
-        ALOGE("%s extension return data %zu exceed vsize %zu", __func__, extBytes.size(),
-              param.getValueSize());
-        param.setStatus(BAD_VALUE);
-        return BAD_VALUE;
-    }
-    return param.writeToValue(extBytes.data(), extBytes.size());
+
+    // copy the AIDL extension data back to effect_param_t
+    return VALUE_OR_RETURN_STATUS(
+            ::aidl::android::aidl2legacy_ParameterExtension_EffectParameterWriter(aidlParam,
+                                                                                  param));
 }
 
 } // namespace effect
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp
index 9ed601f..7e1e6d7 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.cpp
@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+#include <cstddef>
 #include <cstdint>
 #include <cstring>
 #include <optional>
@@ -34,34 +35,142 @@
 namespace effect {
 
 using ::aidl::android::aidl_utils::statusTFromBinderStatus;
+using ::aidl::android::getParameterSpecificField;
 using ::aidl::android::hardware::audio::effect::Parameter;
+using ::aidl::android::hardware::audio::effect::Visualizer;
 using ::android::status_t;
 using utils::EffectParamReader;
 using utils::EffectParamWriter;
 
 status_t AidlConversionVisualizer::setParameter(EffectParamReader& param) {
-    uint32_t type = 0;
-    uint16_t value = 0;
-    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint16_t)) ||
+    uint32_t type = 0, value = 0;
+    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint32_t)) ||
         OK != param.readFromParameter(&type) || OK != param.readFromValue(&value)) {
         ALOGE("%s invalid param %s", __func__, param.toString().c_str());
         return BAD_VALUE;
     }
     Parameter aidlParam;
-    // TODO
+    switch (type) {
+        case VISUALIZER_PARAM_CAPTURE_SIZE: {
+            aidlParam = MAKE_SPECIFIC_PARAMETER(Visualizer, visualizer, captureSamples, value);
+            break;
+        }
+        case VISUALIZER_PARAM_SCALING_MODE: {
+            Visualizer::ScalingMode mode = VALUE_OR_RETURN_STATUS(
+                    aidl::android::legacy2aidl_Parameter_Visualizer_uint32_ScalingMode(value));
+            aidlParam = MAKE_SPECIFIC_PARAMETER(Visualizer, visualizer, scalingMode, mode);
+            break;
+        }
+        case VISUALIZER_PARAM_LATENCY: {
+            aidlParam = MAKE_SPECIFIC_PARAMETER(Visualizer, visualizer, latencyMs, value);
+            break;
+        }
+        case VISUALIZER_PARAM_MEASUREMENT_MODE: {
+            Visualizer::MeasurementMode mode = VALUE_OR_RETURN_STATUS(
+                    aidl::android::legacy2aidl_Parameter_Visualizer_uint32_MeasurementMode(value));
+            aidlParam = MAKE_SPECIFIC_PARAMETER(Visualizer, visualizer, measurementMode, mode);
+            break;
+        }
+        default: {
+            // TODO: implement vendor extension parameters
+            ALOGW("%s unknown param %s", __func__, param.toString().c_str());
+            return BAD_VALUE;
+        }
+    }
     return statusTFromBinderStatus(mEffect->setParameter(aidlParam));
 }
 
 status_t AidlConversionVisualizer::getParameter(EffectParamWriter& param) {
     uint32_t type = 0, value = 0;
-    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(uint32_t)) ||
+    if (!param.validateParamValueSize(sizeof(uint32_t), sizeof(int32_t)) ||
         OK != param.readFromParameter(&type)) {
         ALOGE("%s invalid param %s", __func__, param.toString().c_str());
         param.setStatus(BAD_VALUE);
         return BAD_VALUE;
     }
-    // TODO
-    return param.writeToValue(&value);
+    Parameter aidlParam;
+    switch (type) {
+        case VISUALIZER_PARAM_CAPTURE_SIZE: {
+            Parameter::Id id = MAKE_SPECIFIC_PARAMETER_ID(Visualizer, visualizerTag,
+                                                          Visualizer::captureSamples);
+            RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+            value = VALUE_OR_RETURN_STATUS(GET_PARAMETER_SPECIFIC_FIELD(
+                    aidlParam, Visualizer, visualizer, Visualizer::captureSamples, int32_t));
+            mCaptureSize = value;
+            return param.writeToValue(&value);
+        }
+        case VISUALIZER_PARAM_SCALING_MODE: {
+            Parameter::Id id = MAKE_SPECIFIC_PARAMETER_ID(Visualizer, visualizerTag,
+                                                          Visualizer::scalingMode);
+            RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+            Visualizer::ScalingMode mode = VALUE_OR_RETURN_STATUS(
+                    GET_PARAMETER_SPECIFIC_FIELD(aidlParam, Visualizer, visualizer,
+                                                 Visualizer::scalingMode, Visualizer::ScalingMode));
+            value = VALUE_OR_RETURN_STATUS(
+                    aidl::android::aidl2legacy_Parameter_Visualizer_ScalingMode_uint32(mode));
+            return param.writeToValue(&value);
+        }
+        case VISUALIZER_PARAM_LATENCY: {
+            Parameter::Id id = MAKE_SPECIFIC_PARAMETER_ID(Visualizer, visualizerTag,
+                                                          Visualizer::latencyMs);
+            RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+            value = (int32_t)VALUE_OR_RETURN_STATUS(GET_PARAMETER_SPECIFIC_FIELD(
+                    aidlParam, Visualizer, visualizer, Visualizer::latencyMs, int32_t));
+            return param.writeToValue(&value);
+        }
+        case VISUALIZER_PARAM_MEASUREMENT_MODE: {
+            Parameter::Id id = MAKE_SPECIFIC_PARAMETER_ID(Visualizer, visualizerTag,
+                                                          Visualizer::measurementMode);
+            RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+            Visualizer::MeasurementMode mode = VALUE_OR_RETURN_STATUS(GET_PARAMETER_SPECIFIC_FIELD(
+                    aidlParam, Visualizer, visualizer, Visualizer::measurementMode,
+                    Visualizer::MeasurementMode));
+            value = VALUE_OR_RETURN_STATUS(
+                    aidl::android::aidl2legacy_Parameter_Visualizer_MeasurementMode_uint32(mode));
+            return param.writeToValue(&value);
+        }
+        default: {
+            // TODO: implement vendor extension parameters
+            ALOGW("%s unknown param %s", __func__, param.toString().c_str());
+            return BAD_VALUE;
+        }
+    }
+}
+
+status_t AidlConversionVisualizer::visualizerCapture(uint32_t* replySize, void* pReplyData) {
+    if (!replySize || !pReplyData || *replySize != mCaptureSize) {
+        ALOGE("%s illegal param replySize %p pReplyData %p", __func__, replySize, pReplyData);
+        return BAD_VALUE;
+    }
+
+    Parameter aidlParam;
+    Parameter::Id id = MAKE_SPECIFIC_PARAMETER_ID(Visualizer, visualizerTag,
+                                                    Visualizer::captureSampleBuffer);
+    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+    const auto& samples = VALUE_OR_RETURN_STATUS(
+            GET_PARAMETER_SPECIFIC_FIELD(aidlParam, Visualizer, visualizer,
+                                         Visualizer::captureSampleBuffer, std::vector<uint8_t>));
+    size_t len = std::min((size_t)*replySize, samples.size());
+    std::memcpy(pReplyData, samples.data(), *replySize = len);
+    return OK;
+}
+
+status_t AidlConversionVisualizer::visualizerMeasure(uint32_t* replySize, void* pReplyData) {
+    if (!replySize || !pReplyData || *replySize != 2 * sizeof(int32_t)) {
+        ALOGE("%s illegal param replySize %p pReplyData %p", __func__, replySize, pReplyData);
+        return BAD_VALUE;
+    }
+
+    Parameter aidlParam;
+    Parameter::Id id =
+            MAKE_SPECIFIC_PARAMETER_ID(Visualizer, visualizerTag, Visualizer::measurement);
+    RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getParameter(id, &aidlParam)));
+    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;
+    return OK;
 }
 
 } // namespace effect
diff --git a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.h b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.h
index a7e4ea1..891c331 100644
--- a/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.h
+++ b/media/libaudiohal/impl/effectsAidlConversion/AidlConversionVisualizer.h
@@ -32,8 +32,11 @@
     ~AidlConversionVisualizer() {}
 
   private:
+    uint32_t mCaptureSize;
     status_t setParameter(utils::EffectParamReader& param) override;
     status_t getParameter(utils::EffectParamWriter& param) override;
+    status_t visualizerCapture(uint32_t* replySize, void* pReplyData) override;
+    status_t visualizerMeasure(uint32_t* replySize, void* pReplyData) override;
 };
 
 }  // namespace effect
diff --git a/media/libaudiohal/include/media/audiohal/AudioEffectUuid.h b/media/libaudiohal/include/media/audiohal/AudioEffectUuid.h
index b21e4c9..3b8076f 100644
--- a/media/libaudiohal/include/media/audiohal/AudioEffectUuid.h
+++ b/media/libaudiohal/include/media/audiohal/AudioEffectUuid.h
@@ -42,11 +42,11 @@
                                              0xa0fc,
                                              {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
 // fa81862a-588b-11ed-9b6a-0242ac120002
-static const AudioUuid kDownmixTypeUUID = {static_cast<int32_t>(0xfa81862a),
-                                           0x588b,
-                                           0x11ed,
-                                           0x9b6a,
-                                           {0x02, 0x42, 0xac, 0x12, 0x00, 0x02}};
+static const AudioUuid kDownmixTypeUUID = {static_cast<int32_t>(0x381e49cc),
+                                           0xa858,
+                                           0x4aa2,
+                                           0x87f6,
+                                           {0xe8, 0x38, 0x8e, 0x76, 0x01, 0xb2}};
 // 7261676f-6d75-7369-6364-28e2fd3ac39e
 static const AudioUuid kDynamicsProcessingTypeUUID = {static_cast<int32_t>(0x7261676f),
                                                       0x6d75,
@@ -101,12 +101,12 @@
                                                0x11db,
                                                0x8577,
                                                {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
-// fa819f3e-588b-11ed-9b6a-0242ac120002
-static const AudioUuid kVisualizerTypeUUID = {static_cast<int32_t>(0xfa819f3e),
-                                              0x588b,
-                                              0x11ed,
-                                              0x9b6a,
-                                              {0x02, 0x42, 0xac, 0x12, 0x00, 0x02}};
+// e46b26a0-dddd-11db-8afd-0002a5d5c51b
+static const AudioUuid kVisualizerTypeUUID = {static_cast<int32_t>(0xe46b26a0),
+                                              0xdddd,
+                                              0x11db,
+                                              0x8afd,
+                                              {0x00, 0x02, 0xa5, 0xd5, 0xc5, 0x1b}};
 // fa81a2b8-588b-11ed-9b6a-0242ac120002
 static const AudioUuid kVolumeTypeUUID = {static_cast<int32_t>(0xfa81a2b8),
                                           0x588b,
diff --git a/media/libaudiohal/tests/EffectsFactoryHalInterface_test.cpp b/media/libaudiohal/tests/EffectsFactoryHalInterface_test.cpp
index 71c7586..a8843d6 100644
--- a/media/libaudiohal/tests/EffectsFactoryHalInterface_test.cpp
+++ b/media/libaudiohal/tests/EffectsFactoryHalInterface_test.cpp
@@ -165,7 +165,7 @@
                                                      sizeof(int32_t) /* returnValueSize */)),
         std::make_tuple(EFFECT_UIID_DOWNMIX,
                         createEffectParamCombination(DOWNMIX_PARAM_TYPE, DOWNMIX_TYPE_FOLD,
-                                                     sizeof(int32_t) /* returnValueSize */)),
+                                                     sizeof(int16_t) /* returnValueSize */)),
         std::make_tuple(SL_IID_DYNAMICSPROCESSING,
                         createEffectParamCombination(
                                 std::array<uint32_t, 2>({DP_PARAM_INPUT_GAIN, 0 /* channel */}),
@@ -264,7 +264,8 @@
         if (mCombination->valueSize) {
             std::vector<uint8_t> response(mCombination->valueSize);
             EXPECT_EQ(OK, parameterGet.readFromValue(response.data(), mCombination->valueSize))
-                << parameterGet.toString();
+                    << " try get valueSize " << mCombination->valueSize << " from "
+                    << parameterGet.toString();
             EXPECT_EQ(response, mExpectedValue);
         }
     }
diff --git a/media/libeffects/lvm/wrapper/Reverb/aidl/EffectReverb.cpp b/media/libeffects/lvm/wrapper/Reverb/aidl/EffectReverb.cpp
index 018f3bc..e9bdf94 100644
--- a/media/libeffects/lvm/wrapper/Reverb/aidl/EffectReverb.cpp
+++ b/media/libeffects/lvm/wrapper/Reverb/aidl/EffectReverb.cpp
@@ -184,6 +184,20 @@
                     EX_ILLEGAL_ARGUMENT, "setDecayHfRatioFailed");
             return ndk::ScopedAStatus::ok();
         }
+        case EnvironmentalReverb::reflectionsLevelMb: {
+            RETURN_IF(mContext->setReflectionsLevel(
+                              erParam.get<EnvironmentalReverb::reflectionsLevelMb>()) !=
+                              RetCode::SUCCESS,
+                      EX_ILLEGAL_ARGUMENT, "setReflectionsLevelFailed");
+            return ndk::ScopedAStatus::ok();
+        }
+        case EnvironmentalReverb::reflectionsDelayMs: {
+            RETURN_IF(mContext->setReflectionsDelay(
+                              erParam.get<EnvironmentalReverb::reflectionsDelayMs>()) !=
+                              RetCode::SUCCESS,
+                      EX_ILLEGAL_ARGUMENT, "setReflectionsDelayFailed");
+            return ndk::ScopedAStatus::ok();
+        }
         case EnvironmentalReverb::levelMb: {
             RETURN_IF(mContext->setEnvironmentalReverbLevel(
                               erParam.get<EnvironmentalReverb::levelMb>()) != RetCode::SUCCESS,
@@ -292,6 +306,14 @@
                     mContext->getEnvironmentalReverbDecayHfRatio());
             break;
         }
+        case EnvironmentalReverb::reflectionsLevelMb: {
+            erParam.set<EnvironmentalReverb::reflectionsLevelMb>(mContext->getReflectionsLevel());
+            break;
+        }
+        case EnvironmentalReverb::reflectionsDelayMs: {
+            erParam.set<EnvironmentalReverb::reflectionsDelayMs>(mContext->getReflectionsDelay());
+            break;
+        }
         case EnvironmentalReverb::levelMb: {
             erParam.set<EnvironmentalReverb::levelMb>(mContext->getEnvironmentalReverbLevel());
             break;
diff --git a/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h b/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h
index af49a25..9bb0b1a 100644
--- a/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h
+++ b/media/libeffects/lvm/wrapper/Reverb/aidl/ReverbContext.h
@@ -83,6 +83,18 @@
     RetCode setVolumeStereo(const Parameter::VolumeStereo& volumeStereo) override;
     Parameter::VolumeStereo getVolumeStereo() override { return mVolumeStereo; }
 
+    RetCode setReflectionsDelay(int delay) {
+        mReflectionsDelayMs = delay;
+        return RetCode::SUCCESS;
+    }
+    bool getReflectionsDelay() const { return mReflectionsDelayMs; }
+
+    RetCode setReflectionsLevel(int level) {
+        mReflectionsLevelMb = level;
+        return RetCode::SUCCESS;
+    }
+    bool getReflectionsLevel() const { return mReflectionsLevelMb; }
+
     IEffect::Status lvmProcess(float* in, float* out, int samples);
 
   private:
@@ -146,15 +158,17 @@
     bool mEnabled = false;
     LVREV_Handle_t mInstance GUARDED_BY(mMutex);
 
-    int mRoomLevel;
-    int mRoomHfLevel;
-    int mDecayTime;
-    int mDecayHfRatio;
-    int mLevel;
-    int mDelay;
-    int mDiffusion;
-    int mDensity;
-    bool mBypass;
+    int mRoomLevel = 0;
+    int mRoomHfLevel = 0;
+    int mDecayTime = 0;
+    int mDecayHfRatio = 0;
+    int mLevel = 0;
+    int mDelay = 0;
+    int mDiffusion = 0;
+    int mDensity = 0;
+    bool mBypass = 0;
+    int mReflectionsLevelMb = 0;
+    int mReflectionsDelayMs = 0;
 
     PresetReverb::Presets mPreset;
     PresetReverb::Presets mNextPreset;
diff --git a/media/libmedia/MediaProfiles.cpp b/media/libmedia/MediaProfiles.cpp
index 85768bd..5aa9adc 100644
--- a/media/libmedia/MediaProfiles.cpp
+++ b/media/libmedia/MediaProfiles.cpp
@@ -898,10 +898,9 @@
         }
     }
 
-    for (size_t cameraId = 0; cameraId < mCameraIds.size(); ++cameraId) {
+    for (size_t refIndex = 0; refIndex < mCameraIds.size(); ++refIndex) {
+        const int cameraId = mCameraIds[refIndex];
         for (size_t j = 0; j < kNumRequiredProfiles; ++j) {
-            int refIndex = getRequiredProfileRefIndex(cameraId);
-            CHECK(refIndex != -1);
             RequiredProfileRefInfo *info =
                     &mRequiredProfileRefs[refIndex].mRefs[j];
 
@@ -931,14 +930,14 @@
 
                 int index = getCamcorderProfileIndex(cameraId, profile->mQuality);
                 if (index != -1) {
-                    ALOGV("Profile quality %d for camera %zu already exists",
+                    ALOGV("Profile quality %d for camera %d already exists",
                         profile->mQuality, cameraId);
                     CHECK(index == refIndex);
                     continue;
                 }
 
                 // Insert the new profile
-                ALOGV("Add a profile: quality %d=>%d for camera %zu",
+                ALOGV("Add a profile: quality %d=>%d for camera %d",
                         mCamcorderProfiles[info->mRefProfileIndex]->mQuality,
                         profile->mQuality, cameraId);
 
diff --git a/media/module/extractors/ogg/OggExtractor.cpp b/media/module/extractors/ogg/OggExtractor.cpp
index 1c6f516..4c106b2 100644
--- a/media/module/extractors/ogg/OggExtractor.cpp
+++ b/media/module/extractors/ogg/OggExtractor.cpp
@@ -34,6 +34,9 @@
 #include <system/audio.h>
 #include <utils/String8.h>
 
+#include <inttypes.h>
+#include <stdint.h>
+
 extern "C" {
     #include <Tremolo/codec_internal.h>
 
@@ -346,66 +349,118 @@
         off64_t startOffset, off64_t *pageOffset) {
     *pageOffset = startOffset;
 
-    for (;;) {
-        char signature[4];
-        ssize_t n = mSource->readAt(*pageOffset, &signature, 4);
+    // balance between larger reads and reducing how much we over-read.
+    const int FIND_BUF_SIZE = 2048;
+    const int lenOggS = strlen("OggS");
+    while(1) {
 
-        if (n < 4) {
+        // work with big buffers to amortize readAt() costs
+        char signatureBuffer[FIND_BUF_SIZE];
+        ssize_t n = mSource->readAt(*pageOffset, &signatureBuffer, sizeof(signatureBuffer));
+
+        if (n < lenOggS) {
             *pageOffset = 0;
-
             return (n < 0) ? n : (status_t)ERROR_END_OF_STREAM;
         }
 
-        if (!memcmp(signature, "OggS", 4)) {
-            if (*pageOffset > startOffset) {
-                ALOGV("skipped %lld bytes of junk to reach next frame",
-                     (long long)(*pageOffset - startOffset));
-            }
-
-            return OK;
-        }
-
-        // see how far ahead to skip; avoid some fruitless comparisons
-        unsigned int i;
-        for (i = 1; i < 4 ; i++) {
-            if (signature[i] == 'O')
+        for(int i = 0; i < n - (lenOggS - 1) ; i++) {
+            // fast scan for 1st character in a signature
+            char *p = (char *)memchr(&signatureBuffer[i], 'O', n - (lenOggS - 1) - i);
+            if (p == NULL) {
+                // no signature start in the rest of this buffer.
                 break;
+            }
+            int jump = (p-&signatureBuffer[i]);
+            i += jump;
+            if (memcmp("OggS", &signatureBuffer[i], lenOggS) == 0) {
+                *pageOffset += i;
+                if (*pageOffset > startOffset) {
+                    ALOGD("skipped %" PRIu64 " bytes of junk to reach next frame",
+                         (*pageOffset - startOffset));
+                }
+                return OK;
+            }
         }
-        *pageOffset += i;
+
+        // on to next block. buffer didn't end with "OggS", but could end with "Ogg".
+        // overlap enough to detect this. n >= lenOggS, so this always advances.
+        *pageOffset += n - (lenOggS - 1);
     }
+    return (status_t)ERROR_END_OF_STREAM;
 }
 
 // Given the offset of the "current" page, find the page immediately preceding
 // it (if any) and return its granule position.
 // To do this we back up from the "current" page's offset until we find any
 // page preceding it and then scan forward to just before the current page.
+//
 status_t MyOggExtractor::findPrevGranulePosition(
         off64_t pageOffset, uint64_t *granulePos) {
     *granulePos = 0;
 
-    off64_t prevPageOffset = 0;
-    off64_t prevGuess = pageOffset;
-    for (;;) {
-        if (prevGuess >= 5000) {
-            prevGuess -= 5000;
+    const int FIND_BUF_SIZE = 2048;
+    const int lenOggS = strlen("OggS");
+
+    if (pageOffset == 0) {
+        ALOGV("no page before the first page");
+        return UNKNOWN_ERROR;
+    }
+
+    off64_t prevPageOffset = pageOffset;
+
+    // we start our search on the byte immediately in front of pageOffset
+    // which could mean "O" immediately before and "ggS" starting at pageOffset
+    //
+    // if there was an "OggS" at pageOffset, we'll have scanned a few extra bytes
+    // but if pageOffset was chosen by a seek operation, we don't know that it
+    // reflects the beginning of a page. By choosing to scan 3 possibly unneeded
+    // bytes at the start we cover both cases.
+    //
+    off64_t firstAfter = pageOffset + lenOggS - 1;    // NOT within our buffer
+    off64_t nextOffset = pageOffset;
+
+    while(prevPageOffset == pageOffset) {
+        // work with big buffers to amortize readAt() costs
+        char signatureBuffer[FIND_BUF_SIZE];
+
+        ssize_t desired = sizeof(signatureBuffer);
+        if (firstAfter >= desired) {
+            nextOffset = firstAfter - desired;
         } else {
-            prevGuess = 0;
+            nextOffset = 0;
+            desired = firstAfter;
         }
+        ssize_t n = mSource->readAt(nextOffset, &signatureBuffer, desired);
 
-        ALOGV("backing up %lld bytes", (long long)(pageOffset - prevGuess));
-
-        status_t err = findNextPage(prevGuess, &prevPageOffset);
-        if (err == ERROR_END_OF_STREAM) {
-            // We are at the last page and didn't back off enough;
-            // back off 5000 bytes more and try again.
-            continue;
-        } else if (err != OK) {
-            return err;
-        }
-
-        if (prevPageOffset < pageOffset || prevGuess == 0) {
+        if (n < lenOggS) {
+            ALOGD("short read, get out");
             break;
         }
+
+        // work backwards
+        // loop control ok for n >= 0
+        for(int i = n - lenOggS; i >= 0 ; i--) {
+            // fast scan for 1st character in the signature
+            char *p = (char *)memrchr(&signatureBuffer[0], 'O', i);
+            if (p == NULL) {
+                // no signature start in the rest of this buffer.
+                break;
+            }
+            i = (p-&signatureBuffer[0]);
+            // loop start chosen to ensure we will always have lenOggS bytes
+            if (memcmp("OggS", &signatureBuffer[i], lenOggS) == 0) {
+                prevPageOffset = nextOffset + i;
+                break;
+            }
+        }
+
+        // back up for next read; make sure we catch overlaps
+        if (nextOffset == 0) {
+            // can't back up any further
+            break;
+        }
+        // current buffer might start with "ggS", include those bytes in the next iteration
+        firstAfter = nextOffset + lenOggS - 1;
     }
 
     if (prevPageOffset == pageOffset) {
@@ -413,8 +468,8 @@
         return UNKNOWN_ERROR;
     }
 
-    ALOGV("prevPageOffset at %lld, pageOffset at %lld",
-            (long long)prevPageOffset, (long long)pageOffset);
+    ALOGV("prevPageOffset at %" PRIu64 ", pageOffset at %" PRIu64,
+          prevPageOffset, pageOffset);
     uint8_t flag = 0;
     for (;;) {
         Page prevPage;
@@ -993,16 +1048,21 @@
     size_t numerator = mTableOfContents.size();
 
     if (numerator > kMaxNumTOCEntries) {
-        size_t denom = numerator - kMaxNumTOCEntries;
+        Vector<TOCEntry> maxTOC;
+        maxTOC.setCapacity(kMaxNumTOCEntries);
 
+        size_t denom = numerator - kMaxNumTOCEntries;
         size_t accum = 0;
-        for (ssize_t i = mTableOfContents.size(); i > 0; --i) {
+        for (ssize_t i = 0; i < mTableOfContents.size(); i++) {
             accum += denom;
             if (accum >= numerator) {
-                mTableOfContents.removeAt(i);
                 accum -= numerator;
+            } else {
+                maxTOC.push(mTableOfContents.itemAt(i));
             }
         }
+
+        mTableOfContents = maxTOC;
     }
 }