Merge "Support effect destroy at any state" into main (3d4cc3ba) · Commits · e / os / android_hardware_interfaces

audio/aidl/android/hardware/audio/effect/IFactory.aidl

+8 −3

Original line number	Diff line number	Diff line
		@@ -74,13 +74,18 @@ interface IFactory {

		/**
		* Called by the framework to destroy the effect and free up all currently allocated resources.
		* It is recommended to destroy the effect from the client side as soon as it is becomes unused.
		* This method can be called at any time to destroy an effect instance. It is recommended to
		* destroy the effect from the client side as soon as it becomes unused to free up resources.
		*
		* The client must ensure effect instance is closed before destroy.
		* The effect instance must handle any necessary cleanup and resource deallocation.
		* If the effect is in the PROCESSING or DRAINING state, it must gracefully stop
		* processing before destruction.
		* The effect must ensure that all internal states are properly cleaned up to prevent resource
		* leaks.
		*
		* @param handle The handle of effect instance to be destroyed.
		* @throws EX_ILLEGAL_ARGUMENT if the effect handle is not valid.
		* @throws EX_ILLEGAL_STATE if the effect instance is not in a proper state to be destroyed.
		* @throws EX_ILLEGAL_STATE if the effect instance can not be destroyed.
		*/
		void destroyEffect(in IEffect handle);
		}

audio/aidl/android/hardware/audio/effect/State.aidl

+5 −0

