Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong thumbnail in notification on Android 13 #8899

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 47 additions & 37 deletions app/src/main/java/org/schabi/newpipe/player/Player.java
Original file line number Diff line number Diff line change
Expand Up @@ -765,17 +765,15 @@ public void onBitmapLoaded(final Bitmap bitmap, final Picasso.LoadedFrom from) {
+ " -> " + bitmap.getWidth() + "x" + bitmap.getHeight() + "], from = ["
+ from + "]");
}
currentThumbnail = bitmap;
// there is a new thumbnail, so e.g. the end screen thumbnail needs to change, too.
UIs.call(playerUi -> playerUi.onThumbnailLoaded(bitmap));
onThumbnailLoaded(bitmap);
}

@Override
public void onBitmapFailed(final Exception e, final Drawable errorDrawable) {
Log.e(TAG, "Thumbnail - onBitmapFailed() called", e);
currentThumbnail = null;
// there is a new thumbnail, so e.g. the end screen thumbnail needs to change, too.
UIs.call(playerUi -> playerUi.onThumbnailLoaded(null));
onThumbnailLoaded(null);
}

@Override
Expand All @@ -798,7 +796,7 @@ private void loadCurrentThumbnail(final String url) {

// Unset currentThumbnail, since it is now outdated. This ensures it is not used in media
// session metadata while the new thumbnail is being loaded by Picasso.
currentThumbnail = null;
onThumbnailLoaded(null);
if (isNullOrEmpty(url)) {
return;
}
Expand All @@ -813,6 +811,16 @@ private void cancelLoadingCurrentThumbnail() {
// cancel the Picasso job associated with the player thumbnail, if any
PicassoHelper.cancelTag(PICASSO_PLAYER_THUMBNAIL_TAG);
}

private void onThumbnailLoaded(@Nullable final Bitmap bitmap) {
// Avoid useless thumbnail updates, if the thumbnail has not actually changed. Based on the
// thumbnail loading code, this if would be skipped only when both bitmaps are `null`, since
// onThumbnailLoaded won't be called twice with the same nonnull bitmap by Picasso's target.
if (currentThumbnail != bitmap) {
currentThumbnail = bitmap;
UIs.call(playerUi -> playerUi.onThumbnailLoaded(bitmap));
}
}
//endregion


Expand Down Expand Up @@ -1501,48 +1509,50 @@ public void onPlaybackSynchronize(@NonNull final PlayQueueItem item, final boole
Log.d(TAG, "Playback - onPlaybackSynchronize(was blocked: " + wasBlocked
+ ") called with item=[" + item.getTitle() + "], url=[" + item.getUrl() + "]");
}
if (exoPlayerIsNull() || playQueue == null) {
return;
if (exoPlayerIsNull() || playQueue == null || currentItem == item) {
return; // nothing to synchronize
}

final boolean hasPlayQueueItemChanged = currentItem != item;

final int currentPlayQueueIndex = playQueue.indexOf(item);
final int currentPlaylistIndex = simpleExoPlayer.getCurrentMediaItemIndex();
final int currentPlaylistSize = simpleExoPlayer.getCurrentTimeline().getWindowCount();
final int playQueueIndex = playQueue.indexOf(item);
final int playlistIndex = simpleExoPlayer.getCurrentMediaItemIndex();
final int playlistSize = simpleExoPlayer.getCurrentTimeline().getWindowCount();
final boolean removeThumbnailBeforeSync = currentItem == null
|| currentItem.getServiceId() != item.getServiceId()
|| !currentItem.getUrl().equals(item.getUrl());

// If nothing to synchronize
if (!hasPlayQueueItemChanged) {
return;
}
currentItem = item;

// Check if on wrong window
if (currentPlayQueueIndex != playQueue.getIndex()) {
Log.e(TAG, "Playback - Play Queue may be desynchronized: item "
+ "index=[" + currentPlayQueueIndex + "], "
+ "queue index=[" + playQueue.getIndex() + "]");

// Check if bad seek position
} else if ((currentPlaylistSize > 0 && currentPlayQueueIndex >= currentPlaylistSize)
|| currentPlayQueueIndex < 0) {
Log.e(TAG, "Playback - Trying to seek to invalid "
+ "index=[" + currentPlayQueueIndex + "] with "
+ "playlist length=[" + currentPlaylistSize + "]");

} else if (wasBlocked || currentPlaylistIndex != currentPlayQueueIndex || !isPlaying()) {
if (playQueueIndex != playQueue.getIndex()) {
// wrong window (this should be impossible, as this method is called with
// `item=playQueue.getItem()`, so the index of that item must be equal to `getIndex()`)
Log.e(TAG, "Playback - Play Queue may be not in sync: item index=["
+ playQueueIndex + "], " + "queue index=[" + playQueue.getIndex() + "]");

} else if ((playlistSize > 0 && playQueueIndex >= playlistSize) || playQueueIndex < 0) {
// the queue and the player's timeline are not in sync, since the play queue index
// points outside of the timeline
Log.e(TAG, "Playback - Trying to seek to invalid index=[" + playQueueIndex
+ "] with playlist length=[" + playlistSize + "]");

} else if (wasBlocked || playlistIndex != playQueueIndex || !isPlaying()) {
// either the player needs to be unblocked, or the play queue index has just been
// changed and needs to be synchronized, or the player is not playing
if (DEBUG) {
Log.d(TAG, "Playback - Rewinding to correct "
+ "index=[" + currentPlayQueueIndex + "], "
+ "from=[" + currentPlaylistIndex + "], "
+ "size=[" + currentPlaylistSize + "].");
Log.d(TAG, "Playback - Rewinding to correct index=[" + playQueueIndex + "], "
+ "from=[" + playlistIndex + "], size=[" + playlistSize + "].");
}

if (removeThumbnailBeforeSync) {
// unset the current (now outdated) thumbnail to ensure it is not used during sync
onThumbnailLoaded(null);
}

// sync the player index with the queue index, and seek to the correct position
if (item.getRecoveryPosition() != PlayQueueItem.RECOVERY_UNSET) {
simpleExoPlayer.seekTo(currentPlayQueueIndex, item.getRecoveryPosition());
playQueue.unsetRecovery(currentPlayQueueIndex);
simpleExoPlayer.seekTo(playQueueIndex, item.getRecoveryPosition());
playQueue.unsetRecovery(playQueueIndex);
} else {
simpleExoPlayer.seekToDefaultPosition(currentPlayQueueIndex);
simpleExoPlayer.seekToDefaultPosition(playQueueIndex);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public void destroy() {
@Override
public void onThumbnailLoaded(@Nullable final Bitmap bitmap) {
super.onThumbnailLoaded(bitmap);
notificationUtil.createNotificationIfNeededAndUpdate(false);
notificationUtil.updateThumbnail();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.schabi.newpipe.util.NavigationHelper;

import java.util.List;
import java.util.Objects;
import java.util.Optional;

import static android.app.PendingIntent.FLAG_UPDATE_CURRENT;
import static androidx.media.app.NotificationCompat.MediaStyle;
Expand All @@ -40,8 +42,6 @@

/**
* This is a utility class for player notifications.
*
* @author cool-student
*/
public final class NotificationUtil {
private static final String TAG = NotificationUtil.class.getSimpleName();
Expand Down Expand Up @@ -79,6 +79,19 @@ public synchronized void createNotificationIfNeededAndUpdate(final boolean force
notificationManager.notify(NOTIFICATION_ID, notificationBuilder.build());
}

public synchronized void updateThumbnail() {
if (notificationBuilder != null) {
if (DEBUG) {
Log.d(TAG, "updateThumbnail() called with thumbnail = [" + Integer.toHexString(
Optional.ofNullable(player.getThumbnail()).map(Objects::hashCode).orElse(0))
+ "], title = [" + player.getVideoTitle() + "]");
}

setLargeIcon(notificationBuilder);
notificationManager.notify(NOTIFICATION_ID, notificationBuilder.build());
}
}

private synchronized NotificationCompat.Builder createNotification() {
if (DEBUG) {
Log.d(TAG, "createNotification()");
Expand Down Expand Up @@ -123,6 +136,9 @@ private synchronized NotificationCompat.Builder createNotification() {
.setDeleteIntent(PendingIntent.getBroadcast(player.getContext(), NOTIFICATION_ID,
new Intent(ACTION_CLOSE), FLAG_UPDATE_CURRENT));

// set the initial value for the video thumbnail, updatable with updateNotificationThumbnail
setLargeIcon(builder);

return builder;
}

Expand All @@ -142,7 +158,6 @@ private synchronized void updateNotification() {
notificationBuilder.setTicker(player.getVideoTitle());

updateActions(notificationBuilder);
setLargeIcon(notificationBuilder);
}


Expand Down
20 changes: 7 additions & 13 deletions app/src/main/java/org/schabi/newpipe/player/ui/VideoPlayerUi.java
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ public abstract class VideoPlayerUi extends PlayerUi
private final Handler controlsVisibilityHandler = new Handler(Looper.getMainLooper());
@Nullable private SurfaceHolderCallback surfaceHolderCallback;
boolean surfaceIsSetup = false;
@Nullable private Bitmap thumbnail = null;


/*//////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -385,9 +384,7 @@ public void destroyPlayer() {
@Override
public void destroy() {
super.destroy();
if (binding != null) {
binding.endScreen.setImageBitmap(null);
}
binding.endScreen.setImageDrawable(null);
deinitPlayerSeekOverlay();
deinitListeners();
}
Expand Down Expand Up @@ -422,12 +419,10 @@ protected void setupElementsSize(final int buttonsMinWidth,
public void onBroadcastReceived(final Intent intent) {
super.onBroadcastReceived(intent);
if (Intent.ACTION_CONFIGURATION_CHANGED.equals(intent.getAction())) {
// When the orientation changed, the screen height might be smaller.
// If the end screen thumbnail is not re-scaled,
// it can be larger than the current screen height
// and thus enlarging the whole player.
// This causes the seekbar to be ouf the visible area.
updateEndScreenThumbnail();
// When the orientation changes, the screen height might be smaller. If the end screen
// thumbnail is not re-scaled, it can be larger than the current screen height and thus
// enlarging the whole player. This causes the seekbar to be out of the visible area.
updateEndScreenThumbnail(player.getThumbnail());
}
}
//endregion
Expand All @@ -449,11 +444,10 @@ public void onBroadcastReceived(final Intent intent) {
@Override
public void onThumbnailLoaded(@Nullable final Bitmap bitmap) {
super.onThumbnailLoaded(bitmap);
thumbnail = bitmap;
updateEndScreenThumbnail();
updateEndScreenThumbnail(bitmap);
}

private void updateEndScreenThumbnail() {
private void updateEndScreenThumbnail(@Nullable final Bitmap thumbnail) {
if (thumbnail == null) {
// remove end screen thumbnail
binding.endScreen.setImageDrawable(null);
Expand Down