From 06bd3a65fef7393501fc7c2c63e692e997702202 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 24 Nov 2020 17:12:52 +0000 Subject: [PATCH] Don't inform of static metadata changes if they are all empty. The current code creates placeholder metadata elements if there is no static metadata. This causes onStaticMetadataChanged callbacks even if there is no metadata. Instead, we can keep the empty list as the static metadata is already documented to be an empty list if the metadata is unavailable. #exofixit PiperOrigin-RevId: 344071639 --- .../android/exoplayer2/ExoPlayerImpl.java | 2 +- .../exoplayer2/ExoPlayerImplInternal.java | 10 ++++-- .../com/google/android/exoplayer2/Player.java | 6 ++-- .../analytics/AnalyticsListener.java | 2 +- .../android/exoplayer2/ExoPlayerTest.java | 36 ++++++++++++++----- .../analytics/AnalyticsCollectorTest.java | 8 ----- 6 files changed, 40 insertions(+), 24 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java index 73ad867961..51f897e2e5 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImpl.java @@ -1259,7 +1259,7 @@ import java.util.concurrent.TimeoutException; /* totalBufferedDurationUs= */ 0, TrackGroupArray.EMPTY, emptyTrackSelectorResult, - ImmutableList.of()); + /* staticMetadata= */ ImmutableList.of()); playbackInfo = playbackInfo.copyWithLoadingMediaPeriodId(dummyMediaPeriodId); playbackInfo.bufferedPositionUs = playbackInfo.positionUs; return playbackInfo; diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index 607f775335..7c3dd553b9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -2247,14 +2247,20 @@ import java.util.concurrent.atomic.AtomicBoolean; private ImmutableList extractMetadataFromTrackSelectionArray( TrackSelectionArray trackSelectionArray) { ImmutableList.Builder result = new ImmutableList.Builder<>(); + boolean seenNonEmptyMetadata = false; for (int i = 0; i < trackSelectionArray.length; i++) { @Nullable TrackSelection trackSelection = trackSelectionArray.get(i); if (trackSelection != null) { Format format = trackSelection.getFormat(/* index= */ 0); - result.add(format.metadata == null ? new Metadata() : format.metadata); + if (format.metadata == null) { + result.add(new Metadata()); + } else { + result.add(format.metadata); + seenNonEmptyMetadata = true; + } } } - return result.build(); + return seenNonEmptyMetadata ? result.build() : ImmutableList.of(); } private void enableRenderers() throws ExoPlaybackException { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/Player.java b/library/core/src/main/java/com/google/android/exoplayer2/Player.java index 8de498a7e7..20ba9cab19 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/Player.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/Player.java @@ -512,8 +512,8 @@ public interface Player { * *

