Notification is still visible when playlist is empty

[NicolasDucom]

Version

Media3 1.6.0

More version details

Following upgrade to version 1.6.0, when extending SimpleBasePlayer, the notification is still displayed when State.Build().setPlaylist() is given an empty list when building State in getState()
The comments in this commit seem to indicate that the notification should be dismissed if the playlist is cleared

Devices that reproduce the issue

Xiaomi, Samsung and Pixel devices

Devices that do not reproduce the issue

Non for the moment

Reproducible in the demo app?

Not tested

Reproduction steps

Probably not reproductible since the demo app uses ExoPlayer and not an implementation of simpleBasePlayer

Expected result

When the playlist returned in getState is empty, notification shouldn't be visible

Actual result

Notification is visible

Media

Not streaming using Exoplayer

Bug Report

• You will email the zip file produced by adb bugreport to android-media-github@google.com after filing this issue.

[marcbaechinger]

Thanks for your report!

I changed the test case that got added in the commit you are pointing to to use a SimpleBasePlayer.

When I run the test and add and remove the playlist this seems to create a notification and then remove the notification again.

So if the playlist of the simple base player is empty, then the notification is removed as expected. From the devices I see you can repro the issue it looks it just generally does not work.

Unfortunately, we can't simply add an EventLogger to SimpleBasePlayer. Would it be possible to add some kind of a Player.Listener and then log what you see is reported with onTimelineChanged() and check whether the timeline is empty as expected?

Any other hints where around the condition the player may be in at that very instance?

  @Test
  public void service_sessionIdleWithMediaCleared_createsThenRemovesNotification() {
    Player player =
        new SimpleBasePlayer(Looper.myLooper()) {
          private List<MediaItem> mediaItems = new ArrayList<>();

          @Override
          protected State getState() {
            return new State.Builder()
                .setAvailableCommands(new Commands.Builder().addAllCommands().build())
                .setPlaybackState(Player.STATE_IDLE)
                .setPlaylist(createPlaylist())
                .build();
          }

          @Override
          protected ListenableFuture<?> handleSetMediaItems(
              List<MediaItem> mediaItems, int startIndex, long startPositionMs) {
            this.mediaItems = mediaItems;
            return Futures.immediateVoidFuture();
          }

          @Override
          protected ListenableFuture<?> handleRemoveMediaItems(int fromIndex, int toIndex) {
            this.mediaItems = new ArrayList<>();
            return Futures.immediateVoidFuture();
          }

          @Override
          protected ListenableFuture<?> handleRelease() {
            return Futures.immediateVoidFuture();
          }

          public ImmutableList<MediaItemData> createPlaylist() {
            ImmutableList.Builder<MediaItemData> playlist = new ImmutableList.Builder<>();
            for (MediaItem item : mediaItems) {
              playlist.add(new MediaItemData.Builder(new Object()).setMediaItem(item).build());
            }
            return playlist.build();
          }
        };
    MediaSession session = new MediaSession.Builder(context, player).build();
    ServiceController<TestService> serviceController = Robolectric.buildService(TestService.class);
    TestService service = serviceController.create().get();
    service.setMediaNotificationProvider(
        new DefaultMediaNotificationProvider(
            service,
            /* notificationIdProvider= */ unused -> 2000,
            DefaultMediaNotificationProvider.DEFAULT_CHANNEL_ID,
            DefaultMediaNotificationProvider.DEFAULT_CHANNEL_NAME_RESOURCE_ID));
    service.addSession(session);

    // Add media and give the service a chance to create a notification.
    player.setMediaItem(MediaItem.fromUri("asset:///media/mp4/sample.mp4"));
    ShadowLooper.idleMainLooper();

    assertThat(getStatusBarNotification(2000)).isNotNull();

    player.clearMediaItems();
    ShadowLooper.idleMainLooper();

    assertThat(getStatusBarNotification(2000)).isNull();

    session.release();
    player.release();
    serviceController.destroy();
  }

[marcbaechinger]

Oh, wait - you are saying this worked with an earlier version?

[NicolasDucom]

Hi Marc,
Yes I didn't have this issue with 1.5.1,
I added a listener with a log on onTimelineChanged, when my queue is empty : timeline.isEmpty is true, periodCount and windowCount are at zero, I added a couple of breakpoints and here's what i can see :
When i have some items in my playlist :

• in MediaNotificationManager -> shouldShowNotification, controller.getCurrentTimeline().isEmpty() returns false, getWindowCount in Timeline calls the implementation from PlaylistTimeline and returns the playlist size
• when i update with an empty playlist, windowCount from CurrentMediaItemOnlyTimeline is called and returns 1

I think this worked with 1.5.1 because my playerState was set to IDLE at that point

[marcbaechinger]

Thanks for your response!

This looks to me like a bug in PlayerWrapper.getCurrentTimelineWIthCommandCheck(). If the timeline is empty, we return a new CurrentMediaItemOnlyTimeline(this) which always has a window size of 1 as you report above.

It appears to me that we need to check whether the timeline is empty and then return the empty timeline with whatever commands are set.

I'm not sure if this is helpful for you. In case you add COMMAND_GET_TIMELINE to the available commands of your SimpleBasePlayer, then the actual timeline is returned that is empty and this should resolve the issue above without any changes in the library. I'm not sure if it acceptable for your to add COMMAND_GET_TIMELINE.

In any case, I'll do a change that makes getCurrentTimelineWIthCommandCheck working as intended.

Thanks again for looking into this again!

[NicolasDucom]

My pleasure !
Can confirm I don't have the issue with COMMAND_GET_TIMELINE 🙂 👍

[marcbaechinger]

Glad to hear that worked!

The commit above will be included in the next release to fix that behavior.

I'm closing this issue as there a workaround for your use case available and a fix is in the main branch. Thanks again for your report, Nicolas!