From 94b256960c2e022fe40d8d5b00d1c16d5e0cde71 Mon Sep 17 00:00:00 2001
From: Etienne Ruffieux <eruffieux@google.com>
Date: Thu, 24 Aug 2023 02:01:09 -0700
Subject: [PATCH] Fix null playstate update from players.
Some players will send a null object as parameter for
playstate change callback, without updating the
MediaSession. These events should still trigger an update
by using the Mediasession metadata if it is different.
Bug: 297191838
Tag: #feature
Test: atest BluetoothInstrumentationTests
Change-Id: Ic5a5b6a68e616af939a4e42d7d64476932dc38d3
---
.../audio_util/MediaPlayerWrapper.java | 22 ++++++++-----
.../audio_util/MediaPlayerWrapperTest.java | 33 ++++++++++++++++++-
2 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/android/app/src/com/android/bluetooth/audio_util/MediaPlayerWrapper.java b/android/app/src/com/android/bluetooth/audio_util/MediaPlayerWrapper.java
index 3225244b3be..256326da64b 100644
--- a/android/app/src/com/android/bluetooth/audio_util/MediaPlayerWrapper.java
+++ b/android/app/src/com/android/bluetooth/audio_util/MediaPlayerWrapper.java
@@ -461,8 +461,11 @@ public class MediaPlayerWrapper {
@Override
public void onMetadataChanged(@Nullable MediaMetadata mediaMetadata) {
if (!isMetadataReady()) {
- Log.v(TAG, "onMetadataChanged(): " + mPackageName
- + " tried to update with no queue");
+ Log.v(
+ TAG,
+ "onMetadataChanged(): "
+ + mPackageName
+ + " tried to update with no metadata");
return;
}
@@ -495,13 +498,16 @@ public class MediaPlayerWrapper {
@Override
public void onPlaybackStateChanged(@Nullable PlaybackState state) {
if (!isPlaybackStateReady()) {
- Log.v(TAG, "onPlaybackStateChanged(): " + mPackageName
- + " tried to update with no queue");
+ Log.v(
+ TAG,
+ "onPlaybackStateChanged(): "
+ + mPackageName
+ + " tried to update with no state");
return;
}
- mPlaybackStateChangeEventLogger.logv(TAG, "onPlaybackStateChanged(): "
- + mPackageName + " : " + state.toString());
+ mPlaybackStateChangeEventLogger.logv(
+ TAG, "onPlaybackStateChanged(): " + mPackageName + " : " + state);
if (!playstateEquals(state, getPlaybackState())) {
e("The callback playback state doesn't match the current state");
@@ -513,8 +519,8 @@ public class MediaPlayerWrapper {
return;
}
- // If there is no playstate, ignore the update.
- if (state.getState() == PlaybackState.STATE_NONE) {
+ // If state isn't null and there is no playstate, ignore the update.
+ if (state != null && state.getState() == PlaybackState.STATE_NONE) {
Log.v(TAG, "Waiting to send update as controller has no playback state");
return;
}
diff --git a/android/app/tests/unit/src/com/android/bluetooth/audio_util/MediaPlayerWrapperTest.java b/android/app/tests/unit/src/com/android/bluetooth/audio_util/MediaPlayerWrapperTest.java
index 8b3ceae577c..9ed1fa0c96d 100644
--- a/android/app/tests/unit/src/com/android/bluetooth/audio_util/MediaPlayerWrapperTest.java
+++ b/android/app/tests/unit/src/com/android/bluetooth/audio_util/MediaPlayerWrapperTest.java
@@ -21,7 +21,6 @@ import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.*;
import android.content.Context;
-import android.content.pm.PackageManager;
import android.content.res.Resources;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
@@ -377,6 +376,38 @@ public class MediaPlayerWrapperTest {
Util.toMetadata(mMockContext, mTestMetadata.build()));
}
+ @Test
+ public void testNullPlayback() {
+ // Create the wrapper object and register the looper with the timeout handler
+ TestLooperManager looperManager = new TestLooperManager(mThread.getLooper());
+ MediaPlayerWrapper wrapper =
+ MediaPlayerWrapperFactory.wrap(mMockContext, mMockController, mThread.getLooper());
+ wrapper.registerCallback(mTestCbs);
+
+ // Return null when getting the queue
+ doReturn(null).when(mMockController).getQueue();
+
+ // Grab the callbacks the wrapper registered with the controller
+ verify(mMockController).registerCallback(mControllerCbs.capture(), any());
+ MediaController.Callback controllerCallbacks = mControllerCbs.getValue();
+
+ // Update Metadata returned by controller
+ mTestState.setState(PlaybackState.STATE_PLAYING, 1000, 1.0f);
+ doReturn(mTestState.build()).when(mMockController).getPlaybackState();
+
+ // Call the callback
+ controllerCallbacks.onPlaybackStateChanged(null);
+
+ // Assert that the metadata returned by getPlaybackState() is used instead of null
+
+ verify(mTestCbs, times(1)).mediaUpdatedCallback(mMediaUpdateData.capture());
+ MediaData data = mMediaUpdateData.getValue();
+ Assert.assertEquals(
+ "Returned PlaybackState is incorrect",
+ data.state.toString(),
+ mTestState.build().toString());
+ }
+
@Test
public void testNullQueue() {
// Create the wrapper object and register the looper with the timeout handler
--
GitLab