diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 032ecb29a0..adfd097277 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -49,13 +49,20 @@ * Text * Parse SSA/ASS bold & italic info in `Style:` lines ([#8435](https://github.com/google/ExoPlayer/issues/8435)). +* Text: + * Don't display subtitles after the end position of the current media + period (if known). This ensures sideloaded subtitles respect the end + point of `ClippingMediaPeriod` and prevents content subtitles from + continuing to be displayed over mid-roll ads + ([#5317](https://github.com/google/ExoPlayer/issues/5317), + [#8456](https://github.com/google/ExoPlayer/issues/8456)). * MediaSession extension: Remove dependency to core module and rely on common only. The `TimelineQueueEditor` uses a new `MediaDescriptionConverter` for this purpose and does not rely on the `ConcatenatingMediaSource` anymore. * Cast extension: - * Fix `onPositionDiscontinuity` event so that it is not triggered with - reason `DISCONTINUITY_REASON_PERIOD_TRANSITION` after a seek to another - media item and so that it is not triggered after a timeline change. + * Fix `onPositionDiscontinuity` event so that it is not triggered with + reason `DISCONTINUITY_REASON_PERIOD_TRANSITION` after a seek to another + media item and so that it is not triggered after a timeline change. ### 2.13.2 (2021-02-25) diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/ClippedPlaybackTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/ClippedPlaybackTest.java new file mode 100644 index 0000000000..b7f05b0869 --- /dev/null +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/ClippedPlaybackTest.java @@ -0,0 +1,155 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.exoplayer2; + +import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation; +import static com.google.common.truth.Truth.assertThat; + +import android.net.Uri; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.google.android.exoplayer2.source.ClippingMediaSource; +import com.google.android.exoplayer2.text.Cue; +import com.google.android.exoplayer2.text.TextOutput; +import com.google.android.exoplayer2.util.ConditionVariable; +import com.google.android.exoplayer2.util.MimeTypes; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * Instrumentation tests for playback of clipped items using {@link MediaItem#clippingProperties} or + * {@link ClippingMediaSource} directly. + */ +@RunWith(AndroidJUnit4.class) +public final class ClippedPlaybackTest { + + @Test + public void subtitlesRespectClipping_singlePeriod() throws Exception { + MediaItem mediaItem = + new MediaItem.Builder() + .setUri("asset:///media/mp4/sample.mp4") + .setSubtitles( + ImmutableList.of( + new MediaItem.Subtitle( + Uri.parse("asset:///media/webvtt/typical"), + MimeTypes.TEXT_VTT, + "en", + C.SELECTION_FLAG_DEFAULT))) + // Expect the clipping to affect both subtitles and video. + .setClipEndPositionMs(1000) + .build(); + AtomicReference player = new AtomicReference<>(); + CapturingTextOutput textOutput = new CapturingTextOutput(); + ConditionVariable playbackEnded = new ConditionVariable(); + getInstrumentation() + .runOnMainSync( + () -> { + player.set(new SimpleExoPlayer.Builder(getInstrumentation().getContext()).build()); + player.get().addTextOutput(textOutput); + player + .get() + .addListener( + new Player.EventListener() { + @Override + public void onPlaybackStateChanged(@Player.State int state) { + if (state == Player.STATE_ENDED) { + playbackEnded.open(); + } + } + }); + player.get().setMediaItem(mediaItem); + player.get().prepare(); + player.get().play(); + }); + + playbackEnded.block(); + + getInstrumentation().runOnMainSync(() -> player.get().release()); + assertThat(Iterables.getOnlyElement(Iterables.concat(textOutput.cues)).text.toString()) + .isEqualTo("This is the first subtitle."); + } + + @Test + public void subtitlesRespectClipping_multiplePeriods() throws Exception { + ImmutableList mediaItems = + ImmutableList.of( + new MediaItem.Builder() + .setUri("asset:///media/mp4/sample.mp4") + .setSubtitles( + ImmutableList.of( + new MediaItem.Subtitle( + Uri.parse("asset:///media/webvtt/typical"), + MimeTypes.TEXT_VTT, + "en", + C.SELECTION_FLAG_DEFAULT))) + // Expect the clipping to affect both subtitles and video. + .setClipEndPositionMs(1000) + .build(), + new MediaItem.Builder() + .setUri("asset:///media/mp4/sample.mp4") + // Not needed for correctness, just makes test run faster. Must be longer than the + // subtitle content (3.5s). + .setClipEndPositionMs(4_000) + .build()); + AtomicReference player = new AtomicReference<>(); + CapturingTextOutput textOutput = new CapturingTextOutput(); + ConditionVariable playbackEnded = new ConditionVariable(); + getInstrumentation() + .runOnMainSync( + () -> { + player.set(new SimpleExoPlayer.Builder(getInstrumentation().getContext()).build()); + player.get().addTextOutput(textOutput); + player + .get() + .addListener( + new Player.EventListener() { + @Override + public void onPlaybackStateChanged(@Player.State int state) { + if (state == Player.STATE_ENDED) { + playbackEnded.open(); + } + } + }); + player.get().setMediaItems(mediaItems); + player.get().prepare(); + player.get().play(); + }); + + playbackEnded.block(); + + getInstrumentation().runOnMainSync(() -> player.get().release()); + assertThat(Iterables.getOnlyElement(Iterables.concat(textOutput.cues)).text.toString()) + .isEqualTo("This is the first subtitle."); + } + + private static class CapturingTextOutput implements TextOutput { + + private final List> cues; + + private CapturingTextOutput() { + cues = new ArrayList<>(); + } + + @Override + public void onCues(List cues) { + this.cues.add(cues); + } + } +} 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 f6da219717..e6c287ba58 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 @@ -40,6 +40,7 @@ import com.google.android.exoplayer2.source.MediaSource.MediaPeriodId; import com.google.android.exoplayer2.source.SampleStream; import com.google.android.exoplayer2.source.ShuffleOrder; import com.google.android.exoplayer2.source.TrackGroupArray; +import com.google.android.exoplayer2.text.TextRenderer; import com.google.android.exoplayer2.trackselection.ExoTrackSelection; import com.google.android.exoplayer2.trackselection.TrackSelector; import com.google.android.exoplayer2.trackselection.TrackSelectorResult; @@ -1932,7 +1933,12 @@ import java.util.concurrent.atomic.AtomicBoolean; if (sampleStream != null && renderer.getStream() == sampleStream && renderer.hasReadStreamToEnd()) { - renderer.setCurrentStreamFinal(); + long streamEndPositionUs = + readingPeriodHolder.info.durationUs != C.TIME_UNSET + && readingPeriodHolder.info.durationUs != C.TIME_END_OF_SOURCE + ? readingPeriodHolder.getRendererOffset() + readingPeriodHolder.info.durationUs + : C.TIME_UNSET; + setCurrentStreamFinal(renderer, streamEndPositionUs); } } } @@ -1957,7 +1963,8 @@ import java.util.concurrent.atomic.AtomicBoolean; && readingPeriodHolder.mediaPeriod.readDiscontinuity() != C.TIME_UNSET) { // The new period starts with a discontinuity, so the renderers will play out all data, then // be disabled and re-enabled when they start playing the next period. - setAllRendererStreamsFinal(); + setAllRendererStreamsFinal( + /* streamEndPositionUs= */ readingPeriodHolder.getStartPositionRendererTime()); return; } for (int i = 0; i < renderers.length; i++) { @@ -1973,7 +1980,9 @@ import java.util.concurrent.atomic.AtomicBoolean; // it's a no-sample renderer for which rendererOffsetUs should be updated only when // starting to play the next period. Mark the SampleStream as final to play out any // remaining data. - renderers[i].setCurrentStreamFinal(); + setCurrentStreamFinal( + renderers[i], + /* streamEndPositionUs= */ readingPeriodHolder.getStartPositionRendererTime()); } } } @@ -2098,14 +2107,21 @@ import java.util.concurrent.atomic.AtomicBoolean; return true; } - private void setAllRendererStreamsFinal() { + private void setAllRendererStreamsFinal(long streamEndPositionUs) { for (Renderer renderer : renderers) { if (renderer.getStream() != null) { - renderer.setCurrentStreamFinal(); + setCurrentStreamFinal(renderer, streamEndPositionUs); } } } + private void setCurrentStreamFinal(Renderer renderer, long streamEndPositionUs) { + renderer.setCurrentStreamFinal(); + if (renderer instanceof TextRenderer) { + ((TextRenderer) renderer).setFinalStreamEndPositionUs(streamEndPositionUs); + } + } + private void handlePeriodPrepared(MediaPeriod mediaPeriod) throws ExoPlaybackException { if (!queue.isLoading(mediaPeriod)) { // Stale event. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java b/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java index e1c9e7cebe..cbfd43775f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/text/TextRenderer.java @@ -16,6 +16,7 @@ package com.google.android.exoplayer2.text; import static com.google.android.exoplayer2.util.Assertions.checkNotNull; +import static com.google.android.exoplayer2.util.Assertions.checkState; import android.os.Handler; import android.os.Handler.Callback; @@ -91,6 +92,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { @Nullable private SubtitleOutputBuffer subtitle; @Nullable private SubtitleOutputBuffer nextSubtitle; private int nextSubtitleEventIndex; + private long finalStreamEndPositionUs; /** * @param output The output. @@ -121,6 +123,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { outputLooper == null ? null : Util.createHandler(outputLooper, /* callback= */ this); this.decoderFactory = decoderFactory; formatHolder = new FormatHolder(); + finalStreamEndPositionUs = C.TIME_UNSET; } @Override @@ -141,6 +144,21 @@ public final class TextRenderer extends BaseRenderer implements Callback { } } + /** + * Sets the position at which to stop rendering the current stream. + * + *

Must be called after {@link #setCurrentStreamFinal()}. + * + * @param streamEndPositionUs The position to stop rendering at or {@link C#LENGTH_UNSET} to + * render until the end of the current stream. + */ + // TODO(internal b/181312195): Remove this when it's no longer needed once subtitles are decoded + // on the loading side of SampleQueue. + public void setFinalStreamEndPositionUs(long streamEndPositionUs) { + checkState(isCurrentStreamFinal()); + this.finalStreamEndPositionUs = streamEndPositionUs; + } + @Override protected void onStreamChanged(Format[] formats, long startPositionUs, long offsetUs) { streamFormat = formats[0]; @@ -156,6 +174,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { clearOutput(); inputStreamEnded = false; outputStreamEnded = false; + finalStreamEndPositionUs = C.TIME_UNSET; if (decoderReplacementState != REPLACEMENT_STATE_NONE) { replaceDecoder(); } else { @@ -166,6 +185,13 @@ public final class TextRenderer extends BaseRenderer implements Callback { @Override public void render(long positionUs, long elapsedRealtimeUs) { + if (isCurrentStreamFinal() + && finalStreamEndPositionUs != C.TIME_UNSET + && positionUs >= finalStreamEndPositionUs) { + releaseBuffers(); + outputStreamEnded = true; + } + if (outputStreamEnded) { return; } @@ -278,6 +304,7 @@ public final class TextRenderer extends BaseRenderer implements Callback { @Override protected void onDisabled() { streamFormat = null; + finalStreamEndPositionUs = C.TIME_UNSET; clearOutput(); releaseDecoder(); }