From 2103fd016b7e4cede6d0aaf7230415e1615aff43 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Wed, 2 Dec 2020 13:40:46 -0400 Subject: [PATCH] Return sane value if player is out of sync with data adapter. --- .../voice/VoiceNoteMediaController.java | 3 +- .../voice/VoiceNoteNotificationManager.java | 20 +++- .../voice/VoiceNotePlaybackPreparer.java | 112 +++++++++--------- .../voice/VoiceNoteProximityManager.java | 6 +- .../voice/VoiceNoteQueueDataAdapter.java | 31 +++-- 5 files changed, 103 insertions(+), 69 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteMediaController.java b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteMediaController.java index 15a7855f41..716e9f29a3 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteMediaController.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteMediaController.java @@ -222,7 +222,8 @@ public class VoiceNoteMediaController implements DefaultLifecycleObserver { MediaMetadataCompat mediaMetadataCompat = mediaController.getMetadata(); if (isPlayerActive(mediaController.getPlaybackState()) && mediaMetadataCompat != null && - mediaMetadataCompat.getDescription() != null) + mediaMetadataCompat.getDescription() != null && + mediaMetadataCompat.getDescription().getMediaUri() != null) { Uri mediaUri = Objects.requireNonNull(mediaMetadataCompat.getDescription().getMediaUri()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteNotificationManager.java b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteNotificationManager.java index 7ca014e801..5a49f74b87 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteNotificationManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteNotificationManager.java @@ -79,7 +79,7 @@ class VoiceNoteNotificationManager { @Override public String getCurrentContentTitle(Player player) { if (hasMetadata()) { - return Objects.requireNonNull(controller.getMetadata().getDescription().getTitle()).toString(); + return Objects.toString(controller.getMetadata().getDescription().getTitle(), null); } else { return null; } @@ -89,9 +89,14 @@ class VoiceNoteNotificationManager { public @Nullable PendingIntent createCurrentContentIntent(Player player) { if (!hasMetadata()) return null; - RecipientId recipientId = RecipientId.from(Objects.requireNonNull(controller.getMetadata().getString(VoiceNoteMediaDescriptionCompatFactory.EXTRA_THREAD_RECIPIENT_ID))); - int startingPosition = (int) controller.getMetadata().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_MESSAGE_POSITION); - long threadId = controller.getMetadata().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_THREAD_ID); + String serializedRecipientId = controller.getMetadata().getString(VoiceNoteMediaDescriptionCompatFactory.EXTRA_THREAD_RECIPIENT_ID); + if (serializedRecipientId == null) { + return null; + } + + RecipientId recipientId = RecipientId.from(serializedRecipientId); + int startingPosition = (int) controller.getMetadata().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_MESSAGE_POSITION); + long threadId = controller.getMetadata().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_THREAD_ID); MaterialColor color; try { @@ -131,7 +136,12 @@ class VoiceNoteNotificationManager { return null; } - RecipientId currentRecipientId = RecipientId.from(Objects.requireNonNull(controller.getMetadata().getString(VoiceNoteMediaDescriptionCompatFactory.EXTRA_AVATAR_RECIPIENT_ID))); + String serializedRecipientId = controller.getMetadata().getString(VoiceNoteMediaDescriptionCompatFactory.EXTRA_AVATAR_RECIPIENT_ID); + if (serializedRecipientId == null) { + return null; + } + + RecipientId currentRecipientId = RecipientId.from(serializedRecipientId); if (Objects.equals(currentRecipientId, cachedRecipientId) && cachedBitmap != null) { return cachedBitmap; diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackPreparer.java b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackPreparer.java index 123a5a4a88..f745f0a305 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackPreparer.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNotePlaybackPreparer.java @@ -7,6 +7,7 @@ import android.os.ResultReceiver; import android.support.v4.media.MediaDescriptionCompat; import android.support.v4.media.session.PlaybackStateCompat; +import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -106,13 +107,11 @@ final class VoiceNotePlaybackPreparer implements MediaSessionConnector.PlaybackP } }, descriptions -> { - if (Util.hasItems(descriptions) && Objects.equals(latestUri, uri)) { - synchronized (queueDataAdapter) { - queueDataAdapter.clear(); - dataSource.clear(); + queueDataAdapter.clear(); + dataSource.clear(); - applyDescriptionsToQueue(descriptions); - } + if (Util.hasItems(descriptions) && Objects.equals(latestUri, uri)) { + applyDescriptionsToQueue(descriptions); int window = Math.max(0, queueDataAdapter.indexOf(uri)); @@ -141,61 +140,64 @@ final class VoiceNotePlaybackPreparer implements MediaSessionConnector.PlaybackP public void onCommand(Player player, String command, Bundle extras, ResultReceiver cb) { } + @MainThread private void applyDescriptionsToQueue(@NonNull List descriptions) { - synchronized (queueDataAdapter) { - for (MediaDescriptionCompat description : descriptions) { - int holderIndex = queueDataAdapter.indexOf(description.getMediaUri()); - MediaDescriptionCompat next = createNextClone(description); - int currentIndex = player.getCurrentWindowIndex(); + for (MediaDescriptionCompat description : descriptions) { + int holderIndex = queueDataAdapter.indexOf(description.getMediaUri()); + MediaDescriptionCompat next = createNextClone(description); + int currentIndex = player.getCurrentWindowIndex(); - if (holderIndex != -1) { + if (holderIndex != -1) { + queueDataAdapter.remove(holderIndex); + + if (!queueDataAdapter.isEmpty()) { queueDataAdapter.remove(holderIndex); - - if (!queueDataAdapter.isEmpty()) { - queueDataAdapter.remove(holderIndex); - } - - queueDataAdapter.add(holderIndex, createNextClone(description)); - queueDataAdapter.add(holderIndex, description); - - if (currentIndex != holderIndex) { - dataSource.removeMediaSource(holderIndex); - dataSource.addMediaSource(holderIndex, mediaSourceFactory.createMediaSource(description)); - } - - if (currentIndex != holderIndex + 1) { - if (dataSource.getSize() > 1) { - dataSource.removeMediaSource(holderIndex + 1); - } - - dataSource.addMediaSource(holderIndex + 1, mediaSourceFactory.createMediaSource(next)); - } - } else { - int insertLocation = queueDataAdapter.indexAfter(description); - - queueDataAdapter.add(insertLocation, next); - queueDataAdapter.add(insertLocation, description); - - dataSource.addMediaSource(insertLocation, mediaSourceFactory.createMediaSource(next)); - dataSource.addMediaSource(insertLocation, mediaSourceFactory.createMediaSource(description)); } - } - int lastIndex = queueDataAdapter.size() - 1; - MediaDescriptionCompat last = queueDataAdapter.getMediaDescription(lastIndex); + queueDataAdapter.add(holderIndex, createNextClone(description)); + queueDataAdapter.add(holderIndex, description); - if (Objects.equals(last.getMediaUri(), NEXT_URI)) { - queueDataAdapter.remove(lastIndex); - dataSource.removeMediaSource(lastIndex); - - if (queueDataAdapter.size() > 1) { - MediaDescriptionCompat end = createEndClone(last); - - queueDataAdapter.add(lastIndex, end); - dataSource.addMediaSource(lastIndex, mediaSourceFactory.createMediaSource(end)); + if (currentIndex != holderIndex) { + dataSource.removeMediaSource(holderIndex); + dataSource.addMediaSource(holderIndex, mediaSourceFactory.createMediaSource(description)); } + + if (currentIndex != holderIndex + 1) { + if (dataSource.getSize() > 1) { + dataSource.removeMediaSource(holderIndex + 1); + } + + dataSource.addMediaSource(holderIndex + 1, mediaSourceFactory.createMediaSource(next)); + } + } else { + int insertLocation = queueDataAdapter.indexAfter(description); + + queueDataAdapter.add(insertLocation, next); + queueDataAdapter.add(insertLocation, description); + + dataSource.addMediaSource(insertLocation, mediaSourceFactory.createMediaSource(next)); + dataSource.addMediaSource(insertLocation, mediaSourceFactory.createMediaSource(description)); } } + + int lastIndex = queueDataAdapter.size() - 1; + MediaDescriptionCompat last = queueDataAdapter.getMediaDescription(lastIndex); + + if (Objects.equals(last.getMediaUri(), NEXT_URI)) { + queueDataAdapter.remove(lastIndex); + dataSource.removeMediaSource(lastIndex); + + if (queueDataAdapter.size() > 1) { + MediaDescriptionCompat end = createEndClone(last); + + queueDataAdapter.add(lastIndex, end); + dataSource.addMediaSource(lastIndex, mediaSourceFactory.createMediaSource(end)); + } + } + + if (queueDataAdapter.size() != dataSource.getSize()) { + throw new IllegalStateException("QueueDataAdapter and DataSource size inconsistency."); + } } private @NonNull MediaDescriptionCompat createEndClone(@NonNull MediaDescriptionCompat source) { @@ -223,7 +225,11 @@ final class VoiceNotePlaybackPreparer implements MediaSessionConnector.PlaybackP } MediaDescriptionCompat mediaDescriptionCompat = queueDataAdapter.getMediaDescription(player.getCurrentWindowIndex()); - long messageId = mediaDescriptionCompat.getExtras().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_MESSAGE_ID); + if (Objects.equals(mediaDescriptionCompat, VoiceNoteQueueDataAdapter.EMPTY)) { + return; + } + + long messageId = mediaDescriptionCompat.getExtras().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_MESSAGE_ID); SimpleTask.run(EXECUTOR, () -> loadMediaDescriptionsForConsecutivePlayback(messageId), diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteProximityManager.java b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteProximityManager.java index ca7da07c0f..e5245cc537 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteProximityManager.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteProximityManager.java @@ -96,7 +96,11 @@ class VoiceNoteProximityManager implements SensorEventListener { } else { MediaDescriptionCompat mediaDescriptionCompat = queueDataAdapter.getMediaDescription(windowIndex); - threadId = mediaDescriptionCompat.getExtras().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_THREAD_ID, -1); + if (mediaDescriptionCompat.getExtras() == null) { + threadId = -1; + } else { + threadId = mediaDescriptionCompat.getExtras().getLong(VoiceNoteMediaDescriptionCompatFactory.EXTRA_THREAD_ID, -1); + } } if (desiredStreamType == AudioManager.STREAM_VOICE_CALL && diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteQueueDataAdapter.java b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteQueueDataAdapter.java index 97210216e5..cbd55dd430 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteQueueDataAdapter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/components/voice/VoiceNoteQueueDataAdapter.java @@ -3,11 +3,14 @@ package org.thoughtcrime.securesms.components.voice; import android.net.Uri; import android.support.v4.media.MediaDescriptionCompat; +import androidx.annotation.MainThread; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.google.android.exoplayer2.ext.mediasession.TimelineQueueEditor; +import org.thoughtcrime.securesms.logging.Log; + import java.util.LinkedList; import java.util.List; import java.util.Objects; @@ -15,36 +18,46 @@ import java.util.Objects; /** * DataAdapter which maintains the current queue of MediaDescriptionCompat objects. */ +@MainThread final class VoiceNoteQueueDataAdapter implements TimelineQueueEditor.QueueDataAdapter { + private static final String TAG = Log.tag(VoiceNoteQueueDataAdapter.class); + + public static final MediaDescriptionCompat EMPTY = new MediaDescriptionCompat.Builder().build(); + private final List descriptions = new LinkedList<>(); @Override - public synchronized MediaDescriptionCompat getMediaDescription(int position) { + public MediaDescriptionCompat getMediaDescription(int position) { + if (descriptions.size() <= position) { + Log.i(TAG, "getMediaDescription: Returning EMPTY MediaDescriptionCompat for index " + position); + return EMPTY; + } + return descriptions.get(position); } @Override - public synchronized void add(int position, MediaDescriptionCompat description) { + public void add(int position, MediaDescriptionCompat description) { descriptions.add(position, description); } @Override - public synchronized void remove(int position) { + public void remove(int position) { descriptions.remove(position); } @Override - public synchronized void move(int from, int to) { + public void move(int from, int to) { MediaDescriptionCompat description = descriptions.remove(from); descriptions.add(to, description); } - synchronized int size() { + int size() { return descriptions.size(); } - synchronized int indexOf(@NonNull Uri uri) { + int indexOf(@NonNull Uri uri) { for (int i = 0; i < descriptions.size(); i++) { if (Objects.equals(uri, descriptions.get(i).getMediaUri())) { return i; @@ -54,7 +67,7 @@ final class VoiceNoteQueueDataAdapter implements TimelineQueueEditor.QueueDataAd return -1; } - synchronized int indexAfter(@NonNull MediaDescriptionCompat target) { + int indexAfter(@NonNull MediaDescriptionCompat target) { if (isEmpty()) { return 0; } @@ -71,11 +84,11 @@ final class VoiceNoteQueueDataAdapter implements TimelineQueueEditor.QueueDataAd return descriptions.size(); } - synchronized boolean isEmpty() { + boolean isEmpty() { return descriptions.isEmpty(); } - synchronized void clear() { + void clear() { descriptions.clear(); } }