From 81b0519f04c9b378403fff9f399a2d085eecbf7d Mon Sep 17 00:00:00 2001 From: Francois Gaffie Date: Tue, 20 Apr 2021 08:59:46 +0200 Subject: [PATCH 1/2] AudioSystem: startAudioSource: ensure port handle initialized In case of failure, port handle reported is not initialized by JNI layer. Any failing call to native AudioSystem::startAudioSource will not set the port handle, but returning an error status. JNI checks for port handle validity rather than error status. This CL ensures JNI checks first status to report an error, otherwise asserts on handle validity. Bug: 186088557 Test: make Signed-off-by: Francois Gaffie Change-Id: I4eb8046a1263aeb7d464d35a360a8adb77c4b1a5 --- core/jni/android_media_AudioSystem.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/jni/android_media_AudioSystem.cpp b/core/jni/android_media_AudioSystem.cpp index 7c4c97036bd83..5eb1e00947254 100644 --- a/core/jni/android_media_AudioSystem.cpp +++ b/core/jni/android_media_AudioSystem.cpp @@ -1930,7 +1930,11 @@ android_media_AudioSystem_startAudioSource(JNIEnv *env, jobject clazz, audio_port_handle_t handle; status_t status = AudioSystem::startAudioSource(&nAudioPortConfig, paa.get(), &handle); ALOGV("AudioSystem::startAudioSource() returned %d handle %d", status, handle); - return handle > 0 ? handle : nativeToJavaStatus(status); + if (status != NO_ERROR) { + return nativeToJavaStatus(status); + } + ALOG_ASSERT(handle > 0, "%s: invalid handle reported on successful call", __func__); + return handle; } static jint From 5253a31c7adad9b07f0a8c3b59f59fb91e97674f Mon Sep 17 00:00:00 2001 From: Francois Gaffie Date: Wed, 21 Apr 2021 08:34:02 +0200 Subject: [PATCH 2/2] HwAudioSource: wrong native handle check Playing status of audio source is checked using the native handler not 0. However, if AudioSystem#startAudioSource fails, the native handler returned is not 0, but an error code. In fact AudioPolicyManager sets the PORT to NONE (0) and sets an error status. JNI layer transforms the failure status into a native handler returned to the caller. Bug: 186088557 Test: make Signed-off-by: Francois Gaffie Change-Id: Iaa920c073885c2556a93a1b5ae23467fa6ffad4f --- media/java/android/media/HwAudioSource.java | 25 ++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/media/java/android/media/HwAudioSource.java b/media/java/android/media/HwAudioSource.java index e339ae8ef7065..167ab6535843f 100644 --- a/media/java/android/media/HwAudioSource.java +++ b/media/java/android/media/HwAudioSource.java @@ -37,7 +37,13 @@ public class HwAudioSource extends PlayerBase { private final AudioDeviceInfo mAudioDeviceInfo; private final AudioAttributes mAudioAttributes; - private int mNativeHandle; + /** + * The value of the native handle encodes the HwAudioSource state. + * The native handle returned by {@link AudioSystem#startAudioSource} is either valid + * (aka > 0, so successfully started) or hosting an error code (negative). + * 0 corresponds to an untialized or stopped HwAudioSource. + */ + private int mNativeHandle = 0; /** * Class constructor for a hardware audio source based player. @@ -129,14 +135,18 @@ public class HwAudioSource extends PlayerBase { /** * Starts the playback from {@link AudioDeviceInfo}. + * Starts does not return any error code, caller must check {@link HwAudioSource#isPlaying} to + * ensure the state of the HwAudioSource encoded in {@link mNativeHandle}. */ public void start() { Preconditions.checkState(!isPlaying(), "HwAudioSource is currently playing"); mNativeHandle = AudioSystem.startAudioSource( mAudioDeviceInfo.getPort().activeConfig(), mAudioAttributes); - // FIXME: b/174876389 clean up device id reporting - baseStart(getDeviceId()); + if (isPlaying()) { + // FIXME: b/174876389 clean up device id reporting + baseStart(getDeviceId()); + } } private int getDeviceId() { @@ -162,18 +172,23 @@ public class HwAudioSource extends PlayerBase { /** * Checks whether the HwAudioSource player is playing. + * It checks the state of the HwAudioSource encoded in {@link HwAudioSource#isPlaying}. + * 0 corresponds to a stopped or uninitialized HwAudioSource. + * Negative value corresponds to a status reported by {@link AudioSystem#startAudioSource} to + * indicate a failure when trying to start the HwAudioSource. + * * @return true if currently playing, false otherwise */ public boolean isPlaying() { - return mNativeHandle != 0; + return mNativeHandle > 0; } /** * Stops the playback from {@link AudioDeviceInfo}. */ public void stop() { - baseStop(); if (mNativeHandle > 0) { + baseStop(); AudioSystem.stopAudioSource(mNativeHandle); mNativeHandle = 0; }