diff mbox series

[FFmpeg-devel,08/15] avformat/mov:Do not set `next_root_atom` for unfragmented files

Message ID f63ee8db7a71e35592b3cf760ee4aae4cd236f9a.camel@haerdin.se
State New
Headers show
Series Spotify patchset | expand

Checks

Context Check Description
andriy/configure_x86 warning Failed to apply patch
yinshiyou/configure_loongarch64 warning Failed to apply patch

Commit Message

Tomas Härdin Oct. 29, 2024, 2:49 p.m. UTC 
Needs sample. Not a huge fan of heuristics like this - surely there's a
way to tell within the first mdat/moov/moof seen?

Spotify comments
----------------
Failure to parse non-fragmented file with many mdat-boxes

/Tomas
diff mbox series

Patch

From 1c67cfd91cc3d18a2e4e1402bb439fe4db0f76b2 Mon Sep 17 00:00:00 2001
From: Ulrik <ulrikm@spotify.com>
Date: Mon, 13 Dec 2021 22:30:42 +0100
Subject: [PATCH 08/15] avformat/mov:Do not set `next_root_atom` for
 unfragmented files

"Root boxes" in the mp4 demuxer denotes groups of boxes in a fragmented
file that works self-contained. Example of such "root boxes"
`ftyp,moov,sidx', `ftyp,moov,mdat`, `moof,mdat`...

Therefore, if `next_root_atom` is 0, that can be seen as a sign that the
file is fragmented, and we should avoid setting it when assuming the file
is not fragmented.
---
 libavformat/mov.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index f58f8f3102..73d337c882 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -90,6 +90,9 @@  static void mov_free_stream_context(AVFormatContext *s, AVStream *st);
 static int64_t add_ctts_entry(MOVCtts** ctts_data, unsigned int* ctts_count, unsigned int* allocated_size,
                               int count, int duration);
 
+// Number of mdats we see without moof, before we assume non-fragmented file with lots of mdats
+#define FRAG_HEURISTIC_THRESHOLD_MDAT_WITHOUT_MOOx 5
+
 static int mov_metadata_track_or_disc_number(MOVContext *c, AVIOContext *pb,
                                              unsigned len, const char *key)
 {
@@ -1363,7 +1366,7 @@  static int mov_read_mdat(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (atom.size == 0) /* wrong one (MP4) */
         return 0;
     c->found_mdat+=1;
-    if (c->found_mdat == 5 && !c->found_moov) {
+    if (c->found_mdat == FRAG_HEURISTIC_THRESHOLD_MDAT_WITHOUT_MOOx && !c->found_moov) {
         /* Some mp4:s consist of 100s or even 1000s of mdats, with a moov at the end. To avoid a
         ton of seeking, we heuristically look for a moov at the end of the file instead */
         mov_heuristic_scan_moov(c, pb);
@@ -1647,7 +1650,7 @@  static int mov_try_read_moov(MOVContext *c, AVIOContext *pb, int64_t offset, int
             break;
         }
         if (child.size - ret + avio_tell(pb) > filesize) {
-            av_log(c->fc, AV_LOG_TRACE, "Remaining boxes does not align with EOF", offset);
+            av_log(c->fc, AV_LOG_TRACE, "Remaining boxes does not align with EOF");
             return -1;
         }
         avio_seek(pb, child.size - ret, SEEK_CUR);
@@ -9496,20 +9499,20 @@  static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         } else {
             int64_t start_pos = avio_tell(pb);
             int64_t left;
-            uint8_t index_satisfied, at_end;
+            uint8_t might_be_fragmented, index_satisfied, at_end;
             int err = parse(c, pb, a);
             if (err < 0) {
                 c->atom_depth --;
                 return err;
             }
+            /* If we've seen 5 mdats, with no fragments seen, we assume file is not fragmented */
+            might_be_fragmented = (c->found_mdat < FRAG_HEURISTIC_THRESHOLD_MDAT_WITHOUT_MOOx) || (c->frag_index.nb_items > 0);
             index_satisfied = (!(pb->seekable & AVIO_SEEKABLE_NORMAL))
                 || (c->fc->flags & AVFMT_FLAG_IGNIDX)
-                || c->frag_index.complete
-                || (c->found_mdat > 2 && (c->frag_index.nb_items == 0)); /* If we've read past 2
-                mdats, with no fragments in fragment-index, we assume file is not fragmented */
+                || c->frag_index.complete;
             at_end = (start_pos + a.size) == avio_size(pb);
-            if (c->found_moov && c->found_mdat && (a.size <= INT64_MAX - start_pos) && (index_satisfied || at_end)) {
-                if (!at_end)
+            if (c->found_moov && c->found_mdat && (a.size <= INT64_MAX - start_pos) && (index_satisfied || at_end || !might_be_fragmented)) {
+                if (!at_end && might_be_fragmented)
                     c->next_root_atom = start_pos + a.size;
                 c->atom_depth --;
                 return 0;
-- 
2.39.2