Skip to content

Commit fa71bf2

Browse files
committed
Wait for first frame when using placeholder surface
We currently pretend to be ready when using a placeholder surface irrespective of whether the renderer is actually ready to immediately render when a surface is attached. This causes issues in two ways: - Apps don't know when a player without a surface can be connected to a surface and immediately start playing - A paused player without a surface will use the 1 second default doSomeWork loop, causing any pending decoding to be extremely slow (see Issue: #1973). The fix is to let the placeholder surface case use the same paths as the regular surface and with marking the first frame as available as soon as the codec output buffer for it is pending and ready for release. PiperOrigin-RevId: 737942496 (cherry picked from commit eef678f)
1 parent e2017e3 commit fa71bf2

File tree

6 files changed

+107
-18
lines changed

6 files changed

+107
-18
lines changed

RELEASENOTES.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
# Release notes
22

3-
### Unreleased changes
4-
5-
6-
73
## 1.6
84

95
### 1.6.0-rc02 (2025-03-18)
@@ -18,6 +14,10 @@ This release includes the following changes since
1814
* Audio:
1915
* Add support for float PCM to `ChannelMappingAudioProcessor`.
2016
* Add support for float PCM to `TrimmingAudioProcessor`.
17+
* Video:
18+
* Fix issue where a player without a surface was ready immediately and
19+
very slow decoding any pending frames
20+
([#1973](https://github.com/androidx/media/issues/1973)).
2121
* Session:
2222
* Fix bug where a stale notification stays visible when the playlist is
2323
cleared ([#2211](https://github.com/androidx/media/issues/2211)).

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/DefaultVideoSink.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public void flush(boolean resetPosition) {
129129

130130
@Override
131131
public boolean isReady(boolean rendererOtherwiseReady) {
132-
return outputSurface == null || videoFrameReleaseControl.isReady(rendererOtherwiseReady);
132+
return videoFrameReleaseControl.isReady(rendererOtherwiseReady);
133133
}
134134

135135
@Override

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/MediaCodecVideoRenderer.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -990,7 +990,7 @@ public boolean isReady() {
990990
if (videoSink != null) {
991991
return videoSink.isReady(rendererOtherwiseReady);
992992
}
993-
if (rendererOtherwiseReady && (getCodec() == null || displaySurface == null || tunneling)) {
993+
if (rendererOtherwiseReady && (getCodec() == null || tunneling)) {
994994
// Not releasing frames.
995995
return true;
996996
}
@@ -1157,24 +1157,24 @@ private void setOutput(@Nullable Object output) throws ExoPlaybackException {
11571157
if (displaySurface != null) {
11581158
// If we know the video size, report it again immediately.
11591159
maybeRenotifyVideoSizeChanged();
1160-
if (state == STATE_STARTED) {
1161-
// We want to "join" playback to prevent an intermediate buffering state in the player
1162-
// before we rendered the new first frame. Since there is no reason to believe the next
1163-
// frame is delayed and the renderer needs to catch up, we still request to render the
1164-
// next frame as soon as possible.
1165-
if (videoSink != null) {
1166-
videoSink.join(/* renderNextFrameImmediately= */ true);
1167-
} else {
1168-
videoFrameReleaseControl.join(/* renderNextFrameImmediately= */ true);
1169-
}
1170-
}
11711160
} else {
11721161
// The display surface has been removed.
11731162
reportedVideoSize = null;
11741163
if (videoSink != null) {
11751164
videoSink.clearOutputSurfaceInfo();
11761165
}
11771166
}
1167+
if (state == STATE_STARTED) {
1168+
// We want to "join" playback to prevent an intermediate buffering state in the player
1169+
// before we rendered the new first frame. Since there is no reason to believe the next
1170+
// frame is delayed and the renderer needs to catch up, we still request to render the
1171+
// next frame as soon as possible.
1172+
if (videoSink != null) {
1173+
videoSink.join(/* renderNextFrameImmediately= */ true);
1174+
} else {
1175+
videoFrameReleaseControl.join(/* renderNextFrameImmediately= */ true);
1176+
}
1177+
}
11781178
maybeSetupTunnelingForFirstFrame();
11791179
} else if (displaySurface != null) {
11801180
// The display surface is set and unchanged. If we know the video size and/or have already

libraries/exoplayer/src/main/java/androidx/media3/exoplayer/video/VideoFrameReleaseControl.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ boolean shouldIgnoreFrame(
181181
private float playbackSpeed;
182182
private Clock clock;
183183
private boolean hasOutputSurface;
184+
private boolean frameReadyWithoutSurface;
184185

185186
/**
186187
* Creates an instance.
@@ -242,6 +243,7 @@ public void onProcessedStreamChange() {
242243
/** Called when the display surface changed. */
243244
public void setOutputSurface(@Nullable Surface outputSurface) {
244245
hasOutputSurface = outputSurface != null;
246+
frameReadyWithoutSurface = false;
245247
frameReleaseHelper.onSurfaceChanged(outputSurface);
246248
lowerFirstFrameState(C.FIRST_FRAME_NOT_RENDERED);
247249
}
@@ -288,7 +290,9 @@ public void allowReleaseFirstFrameBeforeStarted() {
288290
* @return Whether the release control is ready.
289291
*/
290292
public boolean isReady(boolean rendererOtherwiseReady) {
291-
if (rendererOtherwiseReady && firstFrameState == C.FIRST_FRAME_RENDERED) {
293+
if (rendererOtherwiseReady
294+
&& (firstFrameState == C.FIRST_FRAME_RENDERED
295+
|| (!hasOutputSurface && frameReadyWithoutSurface))) {
292296
// Ready. If we were joining then we've now joined, so clear the joining deadline.
293297
joiningDeadlineMs = C.TIME_UNSET;
294298
return true;
@@ -364,6 +368,7 @@ public void join(boolean renderNextFrameImmediately) {
364368
return FRAME_RELEASE_SKIP;
365369
}
366370
if (!hasOutputSurface) {
371+
frameReadyWithoutSurface = true;
367372
// Skip frames in sync with playback, so we'll be at the right frame if a surface is set.
368373
if (frameTimingEvaluator.shouldIgnoreFrame(
369374
frameReleaseInfo.earlyUs,

libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/MediaCodecVideoRendererTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static androidx.media3.test.utils.TestUtil.createByteArray;
2727
import static com.google.common.truth.Truth.assertThat;
2828
import static org.junit.Assert.assertThrows;
29+
import static org.mockito.ArgumentMatchers.any;
2930
import static org.mockito.ArgumentMatchers.anyLong;
3031
import static org.mockito.ArgumentMatchers.eq;
3132
import static org.mockito.Mockito.never;
@@ -416,6 +417,49 @@ public void render_earlyWithoutSurfaceAndNotStarted_doesNotSkipBuffer() throws E
416417
assertThat(argumentDecoderCounters.getValue().skippedOutputBufferCount).isEqualTo(0);
417418
}
418419

420+
@Test
421+
public void render_withNewSurfaceAndAlreadyReadyWithPlaceholder_canRenderFirstFrameImmediately()
422+
throws Exception {
423+
FakeSampleStream fakeSampleStream =
424+
new FakeSampleStream(
425+
new DefaultAllocator(/* trimOnReset= */ true, /* individualAllocationSize= */ 1024),
426+
/* mediaSourceEventDispatcher= */ null,
427+
DrmSessionManager.DRM_UNSUPPORTED,
428+
new DrmSessionEventListener.EventDispatcher(),
429+
/* initialFormat= */ VIDEO_H264,
430+
ImmutableList.of(
431+
oneByteSample(/* timeUs= */ 0, C.BUFFER_FLAG_KEY_FRAME),
432+
oneByteSample(/* timeUs= */ 10_000),
433+
oneByteSample(/* timeUs= */ 20_000),
434+
oneByteSample(/* timeUs= */ 30_000),
435+
END_OF_STREAM_ITEM));
436+
fakeSampleStream.writeData(/* startPositionUs= */ 0);
437+
// Set placeholder surface.
438+
mediaCodecVideoRenderer.handleMessage(Renderer.MSG_SET_VIDEO_OUTPUT, null);
439+
// Enable at a non-zero start position to ensure the renderer isn't ready with one render call.
440+
mediaCodecVideoRenderer.enable(
441+
RendererConfiguration.DEFAULT,
442+
new Format[] {VIDEO_H264},
443+
fakeSampleStream,
444+
/* positionUs= */ 0,
445+
/* joining= */ false,
446+
/* mayRenderStartOfStream= */ true,
447+
/* startPositionUs= */ 20_000,
448+
/* offsetUs= */ 0,
449+
/* mediaPeriodId= */ new MediaSource.MediaPeriodId(new Object()));
450+
while (!mediaCodecVideoRenderer.isReady()) {
451+
mediaCodecVideoRenderer.render(/* positionUs= */ 0, SystemClock.elapsedRealtime() * 1000);
452+
}
453+
ShadowLooper.idleMainLooper(); // Ensure all pending events are delivered.
454+
verify(eventListener, never()).onRenderedFirstFrame(any(), anyLong());
455+
456+
mediaCodecVideoRenderer.handleMessage(Renderer.MSG_SET_VIDEO_OUTPUT, surface);
457+
mediaCodecVideoRenderer.render(/* positionUs= */ 0, SystemClock.elapsedRealtime() * 1000);
458+
ShadowLooper.idleMainLooper(); // Ensure all pending events are delivered.
459+
460+
verify(eventListener).onRenderedFirstFrame(any(), anyLong());
461+
}
462+
419463
@Test
420464
public void render_withBufferLimitEqualToNumberOfSamples_rendersLastFrameAfterEndOfStream()
421465
throws Exception {

libraries/exoplayer/src/test/java/androidx/media3/exoplayer/video/VideoFrameReleaseControlTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,46 @@ public void isReady_afterReleasingFrame_returnsTrue() {
6060
assertThat(videoFrameReleaseControl.isReady(/* rendererOtherwiseReady= */ true)).isTrue();
6161
}
6262

63+
@Test
64+
public void isReady_withoutSurfaceFirstFrameNotReady_returnsFalse() throws Exception {
65+
VideoFrameReleaseControl.FrameReleaseInfo frameReleaseInfo =
66+
new VideoFrameReleaseControl.FrameReleaseInfo();
67+
VideoFrameReleaseControl videoFrameReleaseControl = createVideoFrameReleaseControl();
68+
videoFrameReleaseControl.setOutputSurface(/* outputSurface= */ null);
69+
70+
// Process decode-only frame to ensure it doesn't make the release control ready.
71+
videoFrameReleaseControl.getFrameReleaseAction(
72+
/* presentationTimeUs= */ 0,
73+
/* positionUs= */ 0,
74+
/* elapsedRealtimeUs= */ 0,
75+
/* outputStreamStartPositionUs= */ 0,
76+
/* isDecodeOnlyFrame= */ true,
77+
/* isLastFrame= */ false,
78+
frameReleaseInfo);
79+
80+
assertThat(videoFrameReleaseControl.isReady(/* otherwiseReady= */ true)).isFalse();
81+
}
82+
83+
@Test
84+
public void isReady_withoutSurfaceFirstFrameReady_returnsFalse() throws Exception {
85+
VideoFrameReleaseControl.FrameReleaseInfo frameReleaseInfo =
86+
new VideoFrameReleaseControl.FrameReleaseInfo();
87+
VideoFrameReleaseControl videoFrameReleaseControl = createVideoFrameReleaseControl();
88+
videoFrameReleaseControl.setOutputSurface(/* outputSurface= */ null);
89+
90+
// Process first frame.
91+
videoFrameReleaseControl.getFrameReleaseAction(
92+
/* presentationTimeUs= */ 0,
93+
/* positionUs= */ 0,
94+
/* elapsedRealtimeUs= */ 0,
95+
/* outputStreamStartPositionUs= */ 0,
96+
/* isDecodeOnlyFrame= */ false,
97+
/* isLastFrame= */ false,
98+
frameReleaseInfo);
99+
100+
assertThat(videoFrameReleaseControl.isReady(/* otherwiseReady= */ true)).isTrue();
101+
}
102+
63103
@Test
64104
public void isReady_withinJoiningDeadlineWhenRenderingNextFrameImmediately_returnsTrue() {
65105
FakeClock clock = new FakeClock(/* isAutoAdvancing= */ false);

0 commit comments

Comments
 (0)