Original line number	Diff line number	Diff line
		@@ -24,6 +24,8 @@ package android.hardware.audio.effect;
		* it should transfer to IDLE state after handle the command successfully. Effect instance should
		* consume minimal resource and transfer to INIT state after it was close().
		*
		* An effect instance can be destroyed from any state using `IFactory.destroyEffect()`.
		*
		* Refer to the state machine diagram `state.gv` for a detailed state diagram.
		*/
		@VintfStability
		@@ -66,6 +68,7 @@ enum State {
		* - Transitions to INIT on `IEffect.close()`.
		* - Remains in IDLE on `IEffect.getParameter()`, `IEffect.setParameter()`,
		* `IEffect.getDescriptor()`, `IEffect.command(CommandId.RESET)`, and `IEffect.reopen()`.
		* - Transitions to the final state on `IFactory.destroyEffect()`.
		*/
		IDLE,

		@@ -98,6 +101,7 @@ enum State {
		* stop processing with `CommandId.STOP` before closing.
		* - If `IEffect.close()` is called in this state, the effect instance should stop processing,
		* transition to IDLE, and then close.
		* - Transitions to the final state on `IFactory.destroyEffect()`.
		*/
		PROCESSING,

		@@ -123,6 +127,7 @@ enum State {
		* - If not implemented, the effect instance may transition directly from PROCESSING to
		* IDLE without this intermediate state.
		* - Any `CommandId.STOP` commands received during DRAINING should be ignored.
		* - Transitions to the final state on `IFactory.destroyEffect()`.
		*/
		DRAINING,
		}

audio/aidl/android/hardware/audio/effect/state.gv

+19 −29

Original line number	Diff line number	Diff line
		@@ -13,8 +13,8 @@
		* See the License for the specific language governing permissions and
		* limitations under the License.
		*/
		// To render: "dot -Tpng state.gv -o state.png"

		// To render: "dot -Tpng state.gv -o state.png"
		digraph effect_state_machine {

		rankdir=LR; // Left to Right layout
		@@ -29,40 +29,30 @@ digraph effect_state_machine {
		I [shape=point, fillcolor=black, width=0.2];

		// Final state node
		F [shape=doublecircle, fillcolor=white, width=0.2];
		F [shape=doublecircle, label="Destroyed"];

		// Define other nodes with colors
		INIT [shape=ellipse, fillcolor=lightgreen];
		IDLE [shape=ellipse, fillcolor=lightblue];
		PROCESSING [shape=ellipse, fillcolor=lightyellow];
		DRAINING [shape=ellipse, fillcolor=lightgrey];
		INIT [shape=ellipse, fillcolor=lightgreen, label="INIT"];
		IDLE [shape=ellipse, fillcolor=lightblue, label="IDLE"];
		PROCESSING [shape=ellipse, fillcolor=lightyellow, label="PROCESSING"];
		DRAINING [shape=ellipse, fillcolor=lightgrey, label="DRAINING"];
		ANY_STATE [shape=ellipse, style=dashed, label="Any State", fillcolor=white];

		// Transitions
		// Main transitions
		I -> INIT [label="IFactory.createEffect", fontcolor="navy"];

		INIT -> F [label="IFactory.destroyEffect"];

		INIT -> IDLE [label="IEffect.open()", fontcolor="lime"];

		IDLE -> PROCESSING [label="IEffect.command(START)"];

		PROCESSING -> IDLE [label="IEffect.command(STOP)\nIEffect.command(RESET)"];

		PROCESSING -> DRAINING [label="IEffect.command(STOP)", fontcolor="orange"];

		DRAINING -> IDLE [label="Draining complete\n(IEffect.command(RESET)\nautomatic)"];

		DRAINING -> PROCESSING [label="IEffect.command(START)\n(Interrupt draining)"];

		PROCESSING -> IDLE [label="IEffect.command(STOP) (if draining not required)\nIEffect.command(RESET)"];
		PROCESSING -> DRAINING [label="IEffect.command(STOP) (if draining required)", fontcolor="orange"];
		DRAINING -> IDLE [label="IEffect.command(RESET)\nDraining complete (automatic transition)"];
		DRAINING -> PROCESSING [label="IEffect.command(START) (Interrupt draining)"];
		IDLE -> INIT [label="IEffect.close()"];

		// Self-loops
		INIT -> INIT [label="IEffect.getState\nIEffect.getDescriptor"];

		IDLE -> IDLE [label="IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.command(RESET)\nIEffect.reopen"];

		PROCESSING -> PROCESSING [label="IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.reopen"];

		DRAINING -> DRAINING [label="IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.reopen\nFading"];

		// Global transitions
		subgraph cluster_global_transitions {
		label="Global Transitions (Any State)";
		style=dashed;
		ANY_STATE -> F [label="IFactory.destroyEffect", style="bold"];
		ANY_STATE -> ANY_STATE [label="IEffect.getParameter\nIEffect.setParameter\nIEffect.getDescriptor\nIEffect.getState\nIEffect.reopen", fontsize=10];
		}
		}

audio/aidl/default/EffectImpl.cpp

+38 −3

Original line number	Diff line number	Diff line
		@@ -22,7 +22,10 @@
		#include "effect-impl/EffectTypes.h"
		#include "include/effect-impl/EffectTypes.h"

		using aidl::android::hardware::audio::effect::CommandId;
		using aidl::android::hardware::audio::effect::Descriptor;
		using aidl::android::hardware::audio::effect::IEffect;
		using aidl::android::hardware::audio::effect::kDestroyAnyStateSupportedVersion;
		using aidl::android::hardware::audio::effect::kEventFlagDataMqNotEmpty;
		using aidl::android::hardware::audio::effect::kEventFlagNotEmpty;
		using aidl::android::hardware::audio::effect::kReopenSupportedVersion;
		@@ -31,13 +34,45 @@ using aidl::android::media::audio::common::PcmType;
		using ::android::hardware::EventFlag;

		extern "C" binder_exception_t destroyEffect(const std::shared_ptr<IEffect>& instanceSp) {
		State state;
		ndk::ScopedAStatus status = instanceSp->getState(&state);
		if (!status.isOk() \|\| State::INIT != state) {
		if (!instanceSp) {
		LOG(ERROR) << __func__ << " nullptr";
		return EX_ILLEGAL_ARGUMENT;
		}

		Descriptor desc;
		ndk::ScopedAStatus status = instanceSp->getDescriptor(&desc);
		if (!status.isOk()) {
		LOG(ERROR) << __func__ << " instance " << instanceSp.get()
		<< " failed to get descriptor, status: " << status.getDescription();
		return EX_ILLEGAL_STATE;
		}

		State state;
		status = instanceSp->getState(&state);
		if (!status.isOk()) {
		LOG(ERROR) << __func__ << " " << desc.common.name << " instance " << instanceSp.get()
		<< " in state: " << toString(state) << ", status: " << status.getDescription();
		return EX_ILLEGAL_STATE;
		}

		int effectVersion = 0;
		if (!instanceSp->getInterfaceVersion(&effectVersion).isOk()) {
		LOG(WARNING) << __func__ << " " << desc.common.name << " failed to get interface version";
		}

		if (effectVersion < kDestroyAnyStateSupportedVersion) {
		if (State::INIT != state) {
		LOG(ERROR) << __func__ << " " << desc.common.name << " can not destroy instance "
		<< instanceSp.get() << " in state: " << toString(state);
		return EX_ILLEGAL_STATE;
		}
		} else {
		instanceSp->command(CommandId::RESET);
		instanceSp->close();
		}

		LOG(DEBUG) << __func__ << " " << desc.common.name << " instance " << instanceSp.get()
		<< " destroyed";
		return EX_NONE;
		}

audio/aidl/vts/VtsHalAudioEffectTargetTest.cpp

+24 −12

Original line number	Diff line number	Diff line
		@@ -42,6 +42,7 @@ using aidl::android::hardware::audio::effect::Descriptor;
		using aidl::android::hardware::audio::effect::Flags;
		using aidl::android::hardware::audio::effect::IEffect;
		using aidl::android::hardware::audio::effect::IFactory;
		using aidl::android::hardware::audio::effect::kDestroyAnyStateSupportedVersion;
		using aidl::android::hardware::audio::effect::Parameter;
		using aidl::android::hardware::audio::effect::State;
		using aidl::android::media::audio::common::AudioDeviceDescription;
		@@ -316,28 +317,39 @@ TEST_P(AudioEffectTest, CloseProcessingStateEffects) {
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect));
		}

		// Expect EX_ILLEGAL_STATE if the effect instance is not in a proper state to be destroyed.
		// Expect EX_ILLEGAL_STATE if the effect instance is not in a proper state to be destroyed before
		// `kDestroyAnyStateSupportedVersion`.
		// For any version after `kDestroyAnyStateSupportedVersion`, expect `destroy` to always success.
		TEST_P(AudioEffectTest, DestroyOpenEffects) {
		ASSERT_NO_FATAL_FAILURE(create(mFactory, mEffect, mDescriptor));
		ASSERT_NO_FATAL_FAILURE(open(mEffect));
		if (mVersion < kDestroyAnyStateSupportedVersion) {
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect, EX_ILLEGAL_STATE));

		// cleanup
		ASSERT_NO_FATAL_FAILURE(close(mEffect));
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect));
		} else {
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect));
		}
		}

		// Expect EX_ILLEGAL_STATE if the effect instance is not in a proper state to be destroyed.
		// Expect EX_ILLEGAL_STATE if the effect instance is not in a proper state to be destroyed before
		// `kDestroyAnyStateSupportedVersion`.
		// For any version after `kDestroyAnyStateSupportedVersion`, expect `destroy` to always success.
		TEST_P(AudioEffectTest, DestroyProcessingEffects) {
		ASSERT_NO_FATAL_FAILURE(create(mFactory, mEffect, mDescriptor));
		ASSERT_NO_FATAL_FAILURE(open(mEffect));
		ASSERT_NO_FATAL_FAILURE(command(mEffect, CommandId::START));
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect, EX_ILLEGAL_STATE));

		if (mVersion < kDestroyAnyStateSupportedVersion) {
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect, EX_ILLEGAL_STATE));
		// cleanup
		ASSERT_NO_FATAL_FAILURE(command(mEffect, CommandId::STOP));
		ASSERT_NO_FATAL_FAILURE(close(mEffect));
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect));
		} else {
		ASSERT_NO_FATAL_FAILURE(destroy(mFactory, mEffect));
		}
		}

		TEST_P(AudioEffectTest, NormalSequenceStates) {