From ef90f501bf51b4627880ae3acc167a1625fd10b4 Mon Sep 17 00:00:00 2001
From: ibaker <ibaker@google.com>
Date: Thu, 14 Nov 2024 11:01:33 -0800
Subject: [PATCH] Don't assume MP4 keyframe metadata is correct for CEA
 re-ordering

The content in Issue: androidx/media#1863 has every sample incorrectly marked as a
sync sample in the MP4 metadata, which results in flushing the
re-ordering queue on every sample, so nothing gets re-ordered, so the
subtitles are garbled.

There are currently two "uses" for this call on every keyframe:
1. It offers a safety valve if we don't read a `maxNumReorderSamples`
value from the video. Without this, the queue will just keep growing
and end up swallowing all subtitle data (similar to the bug fixed by
https://github.com/androidx/media/commit/39c734963f67f8ed3fa79820d573e6b3297c8d28).

2. When we do read (or infer) a `maxNumReorderSamples` it means we can
emit samples from the queue slightly earlier - but this is pretty
marginal, given i think the max possible value for
`maxNumReorderSamples` is 16, so the most benefit we would get is 16
frames (~0.53s at 30fps) - in most cases we will have more than 0.5s
of buffer ahead of the playback position, so the subtitles will still
get shown at the right time with no problem.

(1) is resolved in this change by setting the queue size to zero (no
reordering) if we don't have a value for `maxNumReorderSamples`.

(2) has minimal impact, so we just accept it.

We may be able to inspect the NAL unit to determine IDR vs non-IDR
instead - we will consider this as a follow-up change, but given the
minimal impact of (2) we may not pursue this.

PiperOrigin-RevId: 696583702
(cherry picked from commit e6448f34983f58188e97e08976f219efb10e0178)
---
 RELEASENOTES.md                                    |  3 +++
 .../extractor/mp4/FragmentedMp4Extractor.java      | 14 +++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/RELEASENOTES.md b/RELEASENOTES.md
index 2483a1954e..a083222f52 100644
--- a/RELEASENOTES.md
+++ b/RELEASENOTES.md
@@ -10,6 +10,9 @@
         per sample.
     *   Fix playback hanging on DASH multi-period streams when CEA-608 subtitles
         are enabled ([#1863](https://github.com/androidx/media/issues/1863)).
+    *   Fix garbled CEA-608 subtitles in MP4 files that incorrectly mark every
+        sample as a sync sample
+        ([#1863](https://github.com/androidx/media/issues/1863)).
 *   Demo app
     *   Resolve the memory leaks in demo short-form app
         ([#1839](https://github.com/androidx/media/issues/1839)).
diff --git a/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/FragmentedMp4Extractor.java b/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/FragmentedMp4Extractor.java
index db44bc77c6..05e299e104 100644
--- a/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/FragmentedMp4Extractor.java
+++ b/libraries/extractor/src/main/java/androidx/media3/extractor/mp4/FragmentedMp4Extractor.java
@@ -1637,17 +1637,17 @@ public class FragmentedMp4Extractor implements Extractor {
                     : 0);
             nalBuffer.setLimit(unescapedLength);
 
-            if (track.format.maxNumReorderSamples != Format.NO_VALUE
-                && track.format.maxNumReorderSamples != reorderingSeiMessageQueue.getMaxSize()) {
+            if (track.format.maxNumReorderSamples == Format.NO_VALUE) {
+              if (reorderingSeiMessageQueue.getMaxSize() != 0) {
+                reorderingSeiMessageQueue.setMaxSize(0);
+              }
+            } else if (reorderingSeiMessageQueue.getMaxSize()
+                != track.format.maxNumReorderSamples) {
               reorderingSeiMessageQueue.setMaxSize(track.format.maxNumReorderSamples);
             }
             reorderingSeiMessageQueue.add(sampleTimeUs, nalBuffer);
 
-            boolean sampleIsKeyFrameOrEndOfStream =
-                (trackBundle.getCurrentSampleFlags()
-                        & (C.BUFFER_FLAG_KEY_FRAME | C.BUFFER_FLAG_END_OF_STREAM))
-                    != 0;
-            if (sampleIsKeyFrameOrEndOfStream) {
+            if ((trackBundle.getCurrentSampleFlags() & C.BUFFER_FLAG_END_OF_STREAM) != 0) {
               reorderingSeiMessageQueue.flush();
             }
           } else {