From 04432a6405d1f3a15486fdb1fa7f1649539a0367 Mon Sep 17 00:00:00 2001 From: Edgar Arriaga Date: Wed, 24 Aug 2022 11:01:46 -0700 Subject: Fix to allow setting URI without recreating ringtone This patch splits the behavior of creating the media player and the uri setter and avoids it from being recreated upon being retrieved by getRingtone which harms performance causing a double asset load. Bug: 240621827 Test: Receive a phone call and verify ringtone works Change-Id: I91563ddcc94f25911937e9fbb35ff36bbe9d3012 --- media/java/android/media/Ringtone.java | 110 ++++++++++++++------- media/java/android/media/RingtoneManager.java | 58 +++++++++-- .../com/android/systemui/media/RingtonePlayer.java | 3 +- 3 files changed, 123 insertions(+), 48 deletions(-) diff --git a/media/java/android/media/Ringtone.java b/media/java/android/media/Ringtone.java index 86a94a9e0662..82c3139bb4d3 100644 --- a/media/java/android/media/Ringtone.java +++ b/media/java/android/media/Ringtone.java @@ -29,11 +29,12 @@ import android.net.Uri; import android.os.Binder; import android.os.Build; import android.os.RemoteException; +import android.os.Trace; import android.provider.MediaStore; import android.provider.MediaStore.MediaColumns; import android.provider.Settings; import android.util.Log; - +import com.android.internal.annotations.VisibleForTesting; import java.io.IOException; import java.util.ArrayList; @@ -136,13 +137,73 @@ public class Ringtone { */ public void setAudioAttributes(AudioAttributes attributes) throws IllegalArgumentException { + setAudioAttributesField(attributes); + // The audio attributes have to be set before the media player is prepared. + // Re-initialize it. + setUri(mUri, mVolumeShaperConfig); + createLocalMediaPlayer(); + } + + /** + * Same as {@link #setAudioAttributes(AudioAttributes)} except this one does not create + * the media player. + * @hide + */ + public void setAudioAttributesField(@Nullable AudioAttributes attributes) { if (attributes == null) { throw new IllegalArgumentException("Invalid null AudioAttributes for Ringtone"); } mAudioAttributes = attributes; - // The audio attributes have to be set before the media player is prepared. - // Re-initialize it. - setUri(mUri, mVolumeShaperConfig); + } + + /** + * Creates a local media player for the ringtone using currently set attributes. + * @hide + */ + public void createLocalMediaPlayer() { + Trace.beginSection("createLocalMediaPlayer"); + if (mUri == null) { + Log.e(TAG, "Could not create media player as no URI was provided."); + return; + } + destroyLocalPlayer(); + // try opening uri locally before delegating to remote player + mLocalPlayer = new MediaPlayer(); + try { + mLocalPlayer.setDataSource(mContext, mUri); + mLocalPlayer.setAudioAttributes(mAudioAttributes); + synchronized (mPlaybackSettingsLock) { + applyPlaybackProperties_sync(); + } + if (mVolumeShaperConfig != null) { + mVolumeShaper = mLocalPlayer.createVolumeShaper(mVolumeShaperConfig); + } + mLocalPlayer.prepare(); + + } catch (SecurityException | IOException e) { + destroyLocalPlayer(); + if (!mAllowRemote) { + Log.w(TAG, "Remote playback not allowed: " + e); + } + } + + if (LOGD) { + if (mLocalPlayer != null) { + Log.d(TAG, "Successfully created local player"); + } else { + Log.d(TAG, "Problem opening; delegating to remote player"); + } + } + Trace.endSection(); + } + + /** + * Returns whether a local player has been created for this ringtone. + * @hide + */ + @VisibleForTesting + public boolean hasLocalPlayer() { + return mLocalPlayer != null; } /** @@ -336,8 +397,7 @@ public class Ringtone { } /** - * Set {@link Uri} to be used for ringtone playback. Attempts to open - * locally, otherwise will delegate playback to remote + * Set {@link Uri} to be used for ringtone playback. * {@link IRingtonePlayer}. * * @hide @@ -347,6 +407,13 @@ public class Ringtone { setUri(uri, null); } + /** + * @hide + */ + public void setVolumeShaperConfig(@Nullable VolumeShaper.Configuration volumeShaperConfig) { + mVolumeShaperConfig = volumeShaperConfig; + } + /** * Set {@link Uri} to be used for ringtone playback. Attempts to open * locally, otherwise will delegate playback to remote @@ -356,41 +423,10 @@ public class Ringtone { */ public void setUri(Uri uri, @Nullable VolumeShaper.Configuration volumeShaperConfig) { mVolumeShaperConfig = volumeShaperConfig; - destroyLocalPlayer(); mUri = uri; if (mUri == null) { - return; - } - - // TODO: detect READ_EXTERNAL and specific content provider case, instead of relying on throwing - - // try opening uri locally before delegating to remote player - mLocalPlayer = new MediaPlayer(); - try { - mLocalPlayer.setDataSource(mContext, mUri); - mLocalPlayer.setAudioAttributes(mAudioAttributes); - synchronized (mPlaybackSettingsLock) { - applyPlaybackProperties_sync(); - } - if (mVolumeShaperConfig != null) { - mVolumeShaper = mLocalPlayer.createVolumeShaper(mVolumeShaperConfig); - } - mLocalPlayer.prepare(); - - } catch (SecurityException | IOException e) { destroyLocalPlayer(); - if (!mAllowRemote) { - Log.w(TAG, "Remote playback not allowed: " + e); - } - } - - if (LOGD) { - if (mLocalPlayer != null) { - Log.d(TAG, "Successfully created local player"); - } else { - Log.d(TAG, "Problem opening; delegating to remote player"); - } } } diff --git a/media/java/android/media/RingtoneManager.java b/media/java/android/media/RingtoneManager.java index 27727699d05c..27db41cb9f4e 100644 --- a/media/java/android/media/RingtoneManager.java +++ b/media/java/android/media/RingtoneManager.java @@ -480,8 +480,9 @@ public class RingtoneManager { if (mStopPreviousRingtone && mPreviousRingtone != null) { mPreviousRingtone.stop(); } - - mPreviousRingtone = getRingtone(mContext, getRingtoneUri(position), inferStreamType()); + + mPreviousRingtone = + getRingtone(mContext, getRingtoneUri(position), inferStreamType(), true); return mPreviousRingtone; } @@ -677,7 +678,7 @@ public class RingtoneManager { */ public static Ringtone getRingtone(final Context context, Uri ringtoneUri) { // Don't set the stream type - return getRingtone(context, ringtoneUri, -1); + return getRingtone(context, ringtoneUri, -1, true); } /** @@ -698,7 +699,34 @@ public class RingtoneManager { final Context context, Uri ringtoneUri, @Nullable VolumeShaper.Configuration volumeShaperConfig) { // Don't set the stream type - return getRingtone(context, ringtoneUri, -1 /* streamType */, volumeShaperConfig); + return getRingtone(context, ringtoneUri, -1 /* streamType */, volumeShaperConfig, true); + } + + /** + * @hide + */ + public static Ringtone getRingtone(final Context context, Uri ringtoneUri, + @Nullable VolumeShaper.Configuration volumeShaperConfig, + boolean createLocalMediaPlayer) { + // Don't set the stream type + return getRingtone(context, ringtoneUri, -1 /* streamType */, volumeShaperConfig, + createLocalMediaPlayer); + } + + /** + * @hide + */ + public static Ringtone getRingtone(final Context context, Uri ringtoneUri, + @Nullable VolumeShaper.Configuration volumeShaperConfig, + AudioAttributes audioAttributes) { + // Don't set the stream type + Ringtone ringtone = + getRingtone(context, ringtoneUri, -1 /* streamType */, volumeShaperConfig, false); + if (ringtone != null) { + ringtone.setAudioAttributesField(audioAttributes); + ringtone.createLocalMediaPlayer(); + } + return ringtone; } //FIXME bypass the notion of stream types within the class @@ -707,14 +735,19 @@ public class RingtoneManager { * type. Normally, if you change the stream type on the returned * {@link Ringtone}, it will re-create the {@link MediaPlayer}. This is just * an optimized route to avoid that. - * + * * @param streamType The stream type for the ringtone, or -1 if it should * not be set (and the default used instead). + * @param createLocalMediaPlayer when true, the ringtone returned will be fully + * created otherwise, it will require the caller to create the media player manually + * {@link Ringtone#createLocalMediaPlayer()} in order to play the Ringtone. * @see #getRingtone(Context, Uri) */ @UnsupportedAppUsage - private static Ringtone getRingtone(final Context context, Uri ringtoneUri, int streamType) { - return getRingtone(context, ringtoneUri, streamType, null /* volumeShaperConfig */); + private static Ringtone getRingtone(final Context context, Uri ringtoneUri, int streamType, + boolean createLocalMediaPlayer) { + return getRingtone(context, ringtoneUri, streamType, null /* volumeShaperConfig */, + createLocalMediaPlayer); } //FIXME bypass the notion of stream types within the class @@ -730,16 +763,21 @@ public class RingtoneManager { * @see #getRingtone(Context, Uri) */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) - private static Ringtone getRingtone( - final Context context, Uri ringtoneUri, int streamType, - @Nullable VolumeShaper.Configuration volumeShaperConfig) { + private static Ringtone getRingtone(final Context context, Uri ringtoneUri, int streamType, + @Nullable VolumeShaper.Configuration volumeShaperConfig, + boolean createLocalMediaPlayer) { try { final Ringtone r = new Ringtone(context, true); if (streamType >= 0) { //FIXME deprecated call r.setStreamType(streamType); } + + r.setVolumeShaperConfig(volumeShaperConfig); r.setUri(ringtoneUri, volumeShaperConfig); + if (createLocalMediaPlayer) { + r.createLocalMediaPlayer(); + } return r; } catch (Exception ex) { Log.e(TAG, "Failed to open ringtone " + ringtoneUri + ": " + ex); diff --git a/packages/SystemUI/src/com/android/systemui/media/RingtonePlayer.java b/packages/SystemUI/src/com/android/systemui/media/RingtonePlayer.java index 75fa2f181d7c..0b9b32b0d7d7 100644 --- a/packages/SystemUI/src/com/android/systemui/media/RingtonePlayer.java +++ b/packages/SystemUI/src/com/android/systemui/media/RingtonePlayer.java @@ -96,8 +96,9 @@ public class RingtonePlayer extends CoreStartable { mToken = token; mRingtone = new Ringtone(getContextForUser(user), false); - mRingtone.setAudioAttributes(aa); + mRingtone.setAudioAttributesField(aa); mRingtone.setUri(uri, volumeShaperConfig); + mRingtone.createLocalMediaPlayer(); } @Override -- cgit v1.2.3-59-g8ed1b