Skip to content

Commit

Permalink
Fix NPE when reading from a SampleQueue from a loading thread
Browse files Browse the repository at this point in the history
Issue: #7273
PiperOrigin-RevId: 308238035
  • Loading branch information
AquilesCanta authored and ojw28 committed May 27, 2020
1 parent 7ee08f0 commit 49e5e66
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 15 deletions.
3 changes: 2 additions & 1 deletion RELEASENOTES.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
<<<<<<< HEAD
# Release notes #

### Next release ###
Expand All @@ -14,6 +13,8 @@
`http://dashif.org/guidelines/trickmode`) into the same `TrackGroup` as
the main adaptation sets to which they refer. Trick play tracks are
marked with the `C.ROLE_FLAG_TRICK_PLAY` flag.
* Fix assertion failure in `SampleQueue` when playing DASH streams with
EMSG tracks ([#7273](https://github.com/google/ExoPlayer/issues/7273)).
* MPEG-TS: Fix issue where SEI NAL units were incorrectly dropped from H.265
samples ([#7113](https://github.com/google/ExoPlayer/issues/7113)).
* Text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,8 @@ private TrackOutput prepareTrackOutput(TrackId id) {
return sampleQueues[i];
}
}
SampleQueue trackOutput = new SampleQueue(allocator, drmSessionManager);
SampleQueue trackOutput = new SampleQueue(
allocator, /* playbackLooper= */ handler.getLooper(), drmSessionManager);
trackOutput.setUpstreamFormatChangeListener(this);
@NullableType
TrackId[] sampleQueueTrackIds = Arrays.copyOf(this.sampleQueueTrackIds, trackCount + 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public interface UpstreamFormatChangedListener {
private final SampleExtrasHolder extrasHolder;
private final DrmSessionManager<?> drmSessionManager;
private UpstreamFormatChangedListener upstreamFormatChangeListener;
private final Looper playbackLooper;

@Nullable private Format downstreamFormat;
@Nullable private DrmSession<?> currentDrmSession;
Expand Down Expand Up @@ -91,11 +92,13 @@ public interface UpstreamFormatChangedListener {
* Creates a sample queue.
*
* @param allocator An {@link Allocator} from which allocations for sample data can be obtained.
* @param playbackLooper The looper associated with the media playback thread.
* @param drmSessionManager The {@link DrmSessionManager} to obtain {@link DrmSession DrmSessions}
* from. The created instance does not take ownership of this {@link DrmSessionManager}.
*/
public SampleQueue(Allocator allocator, DrmSessionManager<?> drmSessionManager) {
public SampleQueue(Allocator allocator, Looper playbackLooper, DrmSessionManager<?> drmSessionManager) {
sampleDataQueue = new SampleDataQueue(allocator);
this.playbackLooper = playbackLooper;
this.drmSessionManager = drmSessionManager;
extrasHolder = new SampleExtrasHolder();
capacity = SAMPLE_CAPACITY_INCREMENT;
Expand Down Expand Up @@ -789,8 +792,7 @@ private void onFormatResult(Format newFormat, FormatHolder outputFormatHolder) {
}
// Ensure we acquire the new session before releasing the previous one in case the same session
// is being used for both DrmInitData.
DrmSession<?> previousSession = currentDrmSession;
Looper playbackLooper = Assertions.checkNotNull(Looper.myLooper());
@Nullable DrmSession previousSession = currentDrmSession;
currentDrmSession =
newDrmInitData != null
? drmSessionManager.acquireSession(playbackLooper, newDrmInitData)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.google.android.exoplayer2.source.chunk;

import android.os.Looper;
import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.Format;
Expand Down Expand Up @@ -130,13 +131,19 @@ public ChunkSampleStream(
int[] trackTypes = new int[1 + embeddedTrackCount];
SampleQueue[] sampleQueues = new SampleQueue[1 + embeddedTrackCount];

primarySampleQueue = new SampleQueue(allocator, drmSessionManager);
primarySampleQueue = new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
drmSessionManager);
trackTypes[0] = primaryTrackType;
sampleQueues[0] = primarySampleQueue;

for (int i = 0; i < embeddedTrackCount; i++) {
SampleQueue sampleQueue =
new SampleQueue(allocator, DrmSessionManager.getDummyDrmSessionManager());
new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
DrmSessionManager.getDummyDrmSessionManager());
embeddedSampleQueues[i] = sampleQueue;
sampleQueues[i + 1] = sampleQueue;
trackTypes[i + 1] = embeddedTrackTypes[i];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static org.junit.Assert.assertArrayEquals;
import static org.mockito.Mockito.when;

import android.os.Looper;
import androidx.annotation.Nullable;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.android.exoplayer2.C;
Expand All @@ -40,6 +41,7 @@
import com.google.android.exoplayer2.testutil.TestUtil;
import com.google.android.exoplayer2.upstream.Allocator;
import com.google.android.exoplayer2.upstream.DefaultAllocator;
import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.ParsableByteArray;
import java.io.IOException;
import java.util.Arrays;
Expand Down Expand Up @@ -143,7 +145,10 @@ public void setUp() {
mockDrmSession = (DrmSession<ExoMediaCrypto>) Mockito.mock(DrmSession.class);
when(mockDrmSessionManager.acquireSession(ArgumentMatchers.any(), ArgumentMatchers.any()))
.thenReturn(mockDrmSession);
sampleQueue = new SampleQueue(allocator, mockDrmSessionManager);
sampleQueue = new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
mockDrmSessionManager);
formatHolder = new FormatHolder();
inputBuffer = new DecoderInputBuffer(DecoderInputBuffer.BUFFER_REPLACEMENT_MODE_NORMAL);
}
Expand Down Expand Up @@ -360,7 +365,10 @@ public void testIsReadyReturnsTrueForValidDrmSession() {
public void testIsReadyReturnsTrueForClearSampleAndPlayClearSamplesWithoutKeysIsTrue() {
when(mockDrmSession.playClearSamplesWithoutKeys()).thenReturn(true);
// We recreate the queue to ensure the mock DRM session manager flags are taken into account.
sampleQueue = new SampleQueue(allocator, mockDrmSessionManager);
sampleQueue = new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
mockDrmSessionManager);
writeTestDataWithEncryptedSections();
assertThat(sampleQueue.isReady(/* loadingFinished= */ false)).isTrue();
}
Expand Down Expand Up @@ -542,7 +550,10 @@ public void testReadWithErrorSessionReadsNothingAndThrows() throws IOException {
public void testAllowPlayClearSamplesWithoutKeysReadsClearSamples() {
when(mockDrmSession.playClearSamplesWithoutKeys()).thenReturn(true);
// We recreate the queue to ensure the mock DRM session manager flags are taken into account.
sampleQueue = new SampleQueue(allocator, mockDrmSessionManager);
sampleQueue = new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
mockDrmSessionManager);
when(mockDrmSession.getState()).thenReturn(DrmSession.STATE_OPENED);
writeTestDataWithEncryptedSections();

Expand Down Expand Up @@ -931,7 +942,10 @@ public void testSetSampleOffsetBetweenSamples() {
public void testAdjustUpstreamFormat() {
String label = "label";
sampleQueue =
new SampleQueue(allocator, mockDrmSessionManager) {
new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
mockDrmSessionManager) {
@Override
public Format getAdjustedUpstreamFormat(Format format) {
return super.getAdjustedUpstreamFormat(format.copyWithLabel(label));
Expand All @@ -947,7 +961,10 @@ public Format getAdjustedUpstreamFormat(Format format) {
public void testInvalidateUpstreamFormatAdjustment() {
AtomicReference<String> label = new AtomicReference<>("label1");
sampleQueue =
new SampleQueue(allocator, mockDrmSessionManager) {
new SampleQueue(
allocator,
/* playbackLooper= */ Assertions.checkNotNull(Looper.myLooper()),
mockDrmSessionManager) {
@Override
public Format getAdjustedUpstreamFormat(Format format) {
return super.getAdjustedUpstreamFormat(format.copyWithLabel(label.get()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,10 @@ public final class PlayerTrackEmsgHandler implements TrackOutput {
private final MetadataInputBuffer buffer;

/* package */ PlayerTrackEmsgHandler(Allocator allocator) {
this.sampleQueue = new SampleQueue(allocator, DrmSessionManager.getDummyDrmSessionManager());
this.sampleQueue = new SampleQueue(
allocator,
/* playbackLooper= */ handler.getLooper(),
DrmSessionManager.getDummyDrmSessionManager());
formatHolder = new FormatHolder();
buffer = new MetadataInputBuffer();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import android.net.Uri;
import android.os.Handler;
import android.os.Looper;
import android.util.SparseIntArray;
import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C;
Expand Down Expand Up @@ -907,7 +908,11 @@ private SampleQueue createSampleQueue(int id, int type) {

boolean isAudioVideo = type == C.TRACK_TYPE_AUDIO || type == C.TRACK_TYPE_VIDEO;
FormatAdjustingSampleQueue trackOutput =
new FormatAdjustingSampleQueue(allocator, drmSessionManager, overridingDrmInitData);
new FormatAdjustingSampleQueue(
allocator,
/* playbackLooper= */ handler.getLooper(),
drmSessionManager,
overridingDrmInitData);
if (isAudioVideo) {
trackOutput.setDrmInitData(drmInitData);
}
Expand Down Expand Up @@ -1331,9 +1336,10 @@ private static final class FormatAdjustingSampleQueue extends SampleQueue {

public FormatAdjustingSampleQueue(
Allocator allocator,
Looper playbackLooper,
DrmSessionManager<?> drmSessionManager,
Map<String, DrmInitData> overridingDrmInitData) {
super(allocator, drmSessionManager);
super(allocator, playbackLooper, drmSessionManager);
this.overridingDrmInitData = overridingDrmInitData;
}

Expand Down

0 comments on commit 49e5e66

Please sign in to comment.