Recover from non-monotonic presentationTimeUs caused by naively-muxed B-frame content - #3352
Conversation
On Google TV Streamer 4K (MT8696, c2.mtk.hevc.decoder), specific HEVC Main10 content causes the codec to return output buffers with non-monotonic presentationTimeUs, while the buffer release order itself remains correct. MediaCodecRenderer processes output buffers in raw release order and uses the codec-reported timestamp directly for the render/drop decision, causing MediaCodecVideoRenderer to treat these buffers as arriving too late and drop them. Measured on affected content: ~30% of decoded video frames dropped, sustained throughout playback. This re-derives each output buffer's presentationTimeUs from its release order and the stream's known frame duration, instead of trusting the codec-reported value. Buffer release order is untouched. Verified on-device on the affected hardware/content: dropped frames go to 0, playback confirmed visually smooth by direct observation.
|
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. |
The per-frame duration was rounded to the nearest microsecond once, then multiplied by a growing frame index. Any fractional-microsecond remainder in the true frame duration (e.g. 41708.333...us for 24000/1001fps content) was silently dropped every frame and never recovered, so the error accumulated linearly with runtime: ~0.7ms over a 90-second clip (undetectable, and the only length tested before this fix), but ~58ms by the end of a 2-hour movie — enough to produce audible/visible A/V desync that gets worse the longer playback continues. Fix: keep the per-frame duration unrounded and compute each buffer's offset independently as round(frameIndex * unroundedDurationUs), rounding only the final result. Error no longer accumulates; it stays bounded to at most +/-0.5us for the life of the stream. Found via real-world testing in a third-party player (Plezy) on full- length movies, where the 90-second on-device test clips used to validate the original fix could never have revealed it.
|
Pushed a fix for a second bug: audio/video would slowly drift out of Tested in two real players on the affected hardware (Jellyfin Android |
|
Thanks for the detailed investigation and for providing a legal, However, I don't think the current evidence isolates this as a I downloaded After decoding, the propagated frame PTS becomes non-monotonic: while FFmpeg's This looks consistent with missing or incorrect composition offsets Android documents https://developer.android.com/reference/android/media/MediaCodec.BufferInfo#presentationTimeUs Therefore, reproducing the same output timestamp sequence with Could we rule this out before changing
There is also a separate scope concern with the current patch: despite The patch explains why this particular CFR sample becomes smooth, but |
SummaryMANY thanks for your helpful reply. We looked into all three of your suggestions. Short version: I no longer think this is a 1. Composition timestamps: correctly-muxed vs. naively-muxed contentYou were right about the repro file. Reproduced your The repro README's mux step ( ffmpeg -f lavfi -i "testsrc2=size=640x360:rate=24:duration=15" -f yuv4mpegpipe src.y4m
# encode straight to MP4 — libavformat assigns real composition offsets in-process
ffmpeg -i src.y4m -c:v libx265 -tag:v hvc1 good.mp4
# encode to a detached raw stream, then remux exactly like the repro README does
ffmpeg -i src.y4m -c:v libx265 -f hevc raw.hevc
ffmpeg -fflags +genpts -r 24 -i raw.hevc -c:v copy -tag:v hvc1 bad.mp4Same encoder, same options, same source — only the mux step differs. Confirmed on-device too, on a build with none of our patches applied: correctly-muxed B-frame 2. x265 3.3 vs. current x265 4.1Re-encoded the same Tears of Steel source with current x265 (4.1+1-1d117be), options matched as This briefly looked like it contradicted our own earlier finding that x265 4.1 content didn't So: not a contradiction, and not an x265-version-specific bug. Encoder version is irrelevant to 3. Logging queued input vs. dequeued output timestampsPatch (diagnostic-only, no behavior change):
Queued-input order is perfectly monotonic in both — a direct echo of the container's decode-order The key check: every single dequeued output timestamp, in both files, is one of the exact Where this leaves thingsI don't think this is evidence of a MediaTek decoder defect. However, what it does look like is VLC plays these incorrectly-timestamped files just fine, so I want to dig into why. My guess is |
2bitoperations
changed the title
Why VLC doesn't stutter on this contentInstrumented VLC itself (not just read its source) and ran it on-device against the same repro file. It receives equally or more disordered decoder output than ExoPlayer from VLC just never lets that value reach anything that schedules a frame. Its MP4 demuxer refuses to invent a PTS for a video track when the file has no // mp4.c, https://code.videolan.org/videolan/vlc/-/blob/ac6c2a405d652b5576128ceb9fec2c342f0e83ec/modules/demux/mp4/mp4.c#L1391-L1396
if( MP4_TrackGetPTSDelta( p_demux, tk, &i_delta ) )
p_block->i_pts = p_block->i_dts + i_delta;
else if( tk->fmt.i_cat != VIDEO_ES )
p_block->i_pts = p_block->i_dts;
else
p_block->i_pts = VLC_TICK_INVALID; // video track, no ctts: don't guessThat
// BoxParser.java, https://github.com/androidx/media/blob/5fb306449733dd71595700c1227ad6087578c559/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/BoxParser.java#L516
int timestampOffset = 0;
...
// https://github.com/androidx/media/blob/5fb306449733dd71595700c1227ad6087578c559/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/BoxParser.java#L627
timestamps[i] = timestampTimeUnits + timestampOffset; // ctts == null → always 0 → PTS = DTSThat definite-but-wrong PTS is exactly what So: not a MediaTek defect, not an ExoPlayer bug in the usual sense — one demuxer treats a missing Next: I'm going to look at overhauling this to give media3 a first-class notion of "missing PTS," and only fall back to today's spray-and-pray frame-dropping behavior in that specific case. |
The previous fix (325f2c5, 295aba9) re-derives every video track's output presentationTimeUs from arrival order unconditionally, for any track where the frame rate is known. That's safe for the content that motivated it, but it's a blunt instrument: it discards real per-sample timing information even on correctly-muxed content (harmless there only because arrival order already matches presentation order for a well-behaved decoder), and it would silently corrupt genuine variable-frame-rate content, which was never part of what triggered this in the first place. Root cause (see issue androidx#3347) is not a decoder defect: it's that the source file's MP4 track has no ctts box, so every sample's presentationTimeUs is just its decode time. B-frame-reordered content muxed this way was never given real composition timing, and there's no way to reconstruct it after the fact — MediaCodec just echoes back whatever (undifferentiated) timestamp it was queued with. This adds Format#hasReliablePresentationTimestamps (default true) and sets it to false in Mp4Extractor/BoxParser exactly when a video track has no ctts box — the same signal a completely independent player (VLC, via its MP4 demuxer's MP4_TrackGetPTSDelta) already uses to decide it can't trust a sample's PTS for a video track. Confirmed by instrumenting and running VLC itself on-device against the same repro files: it receives equally/more disordered raw decoder output than ExoPlayer (52.2% vs ~42% non-monotonic steps) but never drops a frame for lateness, because by the time anything checks, its own PTS reconstruction has already replaced the disordered decoder echo with a clean synthetic timeline. Two changes gated on the new flag: - MediaCodecRenderer's arrival-order relabeling now only fires when the track told us up front it has no reliable per-sample timing, instead of unconditionally for every video track. - MediaCodecVideoRenderer's shouldDropOutputBuffer/ shouldDropBuffersToKeyframe no longer drop for lateness on such tracks — the relabeled timestamp is a best-effort reconstruction, not ground truth, so a "how late is this" reading isn't meaningful enough to discard a frame over. Render everything in arrival order instead, matching what VLC's own (independently-implemented, not copied) approach does in practice. Behavior for every other format (anything with a ctts box, or not MP4 at all) is unchanged: the new flag defaults to true and both changed code paths are no-ops unless a track explicitly says otherwise.
The relabeling added in the previous commit re-derived every gated buffer's timestamp from a single assumed constant frame duration (1_000_000/Format#frameRate). Tested that assumption directly against a constructed file with no B-frames (so no ctts is legitimately correct, not a symptom of anything) but genuinely irregular per-sample durations (two spliced segments, 24fps and 8fps): the extractor reports a single blended Format#frameRate (16fps, the average), and the old logic used that one value for every frame in the file, flattening both segments to a uniform ~62.5ms spacing instead of the real ~41.7ms / 125ms — confirmed on-device via added diagnostic logging before this fix, removed after. Replaced the single-frame-duration synthesis with a FIFO of each video sample's own presentationTimeUs, recorded in queue order as input buffers are fed to the codec, and popped in output-arrival order to relabel each dequeued buffer. Queue order is always monotonic by construction (a direct echo of the container's own decode-time-to-sample table) and preserves each sample's true declared duration exactly, whether constant or not — this is what a completely independent player (VLC, via its MP4 demuxer's timestamp FIFO) already does for the same situation, arrived at independently during instrumentation of VLC on real device (see issue androidx#3347 discussion). Re-verified the full on-device test matrix after this change: broken_95s.mp4, good_95s.mp4, waterboy_90s_clip.mp4 (all 0 dropped frames, reach ENDED normally), tos_real_footage_matched.mp4 (real ctts present, hasReliablePresentationTimestamps stays true, behavior unchanged), and the new VFR file (real ~41.7ms/125ms spacing now preserved exactly, confirmed via per-buffer diagnostic logging).
The comments added across the last two commits ran 3-5x longer than comparable existing ones in this file (e.g. hasPrerollSamples's field javadoc, or skippedFlushOffsetUs, which gets two sentences on its getter and no comment at all at the field declaration) and repeated the same reasoning at every call site instead of stating it once. Also fixes a stale reference: shouldDropBuffersToKeyframe's comment still named arrivalOrderPtsBaseUs, a field the VFR fix in the previous commit removed.
|
ok - i think this approach is way more contained and correct. @FongMi please let me know what you think! |
|
This is much better scoped than the previous version, but I found several correctness issues that should be addressed before this is ready:
Please also add focused tests covering:
The investigation and the move away from a synthetic CFR timeline are solid improvements, but the current implementation still has correctness and lifecycle gaps. |
Per review on androidx#3347: missing ctts alone was too broad. A correctly muxed video with no B-frame reordering legitimately has no ctts, and under the old check it would still get flagged hasReliablePresentationTimestamps=false — needlessly activating the FIFO relabeling *and*, more importantly, permanently disabling shouldDropOutputBuffer/shouldDropBuffersToKeyframe for content that never had a timestamp problem. On a genuinely overloaded device, that would let video drift behind audio with no way to recover. Now also requires track.format.maxNumReorderSamples != 0 -- this comes from the bitstream's own SPS (sps_max_num_reorder_pics for HEVC, via HevcConfig.java), not the container, and is already parsed before parseStbl runs. 0 means the encoder positively declared it doesn't reorder; NO_VALUE (-1, config box missing/unparsed) is treated the same as "might reorder" rather than "doesn't" -- conservative by design, per discussion. Verified on-device, all via the same diagnostic logging used throughout this investigation (added temporarily, confirmed, removed): - broken_95s.mp4 (maxNumReorderSamples=2, real reordering): still triggers the workaround, still 0 dropped frames. - good_95s.mp4 (maxNumReorderSamples=0, encoder confirms no reordering): now correctly does NOT trigger -- normal shouldDropOutputBuffer behavior is back in effect for this file, still 0 dropped frames since it never needed the workaround. - tos_real_footage_matched.mp4 (real ctts present): unaffected, as before. - New: mp4v_no_ctts_no_maxreorder.mp4 -- MPEG-4 Part 2 (mp4v) with 159 real B-frames and no ctts, built by encoding normally (ffmpeg mpeg4 encoder, -bf 2, which produces a correctly-muxed file with real ctts) and then surgically stripping just the ctts box (strip_ctts.py, included alongside) rather than going through the raw-ES-remux pipeline used elsewhere in this investigation, since that pipeline doesn't handle mpeg4 cleanly. media3 has no maxNumReorderSamples-setting code path for mp4v at all (only avcC/hvcC/vvcC set it), so this is a real, not contrived, maxNumReorderSamples=NO_VALUE case with genuine reordering. Confirmed hasReliablePresentationTimestamps=false (the conservative choice correctly engaging), played on-device via c2.mtk.mpeg4.decoder, 0 dropped frames, ENDED cleanly.
Targets a specific edge case in the fix proposed in androidx/media#3352: missing ctts + a codec (MPEG-4 Part 2) media3 never derives maxNumReorderSamples for, so the conservative NO_VALUE handling is what has to catch it rather than positive evidence of reordering.
|
Addressed point 2 in |
Per review point 1 on androidx#3347: the flag was only being applied near the final return of parseStbl(), so the sampleCount==0 early return (and by extension the omitTrackSampleTable case, which still runs the rest of the method but was untested against this specific path) never carried it. Many video tracks without ctts would silently never activate the workaround. Moved the computation to the very start of the method, before any return path -- everything it depends on (track.type, whether a ctts box exists, track.format.maxNumReorderSamples) is already available from the parameters, none of it requires the per-sample parsing that happens later. Applies to track's format immediately, so every one of the 6 return sites in this method now carries the correct value. hasPrerollSamples is unaffected -- it genuinely isn't known until the edit-list processing near the bottom of the method runs, so it stays where it was; only removed the accidental "OR with existing value" logic I'd added when the two were sharing one if-block, restoring the original single-flag behavior there. Re-verified on-device: broken_95s.mp4 (still triggers, 0 drops), good_95s.mp4 (still correctly skips, 0 drops), mp4v_no_ctts_no_maxreorder.mp4 (still triggers via the conservative NO_VALUE path, 0 drops).
Per review points 4 and 5 on androidx#3347. Point 4: the push (feedInputBuffer) and pop (drainOutputBuffer) sides of arrivalOrderPtsQueue were gated on inputFormat, while shouldDropOutputBuffer/shouldDropBuffersToKeyframe (MediaCodecVideoRenderer) are gated on codecInputFormat. These are usually the same value -- onInputFormatChanged sets both together for the two seamless-reuse cases (REUSE_RESULT_YES_WITH_FLUSH/RECONFIGURATION) -- but diverge during a full codec reinit (REUSE_RESULT_NO): inputFormat advances immediately, codecInputFormat doesn't catch up until the new codec is actually configured, and in between the old codec is still draining old-format output under the old format's policy. Switched both queue sites to codecInputFormat, which is a field on this same class (no new plumbing) and is the more correct authority anyway: it specifically tracks what the active codec instance is configured with, which is what governs the buffers actually flowing through queue/dequeue. Point 5: MediaCodecVideoRenderer's tunneling mode has its own output path (see updateOutputFormatForTime's javadoc: buffers aren't dequeued from the decoder at all in that mode), so drainOutputBuffer -- and therefore the pop -- never runs. The push wasn't tunneling-aware, so arrivalOrderPtsQueue would grow unboundedly for the duration of any tunneled session with unreliable timestamps. Guarded the push with !getConfiguration().tunneling (BaseRenderer, already accessible, no video-specific field needed). Existing resetCodecStateForFlush() clearing already handles cleanup if tunneling toggles mid-session. No matching guard needed on the pop side -- it's provably unreachable in tunneling mode already, so anything there would be dead code. Re-verified on-device (default demo app config, not tunneled): broken_95s.mp4, good_95s.mp4, mp4v_no_ctts_no_maxreorder.mp4 -- all unchanged, 0 dropped frames each. The specific codecInputFormat-lag scenario (full reinit mid-stream) isn't exercised by any of these; a dedicated seamless-format-change test is still on the list from the original review (along with tunneling itself, which I could not directly exercise against the demo app's default config).
… mismatch, tunneling leak) Adds a package-private @VisibleForTesting arrivalOrderPtsQueue size accessor to MediaCodecRenderer -- there's no black-box way to observe the tunneling leak fix otherwise, since nothing ever reads the queue back out in that mode. Point 4 (MediaCodecVideoRendererTest): reuses the existing render_withIncompatibleFrameRateChangeUpToSdk29_discardsCodec 24fps->30fps REUSE_RESULT_NO trigger, but the first format now has hasReliablePresentationTimestamps=false and out-of-order sample timestamps, and the codec adapter is ForwardingSynchronousMediaCodecAdapterWithReordering (sorts dequeued output ascending, simulating an untrustworthy decoder echo). Captures onProcessedOutputBuffer's sequence and asserts the first format's two samples come back in feed order, not sorted -- proving codecInputFormat (which still lags at "old format" during the drain) governs the FIFO replay rather than the already-advanced inputFormat. Point 5 (MediaCodecRendererTest): adds a minimal video-track-type TestRenderer variant (the existing one hardcodes TRACK_TYPE_AUDIO, but the tunneling guard only applies to video), enables it with RendererConfiguration(tunneling=true) and an unreliable-PTS format, feeds several samples, and asserts the queue size is still 0 -- the queue would otherwise grow for the life of the session since drainOutputBuffer never runs in tunneling mode. Full MediaCodecVideoRendererTest (176 cases) and MediaCodecRendererTest (21 cases) suites re-run clean alongside the new tests.
… it, add VFR regression test Point 3: shouldDropOutputBuffer() and shouldDropBuffersToKeyframe() returned false unconditionally when hasReliablePresentationTimestamps is false, disabling all recovery from genuine decoder slowness -- but earlyUs under that flag is still a real pacing signal (it's derived from the same arrival-order-corrected timestamp arrivalOrderPtsQueue substitutes), just less precise than ground truth. shouldDropOutputBuffer now requires VERY_LATE-level lateness (500ms, vs the normal 30ms) instead of disabling outright -- enough slack for decoder reorder-buffer depth to not look like backlog, but a real backstop against runaway lateness. shouldDropBuffersToKeyframe stays disabled: it decides how many buffers to discard, which needs real per-sample identity this case doesn't have. Two new tests cover the widened-not-removed threshold: a buffer 100ms late (past the old 30ms threshold, short of the new 500ms one) isn't dropped; one 600ms late still is. Point 6: added a regression test feeding irregularly-spaced (VFR) sample timestamps with hasReliablePresentationTimestamps=false, asserting arrivalOrderPtsQueue replays them exactly rather than flattening to an assumed constant frame rate (the bug the arrival-order queue replaced). FongMi's point stands that true per-frame presentation instants can't be reconstructed without ctts; this validates the best achievable fallback (faithful arrival-order replay) instead. Full MediaCodecVideoRendererTest (179 cases) and MediaCodecRendererTest (21 cases) suites re-run clean.
2 participants
Fixes #3347
Problem
On Google TV Streamer 4K (MediaTek MT8696,
c2.mtk.hevc.decoder),decoding specific HEVC Main10 content causes the codec to return
output buffers with non-monotonic
presentationTimeUs, while theorder in which those buffers are released remains correct — confirmed
by comparing each buffer's release-order rank against its
timestamp-sorted rank: the two never diverge by more than a few
positions, and the timestamp values themselves are individually
correct for the frame they belong to.
Reproduced independently of ExoPlayer, using a minimal test harness
built directly on
AMediaCodec/AMediaExtractor(no ExoPlayer, noJava
MediaCodec) against the same file: identical non-monotonictimestamp pattern. This confirms the defect is in the decoder/HAL
output, not in ExoPlayer's handling of it.
MediaCodecRendererprocesses output buffers in raw release orderbut uses the codec-reported
presentationTimeUsdirectly for therender/drop decision in
MediaCodecVideoRenderer. When a buffer'stimestamp is behind the previous one, it's treated as arriving too
late and dropped. Measured on affected content: ~30% of decoded video
frames dropped, sustained throughout playback, reproduced in the
Media3 demo app as well as Plex, Jellyfin, and other ExoPlayer-based
players on the same device/file.
Fix
Re-derive each output buffer's
presentationTimeUsfrom its releaseorder and the stream's known frame duration (from
Format.frameRate),instead of trusting the codec-reported value. Buffer release order is
left untouched — buffers are not held or reordered.
Testing
Verified on the affected device (Google TV Streamer 4K) against three
files that reliably reproduce the issue on unpatched
main,including a 90-second real-world clip: dropped-frame count goes from
~30% to 0, and the fix was visually confirmed on-device — direct
observation of smooth playback, not just log metrics. HOWEVER, with this approach, video and audio lose sync.
An earlier approach that buffered and released output buffers in
sorted-timestamp order was also tried and rejected: it also eliminated
logged drops, but produced visible playback artifacts on-device. That
result is why this PR does not reorder buffers.