Skip to content

Commit

Permalink
Pass the end position of a stream to TextRenderer when marking it final
Browse files Browse the repository at this point in the history
In a period transition we pass the start time of the next period, for
the final period we pass the duration of the period or timeline, if
known.

This means sideloaded subtitles now respect the end point of
ClippingMediaSource and ensures that content subtitles aren't
incorrectly displayed over mid-roll ads.

When transitioning back into the subtitled content the subtitles still
appear slightly before the video transitions, meaning the first subtitle
of the content is shown with the last few frames of the ad. Resolving
this in a way that doesn't break anything else requires a deeper
investigation.

Issue: google#5317
Issue: google#8456
#minor-release
PiperOrigin-RevId: 361797118
  • Loading branch information
icbaker authored and roblav96 committed Apr 17, 2021
1 parent 413826f commit bf7c97a
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 8 deletions.
13 changes: 10 additions & 3 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,20 @@
* Text
* Parse SSA/ASS bold & italic info in `Style:` lines
([#8435](https:/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:/google/ExoPlayer/issues/5317),
[#8456](https:/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)

Expand Down
Original file line number Diff line number Diff line change
@@ -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<SimpleExoPlayer> 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<MediaItem> 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<SimpleExoPlayer> 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<List<Cue>> cues;

private CapturingTextOutput() {
cues = new ArrayList<>();
}

@Override
public void onCues(List<Cue> cues) {
this.cues.add(cues);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
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;
Expand Down Expand Up @@ -1940,7 +1941,12 @@ private void maybeUpdateReadingPeriod() {
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);
}
}
}
Expand All @@ -1965,7 +1971,8 @@ private void maybeUpdateReadingPeriod() {
&& 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++) {
Expand All @@ -1981,7 +1988,9 @@ private void maybeUpdateReadingPeriod() {
// 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());
}
}
}
Expand Down Expand Up @@ -2106,14 +2115,21 @@ private boolean hasReadingPeriodFinishedReading() {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -121,6 +123,7 @@ public TextRenderer(
outputLooper == null ? null : Util.createHandler(outputLooper, /* callback= */ this);
this.decoderFactory = decoderFactory;
formatHolder = new FormatHolder();
finalStreamEndPositionUs = C.TIME_UNSET;
}

@Override
Expand All @@ -141,6 +144,21 @@ public int supportsFormat(Format format) {
}
}

/**
* Sets the position at which to stop rendering the current stream.
*
* <p>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];
Expand All @@ -156,6 +174,7 @@ protected void onPositionReset(long positionUs, boolean joining) {
clearOutput();
inputStreamEnded = false;
outputStreamEnded = false;
finalStreamEndPositionUs = C.TIME_UNSET;
if (decoderReplacementState != REPLACEMENT_STATE_NONE) {
replaceDecoder();
} else {
Expand All @@ -166,6 +185,13 @@ protected void onPositionReset(long positionUs, boolean joining) {

@Override
public void render(long positionUs, long elapsedRealtimeUs) {
if (isCurrentStreamFinal()
&& finalStreamEndPositionUs != C.TIME_UNSET
&& positionUs >= finalStreamEndPositionUs) {
releaseBuffers();
outputStreamEnded = true;
}

if (outputStreamEnded) {
return;
}
Expand Down Expand Up @@ -278,6 +304,7 @@ public void render(long positionUs, long elapsedRealtimeUs) {
@Override
protected void onDisabled() {
streamFormat = null;
finalStreamEndPositionUs = C.TIME_UNSET;
clearOutput();
releaseDecoder();
}
Expand Down

0 comments on commit bf7c97a

Please sign in to comment.