The provided {@code metadataList} is an immutable list of {@link Metadata} instances, * where the elements correspond to the {@link #getCurrentTrackSelections() current track - * selections}, or an empty list if there are no track selections or the implementation does not - * support metadata. + * selections}, or an empty list if there are no track selections or the selected tracks contain + * no static metadata. * *

The metadata is considered static in the sense that it comes from the tracks' declared * Formats, rather than being timed (or dynamic) metadata, which is represented within a @@ -1433,7 +1433,7 @@ public interface Player { * *

The returned {@code metadataList} is an immutable list of {@link Metadata} instances, where * the elements correspond to the {@link #getCurrentTrackSelections() current track selections}, - * or an empty list if there are no track selections or the implementation does not support + * or an empty list if there are no track selections or the selected tracks contain no static * metadata. * *

This metadata is considered static in that it comes from the tracks' declared Formats, diff --git a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java index 8cdfda91f4..6ac6ae5847 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/analytics/AnalyticsListener.java @@ -604,7 +604,7 @@ public interface AnalyticsListener { *

The provided {@code metadataList} is an immutable list of {@link Metadata} instances, where * the elements correspond to the current track selections (as returned by {@link * #onTracksChanged(EventTime, TrackGroupArray, TrackSelectionArray)}, or an empty list if there - * are no track selections or the implementation does not support metadata. + * are no track selections or the selected tracks contain no static metadata. * *

The metadata is considered static in the sense that it comes from the tracks' declared * Formats, rather than being timed (or dynamic) metadata, which is represented within a metadata diff --git a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java index 06c542691f..e4f14cfbb7 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/ExoPlayerTest.java @@ -8260,8 +8260,6 @@ public final class ExoPlayerTest { Format videoFormat = new Format.Builder() .setSampleMimeType(MimeTypes.VIDEO_H264) - .setWidth(1920) - .setHeight(720) .setMetadata( new Metadata( new TextInformationFrame( @@ -8269,11 +8267,9 @@ public final class ExoPlayerTest { /* description= */ "Video", /* value= */ "Video track name"))) .build(); - Format audioFormat = new Format.Builder() .setSampleMimeType(MimeTypes.AUDIO_AAC) - .setSampleRate(44_000) .setMetadata( new Metadata( new TextInformationFrame( @@ -8281,9 +8277,7 @@ public final class ExoPlayerTest { /* description= */ "Audio", /* value= */ "Audio track name"))) .build(); - EventListener eventListener = mock(EventListener.class); - Timeline fakeTimeline = new FakeTimeline( new TimelineWindowDefinition( @@ -8295,14 +8289,38 @@ public final class ExoPlayerTest { player.prepare(); player.play(); runUntilPlaybackState(player, Player.STATE_ENDED); + List metadata = player.getCurrentStaticMetadata(); + player.release(); - assertThat(player.getCurrentStaticMetadata()) - .containsExactly(videoFormat.metadata, audioFormat.metadata) - .inOrder(); + assertThat(metadata).containsExactly(videoFormat.metadata, audioFormat.metadata).inOrder(); verify(eventListener) .onStaticMetadataChanged(ImmutableList.of(videoFormat.metadata, audioFormat.metadata)); } + @Test + public void staticMetadata_callbackIsNotCalledWhenMetadataEmptyAndGetterReturnsEmptyList() + throws Exception { + Format videoFormat = new Format.Builder().setSampleMimeType(MimeTypes.VIDEO_H264).build(); + Format audioFormat = new Format.Builder().setSampleMimeType(MimeTypes.AUDIO_AAC).build(); + EventListener eventListener = mock(EventListener.class); + Timeline fakeTimeline = + new FakeTimeline( + new TimelineWindowDefinition( + /* isSeekable= */ true, /* isDynamic= */ false, /* durationUs= */ 100000)); + SimpleExoPlayer player = new TestExoPlayerBuilder(context).build(); + + player.setMediaSource(new FakeMediaSource(fakeTimeline, videoFormat, audioFormat)); + player.addListener(eventListener); + player.prepare(); + player.play(); + runUntilPlaybackState(player, Player.STATE_ENDED); + List metadata = player.getCurrentStaticMetadata(); + player.release(); + + assertThat(metadata).isEmpty(); + verify(eventListener, never()).onStaticMetadataChanged(any()); + } + @Test public void targetLiveOffsetInMedia_adjustsLiveOffsetToTargetOffset() throws Exception { long windowStartUnixTimeMs = 987_654_321_000L; diff --git a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java index ac079b99a3..c60a30c3b6 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/analytics/AnalyticsCollectorTest.java @@ -40,7 +40,6 @@ import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_PL import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_PLAY_WHEN_READY_CHANGED; import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_POSITION_DISCONTINUITY; import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_RENDERED_FIRST_FRAME; -import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_STATIC_METADATA_CHANGED; import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_TIMELINE_CHANGED; import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_TRACKS_CHANGED; import static com.google.android.exoplayer2.analytics.AnalyticsListener.EVENT_VIDEO_DECODER_INITIALIZED; @@ -1714,10 +1713,6 @@ public final class AnalyticsCollectorTest { ArgumentCaptor.forClass(AnalyticsListener.EventTime.class); verify(listener, atLeastOnce()) .onAudioEnabled(individualAudioEnabledEventTimes.capture(), any()); - ArgumentCaptor individualStaticMetadataChangedEventTimes = - ArgumentCaptor.forClass(AnalyticsListener.EventTime.class); - verify(listener, atLeastOnce()) - .onStaticMetadataChanged(individualStaticMetadataChangedEventTimes.capture(), any()); ArgumentCaptor individualDownstreamFormatChangedEventTimes = ArgumentCaptor.forClass(AnalyticsListener.EventTime.class); verify(listener, atLeastOnce()) @@ -1839,9 +1834,6 @@ public final class AnalyticsCollectorTest { assertThat(individualAudioEnabledEventTimes.getAllValues()) .containsAtLeastElementsIn(onEventsEventTimes.get(EVENT_AUDIO_ENABLED)) .inOrder(); - assertThat(individualStaticMetadataChangedEventTimes.getAllValues()) - .containsAtLeastElementsIn(onEventsEventTimes.get(EVENT_STATIC_METADATA_CHANGED)) - .inOrder(); assertThat(individualDownstreamFormatChangedEventTimes.getAllValues()) .containsAtLeastElementsIn(onEventsEventTimes.get(EVENT_DOWNSTREAM_FORMAT_CHANGED)) .inOrder();