[FFmpeg-devel] [PATCH] lavf/mov: Always prefer tfdt over sidx

[Thilo Borgmann]

Am 03.11.21 um 17:41 schrieb Gyan Doshi:
> 
> 
> On 2021-11-03 03:16 pm, Thilo Borgmann wrote:
>> Hi,
>>
>> this effectively reverts 071930de724166bfb90fc6d368c748771188fd94 and fixes the underlying issue by always preferring TFDT.
>>
>> Furthermore, the current solution fails if the -use_tfdt flag is given but no TFDT box is found in the stream.
> 
> Thanks for the heads-up.
> 
> The original impetus for my patch was a client's sample file which had rubbish sidx values but sane tfdt ones, likely due to a buggy muxer.
> The motivation for an option was to give the user control.
> 
> I think hardcoding a reversed preference can lead to the same impasse. How about change the default value of use_tfdt and check for tfdt presence when enabled?

Changed the default to use TFDT and add warnings about fallback to SIDX or TFDT where useful.
Looks better?

Thanks,
Thilo
-------------- next part --------------
From 435196bbb406ed7d4311f327d96664cb140af048 Mon Sep 17 00:00:00 2001
From: Thilo Borgmann <thilo.borgmann at mail.de>
Date: Thu, 4 Nov 2021 12:00:07 +0100
Subject: [PATCH] lavf/mov: Change default to prefer TFDT time and allow for
 fallback to SIDX or TFDT

---
 doc/demuxers.texi |  2 +-
 libavformat/mov.c | 42 ++++++++++++++++++++++++++++--------------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 1c9575b2e8..cab8a7072c 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -691,7 +691,7 @@ Don't use mfra box to set timestamps
 
 @item use_tfdt
 For fragmented input, set fragment's starting timestamp to @code{baseMediaDecodeTime} from the @code{tfdt} box.
-Default is disabled, which will preferentially use the @code{earliest_presentation_time} from the @code{sidx} box.
+Default is enabled, which will prefer to use the @code{tfdt} box to set DTS. Disable to use the @code{earliest_presentation_time} from the @code{sidx} box.
 In either case, the timestamp from the @code{mfra} box will be used if it's available and @code{use_mfra_for} is
 set to pts or dts.
 
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 3fcb1dc908..8a910a3165 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4828,20 +4828,34 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             dts = frag_stream_info->first_tfra_pts;
             av_log(c->fc, AV_LOG_DEBUG, "found mfra time %"PRId64
                     ", using it for dts\n", pts);
-        } else if (frag_stream_info->sidx_pts != AV_NOPTS_VALUE && !c->use_tfdt) {
-            // FIXME: sidx earliest_presentation_time is *PTS*, s.b.
-            // pts = frag_stream_info->sidx_pts;
-            dts = frag_stream_info->sidx_pts - sc->time_offset;
-            av_log(c->fc, AV_LOG_DEBUG, "found sidx time %"PRId64
-                    ", using it for pts\n", pts);
-        } else if (frag_stream_info->tfdt_dts != AV_NOPTS_VALUE) {
-            dts = frag_stream_info->tfdt_dts - sc->time_offset;
-            av_log(c->fc, AV_LOG_DEBUG, "found tfdt time %"PRId64
-                    ", using it for dts\n", dts);
         } else {
-            dts = sc->track_end - sc->time_offset;
-            av_log(c->fc, AV_LOG_DEBUG, "found track end time %"PRId64
-                    ", using it for dts\n", dts);
+            int has_tfdt = frag_stream_info->tfdt_dts != AV_NOPTS_VALUE;
+            int has_sidx = frag_stream_info->sidx_pts != AV_NOPTS_VALUE;
+            int fallback_tfdt = !c->use_tfdt && !has_sidx && has_tfdt;
+            int fallback_sidx =  c->use_tfdt && !has_tfdt && has_sidx;
+
+            if (fallback_sidx) {
+                av_log(c->fc, AV_LOG_DEBUG, "use_tfdt set but no tfdt found, using sidx instead\n");
+            }
+            if (fallback_tfdt) {
+                av_log(c->fc, AV_LOG_DEBUG, "use_tfdt not set but no sidx found, using tfdt instead\n");
+            }
+
+            if (has_tfdt && c->use_tfdt || fallback_tfdt) {
+                dts = frag_stream_info->tfdt_dts - sc->time_offset;
+                av_log(c->fc, AV_LOG_DEBUG, "found tfdt time %"PRId64
+                        ", using it for dts\n", dts);
+            } else if (has_sidx && !c->use_tfdt || fallback_sidx) {
+                // FIXME: sidx earliest_presentation_time is *PTS*, s.b.
+                // pts = frag_stream_info->sidx_pts;
+                dts = frag_stream_info->sidx_pts - sc->time_offset;
+                av_log(c->fc, AV_LOG_DEBUG, "found sidx time %"PRId64
+                        ", using it for pts\n", pts);
+            } else {
+                dts = sc->track_end - sc->time_offset;
+                av_log(c->fc, AV_LOG_DEBUG, "found track end time %"PRId64
+                        ", using it for dts\n", dts);
+            }
         }
     } else {
         dts = sc->track_end - sc->time_offset;
@@ -8533,7 +8547,7 @@ static const AVOption mov_options[] = {
         FLAGS, "use_mfra_for" },
     {"pts", "pts", 0, AV_OPT_TYPE_CONST, {.i64 = FF_MOV_FLAG_MFRA_PTS}, 0, 0,
         FLAGS, "use_mfra_for" },
-    {"use_tfdt", "use tfdt for fragment timestamps", OFFSET(use_tfdt), AV_OPT_TYPE_BOOL, {.i64 = 0},
+    {"use_tfdt", "use tfdt for fragment timestamps", OFFSET(use_tfdt), AV_OPT_TYPE_BOOL, {.i64 = 1},
         0, 1, FLAGS},
     { "export_all", "Export unrecognized metadata entries", OFFSET(export_all),
         AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = FLAGS },
-- 
2.20.1 (Apple Git-117)