From 2363e15eeef4064f95eeec1b487c438b62dc7a02 Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Sun, 26 May 2019 01:56:48 +0200
Subject: player: simplify/fix --start/--end handling with
 --rebase-start-time=no

The get_play_start_pts() function was supposed to return "rebased"
(relative to 0) timestamps. This was roundabout, because one of 2
callers just added the offset back, and the other caller actually
expected an absolute timestamp.

Change rel_time_to_abs() (whose return value get_play_start_pts()
returns without further changes) to return absolute times.

This should fix that absolute and relative times passed to --start and
--end were treated the same, which can't be right. It probably also
fixes --end if --rebase-start-time=no is used (which can't have been
correct either).

All in all I'm not sure why --rebase-start-time=no or absolute vs.
relative times in --start/--end even exist, when they were incorrectly
implemented for years.

Untested, because no sample file and I don't care. However, if anyone
cares, and I got it wrong, I hope it's simple to fix.
---
 player/loadfile.c |  7 -------
 player/misc.c     | 30 ++++++++++--------------------
 2 files changed, 10 insertions(+), 27 deletions(-)

(limited to 'player')

diff --git a/player/loadfile.c b/player/loadfile.c
index c16b8e8152..078eac53a6 100644
--- a/player/loadfile.c
+++ b/player/loadfile.c
@@ -1557,13 +1557,6 @@ static void play_current_file(struct MPContext *mpctx)
         play_start_pts = MPMAX(mpctx->demuxer->duration, 0);
 
     if (play_start_pts != MP_NOPTS_VALUE) {
-        /*
-         * get_play_start_pts returns rebased values, but
-         * we want an un rebased value to feed to seeker.
-         */
-        if (!opts->rebase_start_time){
-            play_start_pts += mpctx->demuxer->start_time;
-        }
         queue_seek(mpctx, MPSEEK_ABSOLUTE, play_start_pts, MPSEEK_DEFAULT, 0);
         execute_queued_seek(mpctx);
     }
diff --git a/player/misc.c b/player/misc.c
index 27179154a3..a01eeef479 100644
--- a/player/misc.c
+++ b/player/misc.c
@@ -48,40 +48,30 @@
 static double rel_time_to_abs(struct MPContext *mpctx, struct m_rel_time t)
 {
     double length = get_time_length(mpctx);
-    // declaration up here because of C grammar quirk
-    double chapter_start_pts;
+    // Relative times are an offset to the start of the file.
+    double start = 0;
+    if (mpctx->demuxer && !mpctx->opts->rebase_start_time)
+        start = mpctx->demuxer->start_time;
+
     switch (t.type) {
     case REL_TIME_ABSOLUTE:
         return t.pos;
     case REL_TIME_RELATIVE:
         if (t.pos >= 0) {
-            return t.pos;
+            return start + t.pos;
         } else {
             if (length >= 0)
-                return MPMAX(length + t.pos, 0.0);
+                return start + MPMAX(length + t.pos, 0.0);
         }
         break;
     case REL_TIME_PERCENT:
         if (length >= 0)
-            return length * (t.pos / 100.0);
+            return start + length * (t.pos / 100.0);
         break;
     case REL_TIME_CHAPTER:
-        chapter_start_pts = chapter_start_time(mpctx, t.pos);
-        if (chapter_start_pts != MP_NOPTS_VALUE){
-            /*
-             * rel_time_to_abs always returns rebased timetamps,
-             * even with --rebase-start-time=no. (See the above two
-             * cases.) chapter_start_time values are not rebased without
-             * --rebase-start-time=yes, so we need to rebase them
-             * here to be consistent with the rest of rel_time_to_abs.
-             */
-            if (mpctx->demuxer && !mpctx->opts->rebase_start_time){
-                chapter_start_pts -= mpctx->demuxer->start_time;
-            }
-            return chapter_start_pts;
-        }
-        break;
+        return chapter_start_time(mpctx, t.pos); // already absolute time
     }
+
     return MP_NOPTS_VALUE;
 }
 
-- 
cgit v1.2.3