Skip to content

Fix ProgressiveMediaPeriod hang#3235

Open
emveepee wants to merge 1 commit into
androidx:mainfrom
emveepee:progressive
Open

Fix ProgressiveMediaPeriod hang#3235
emveepee wants to merge 1 commit into
androidx:mainfrom
emveepee:progressive

Conversation

@emveepee
Copy link
Copy Markdown

Fix ProgressiveMediaPeriod hang when MPEG-TS PMT declares a track that carries no PES data

When an MPEG-TS file declares an elementary stream in the PMT but that stream carries zero PES packets (e.g. a "visual impaired commentary" audio service that is listed in the PMT but not actually broadcast), ProgressiveMediaPeriod.maybeFinishPrepare() loops returning early indefinitely because sampleQueue.getUpstreamFormat() == null for that queue. The player times out with StuckPlayerException: Player
stuck buffering and not loading after 4 seconds. This is a real-world problem with DVB-T2 and ATSC recordings.

This was previously reported as ExoPlayer#7873 and addressed by ExoPlayer PR#8063 (closed without merge). The bug is unchanged in media3 1.10.1.

Fix (single file: ProgressiveMediaPeriod.java):

  1. Move handler initialization before the onContinueLoadingRequestedRunnable lambda in the constructor (required by Java's definite-assignment rule for the final field).
  2. In onContinueLoadingRequestedRunnable, post maybeFinishPrepareRunnable before calling callback.onContinueLoadingRequested(). This runnable is posted by the loading thread every 1 MB (the DEFAULT_LOADING_CHECK_INTERVAL_BYTES pacing loop); posting our check first means it runs while loadCondition is still closed and the loading thread is blocked.
  3. In onLoadCompleted(), post maybeFinishPrepareRunnable after setting loadingFinished = true, covering files that reach EOF before the buffer fills.
  4. In maybeFinishPrepare(), replace the unconditional return for a null-format queue with a three-branch guard: (a) if loadingFinished — loading is done, no format will ever arrive; (b) if !loadCondition.isOpen() and at least one real video and one real audio track have reported formats — loading is paused at a checkpoint and the null-format queue is provably an empty PMT-declared track; (c) otherwise —
    still loading, wait. In cases (a) and (b), assign an empty Format.Builder().build() placeholder so preparation can complete on the tracks that do carry data.

The placeholder has a null sampleMimeType, which DefaultTrackSelector leaves unselected and which trackIsAudioVideoFlags marks false (excluded from buffering calculations). No renderer claims it. The happy path (all queues have formats) is unchanged — maybeFinishPrepare() returns immediately at the prepared guard on any extra invocation.

@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 23, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@emveepee
Copy link
Copy Markdown
Author

Because I have been using the earlier PR with my own diff for sometime I asked Claude to create an analysis why this change is needed below since it doesn't think it is a TSExtractor problem

PR Case: Fix ProgressiveMediaPeriod hanging when MPEG-TS PMT declares a track that carries no PES data

Problem Statement

When an MPEG-TS file declares an elementary stream in the PMT (Program Map Table) but that
stream carries zero PES packets, ProgressiveMediaPeriod enters a permanent wait and playback
never starts. The symptom is:

ExoPlayerImplInternal: Caused by: java.lang.IllegalStateException: Playback stuck buffering and not loading.

This is a real-world problem with recordings from DVB-T2 and ATSC broadcasts. The file
notplayable.ts in this repository reproduces the issue.

Evidence: ffprobe -i notplayable.ts -hide_banner

Input #0, mpegts, from 'notplayable.ts':
  Duration: 00:03:08.08, start: 48949.827656, bitrate: 4460 kb/s
  Program 40976
  Stream #0:0[0xc9]: Video: h264 (High) ..., 1920x1080, 25 fps
  Stream #0:1[0xca](eng): Audio: aac_latm (LC) ..., 48000 Hz, stereo
  Stream #0:2[0xcc](eng): Unknown: none ([17][0][0][0] / 0x0011) (visual impaired) (descriptions) (dependent)
  Stream #0:3[0xcb](eng): Subtitle: dvb_subtitle

Stream #0:2 (PID 0xcc) is declared as stream type 0x11 (AAC-LATM) in the PMT — the same
type as the real audio track — but carries no PES packets. ffprobe reports "Unknown: none" with
no codec parameters because it never receives any data for that PID.

This is common when a broadcaster includes a "visual impaired commentary" audio service in the
PMT but does not actually transmit it. The same pattern appears on ATSC multiplexes where a
secondary audio language is declared but not broadcast.

Code path that hangs

  1. TsExtractor.PmtReader.consume() parses the PMT and, for stream type 0x11, calls
    DefaultTsPayloadReaderFactory.createPayloadReader() which returns a
    PesReader(new LatmReader(...)).
  2. PesReader.init() calls reader.createTracks(extractorOutput, idGenerator), allocating a
    SampleQueue for PID 0xcc.
  3. No PES packets ever arrive for PID 0xcc. LatmReader never calls output.format(). The
    queue's getUpstreamFormat() remains null.
  4. endTracks() is called (PMT fully parsed). ProgressiveMediaPeriod.maybeFinishPrepare() is
    posted to the handler.
  5. maybeFinishPrepare() checks every sample queue for a non-null upstream format:
    for (SampleQueue sampleQueue : sampleQueues) {
        if (sampleQueue.getUpstreamFormat() == null) {
            return;  // <-- always returns here for the empty track
        }
    }
  6. After loading finishes (RESULT_END_OF_INPUT), onLoadCompleted() is called. It does NOT
    re-post maybeFinishPrepareRunnable, so the prepare gate is never revisited with the
    knowledge that loading is done. The player eventually times out with "stuck buffering".

This code path is still fully present in media3 1.10.1.


History

Original ExoPlayer PR: google/ExoPlayer#8063
"Progressive Media: Handle empty audio track" by bennettpeter (closed 2020).

Related ExoPlayer issue: google/ExoPlayer#7873
"PMT declares a track which is not present in the file" — referenced by the original reviewers
as the canonical issue; a proper fix was never merged in ExoPlayer and does not appear to have
landed in the androidx/media transition either.

Why the original PR was closed

The ExoPlayer maintainers had two objections:

  1. Wrong layer. ProgressiveMediaPeriod is format-agnostic; the problem is MPEG-TS
    specific. andrewlewis suggested the fix belongs in TsExtractor — e.g. suppress a declared
    stream from the track list if it never produces PES data.
  2. Race condition. AquilesCanta noted that the "uninitialized track" check could fire before
    a legitimate audio format call arrives, misidentifying a real-but-slow track as empty.

The original patch also had implementation issues: it spun Thread.sleep(3000) on the handler
thread and used mutable instance state (nullStreamCount, videoFound, audioFound) that was
not reset on retries.

Status in current media3

The core bug is unchanged. ProgressiveMediaPeriod.maybeFinishPrepare() (currently at line 904)
still returns unconditionally if any sample queue has a null upstream format, with no escape
path when loading has fully completed. onLoadCompleted() does not re-trigger
maybeFinishPrepare() after setting loadingFinished = true.


Proposed Fix

File: libraries/exoplayer/src/main/java/androidx/media3/exoplayer/source/ProgressiveMediaPeriod.java

Three minimal changes. Two add a handler.post(maybeFinishPrepareRunnable) call at points where
preparation might now be unblockable; the third replaces the unconditional null-format early-return
in maybeFinishPrepare() with a two-condition guard.

Change 1 — Move handler initialization before the onContinueLoadingRequestedRunnable lambda

The handler must be assigned before the lambda that uses it, so we swap two lines in the constructor:

loadCondition = new ConditionVariable();
handler = Util.createHandlerForCurrentLooper();  // moved up
maybeFinishPrepareRunnable = this::maybeFinishPrepare;
onContinueLoadingRequestedRunnable = ...;
// (handler assignment removed from here)

This is a no-op behavior change; it only satisfies Java's definite-assignment rule for the final
field used inside the lambda below.

Change 2 — Trigger maybeFinishPrepare on every loading checkpoint

// In onContinueLoadingRequestedRunnable (posted by the loading thread every
// DEFAULT_LOADING_CHECK_INTERVAL_BYTES = 1 MB as a pacing mechanism):
onContinueLoadingRequestedRunnable =
    () -> {
      if (!released) {
        handler.post(maybeFinishPrepareRunnable);   // NEW — runs before MSG_DO_SOME_WORK
        checkNotNull(callback).onContinueLoadingRequested(ProgressiveMediaPeriod.this);
      }
    };

Why this is needed for large files: loadingFinished is only set when RESULT_END_OF_INPUT
is returned. For a large MPEG-TS recording the 4-second StuckPlayerException fires long before
EOF is reached, so the Change 3 / loadingFinished path never triggers in time.

The loading thread in ExtractingLoadable.load() implements a pacing loop: after every
DEFAULT_LOADING_CHECK_INTERVAL_BYTES (1 MB) it calls loadCondition.close() and posts
onContinueLoadingRequestedRunnable, then blocks at loadCondition.block() until
continueLoading() reopens it. Our addition posts maybeFinishPrepareRunnable before
callback.onContinueLoadingRequested(), so the prepare check runs while loadCondition is
still closed and the loading thread is blocked — which is the stable window the
!loadCondition.isOpen() guard requires.

Change 3 — Trigger maybeFinishPrepare when loading finishes (small files / EOS)

// In onLoadCompleted(), after setting loadingFinished = true:
loadingFinished = true;
handler.post(maybeFinishPrepareRunnable);   // NEW
checkNotNull(callback).onContinueLoadingRequested(this);

For smaller files that reach RESULT_END_OF_INPUT before the buffer fills, loadingFinished
becomes true but nothing re-posts the prepare runnable. Without this line continueLoading()
returns false immediately (loading is done) and maybeFinishPrepare() is never revisited.

Change 4 — Replace unconditional null-format return with a two-condition guard

// In maybeFinishPrepare(), replace the single `return`:
for (SampleQueue sampleQueue : sampleQueues) {
    if (sampleQueue.getUpstreamFormat() == null) {
        if (loadingFinished) {
            // File fully exhausted; this track never produced any data.
        } else if (!loadCondition.isOpen()) {
            // Loading is paused (byte budget full). Only assign a placeholder if at least
            // one real video and one real audio track have already been seen, confirming
            // this is an extra PMT-declared stream that carries no data rather than a
            // legitimate track whose format packet hasn't arrived yet.
            boolean haveVideo = false;
            boolean haveAudio = false;
            for (SampleQueue q : sampleQueues) {
                Format f = q.getUpstreamFormat();
                if (f == null) continue;
                if (MimeTypes.isVideo(f.sampleMimeType)) haveVideo = true;
                else if (MimeTypes.isAudio(f.sampleMimeType)) haveAudio = true;
            }
            if (!haveVideo || !haveAudio) {
                return;
            }
        } else {
            // Actively loading; a format may still arrive from the loading thread.
            return;
        }
        sampleQueue.format(new Format.Builder().build());
    }
}

The placeholder Format has a null sampleMimeType. This means:

  • MimeTypes.getTrackType(null) returns C.TRACK_TYPE_UNKNOWN
  • trackIsAudioVideoFlags[i] = false → the track is excluded from AV buffering calculations
  • No renderer claims to support it → DefaultTrackSelector leaves it unselected
  • If the app's UI surfaces track groups, it appears as an unknown/other type, not as audio

How this addresses the original review objections

Objection 1 — Wrong layer (should be in TsExtractor).

A TsExtractor-level fix would require one of:

  • Deferring createTracks() until the first PES byte arrives, then restructuring when
    endTracks() is allowed to be called (significant architecture change), or
  • Adding a maybeOutputPlaceholderFormat() escape hatch to the ElementaryStreamReader
    interface (or to each concrete reader) so TsExtractor can call it at EOS — touching ~12 reader
    classes and a public @UnstableApi interface.

The ProgressiveMediaPeriod fix is simpler and achieves the same observable result. It is also
format-agnostic, which is appropriate: any container format could theoretically declare a track
that never produces data (MP4 with an empty track box, for instance). A future TsExtractor fix
could suppress the empty track earlier in the pipeline, making this fallback moot for that case
while leaving the safety net in place for other formats.

Objection 2 — Race condition (format call might arrive after the check).

The two guards together eliminate the race:

loadingFinished path (Change 3): loadingFinished is set to true in onLoadCompleted(),
which the Loader fires only after the loading thread has returned RESULT_END_OF_INPUT. The
loading thread calls sampleQueue.format() as it processes each PES packet — always before
returning RESULT_END_OF_INPUT. Both onLoadCompleted and maybeFinishPrepareRunnable are
delivered on the same handler thread, in order. By the time the handler sees loadingFinished = true, all format calls from the loading thread have already been applied.

!loadCondition.isOpen() path (Change 2): The loading thread closes loadCondition itself
in ExtractingLoadable.load() every 1 MB, then immediately blocks at loadCondition.block().
maybeFinishPrepareRunnable is posted before callback.onContinueLoadingRequested(), so it
executes on the handler thread while the loading thread is blocked — the loading thread cannot
call sampleQueue.format() while it is blocked, so no format can arrive between the
!loadCondition.isOpen() check and the sampleQueue.format(placeholder) call. The additional
haveVideo && haveAudio guard ensures we only stamp a placeholder when we have already seen the
primary tracks — ruling out the pathological case where the very first 1 MB checkpoint fires
before any format has arrived for any track.


Test Coverage

Existing tests that exercise similar paths

  • libraries/extractor/src/test/java/androidx/media3/extractor/ts/TsExtractorTest.java
    covers PMT parsing and track extraction for known stream types, but does not test the
    empty-track case.
  • libraries/exoplayer/src/test/java/androidx/media3/exoplayer/source/ — contains
    ProgressiveMediaPeriodTest (if present) and MediaPeriodTest.

Suggested new test

Unit test in TsExtractorTest (or a new TsExtractorEmptyTrackTest):

Construct a synthetic TS bitstream where the PMT declares two elementary streams (one H264 video
and one AAC-LATM audio by PID) but only the video PID receives TS packets. Assert that:

  1. The extractor does not block indefinitely.
  2. Exactly one video track is exposed.
  3. The audio queue either has a format (from a future TsExtractor-level fix) or the preparation
    completes via the ProgressiveMediaPeriod fallback.

Integration test / test asset: notplayable.ts could be trimmed to a few seconds (the first
~30 MB of the file should be sufficient to reproduce the issue without including too much of the
actual recording). A trimmed version could live at:
libraries/test_data/src/test/assets/media/ts/sample_h264_aac_empty_audio_track.ts


Risk Assessment

What this changes

  • Constructor: handler is initialized one line earlier (before the onContinueLoadingRequestedRunnable
    lambda). Behavior is identical; this only satisfies Java's definite-assignment rule.

  • ProgressiveMediaPeriod.onContinueLoadingRequestedRunnable: Adds one handler.post() call
    before the existing onContinueLoadingRequested() callback. This fires every time the load
    controller queries whether loading should resume. In the normal case (all queues have formats),
    maybeFinishPrepare() has already set prepared = true and returns immediately at the first
    guard — the extra invocation is a no-op. In the empty-track case, it is what triggers the
    placeholder assignment before the 4-second timeout fires.

  • ProgressiveMediaPeriod.onLoadCompleted(): Adds one handler.post() call after setting
    loadingFinished = true. Covers the small-file / EOS path where the buffer never fills.
    If all queues already have formats, the extra maybeFinishPrepare() invocation is a no-op.

  • ProgressiveMediaPeriod.maybeFinishPrepare(): Replaces the unconditional return with a
    three-branch guard. The guard is a strict tightening: the old code returned early for any
    null-format queue unconditionally; the new code does so only when loading is actively in
    progress (which is always the case during normal preparation, so the happy path is unchanged).

What could go wrong

  1. A format arrives late in a valid file. Not possible after loading finishes; see the race
    analysis above.

  2. A live stream with an empty PMT-declared track. For a live MPEG-TS stream over HTTP the
    pacing-loop path (Change 2) fires after the first 1 MB of data. By that point the PMT has
    been parsed and all real tracks have reported their formats, so haveVideo && haveAudio is
    true. The placeholder is assigned, preparation completes, and the stream plays normally. The
    placeholder track produces no samples and is never selected for rendering. If loadingFinished
    or loadCondition state is reset on a seek or retry (seekToUs(), onLoadCanceled()), the
    guard resets with it — the placeholder would be reassigned on the next 1 MB checkpoint, which
    is harmless.

  3. The placeholder track confuses downstream components. The Format.Builder().build()
    placeholder has null sampleMimeType. All downstream checks (MimeTypes.isAudio(),
    MimeTypes.isVideo(), MimeTypes.getTrackType()) handle null gracefully, returning false
    or C.TRACK_TYPE_UNKNOWN. The DRM session manager's getCryptoType() returns
    C.CRYPTO_TYPE_NONE for unknown formats. No checkNotNull() on sampleMimeType exists in
    the affected code paths.

  4. Two files touched vs. one. Only ProgressiveMediaPeriod.java is modified. TsExtractor,
    DefaultTsPayloadReaderFactory, and all elementary stream readers are unchanged.

Who is affected

Only files/streams where a registered sample queue never receives a format() call AND the
loader completes normally. In practice: MPEG-TS recordings with PMT-declared-but-empty
elementary streams. No other known container format triggers this path in the current media3
codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant