player: modify/simplify AB-loop behavior

This changes the behavior of the --ab-loop-a/b options. In addition, it
makes it work with backward playback mode.

The most obvious change is that the both the A and B point need to be
set now before any looping happens. Unlike before, unset points don't
implicitly use the start or end of the file. I think the old behavior
was a feature that was explicitly added/wanted. Well, it's gone now.

This is because of 2 reasons:

1. I never liked this feature, and it always got in my way (as user).
2. It's inherently annoying with backward playback mode.

In backward playback mode, the user wants to set A/B in the wrong order.
The ab-loop command will first set A, then B, so if you use this command
during backward playback, A will be set to a higher timestamps than B.
If you switch back to forward playback mode, the loop would stop
working. I want the loop to just continue to work, and the chosen
solution conflicts with the removed feature.

The order issue above _could_ be fixed by also switching the AB-loop
user option values around on direction switch. But there are no other
instances of option changes magically affecting other options, and doing
this would probably lead to unexpected misery (dying from corner cases
and such).

Another solution is sorting the A/B points by timestamps after copying
them from the user options. Then A/B options set in backward mode will
work in forward mode. This is the chosen solution. If you sort the
points, you don't know anymore whether the unset point is supposed to
signify the end or the start of the file.

The AB-loop code is slightly better abstracted now, so it should be easy
to restore the removed feature. It would still require coming up with a
solution for backwards playback, though.

A minor change is that if one point is set and the other is unset, I'm
rendering both the chapter markers and the marker for the set point.
Why? I don't know. My test file had chapters, and I guess I decided this
looked better.

This commit also fixes some subtle and obvious issues that I already
forgot about when I wrote this commit message. It cleans up some minor
code duplication and nonsense too.

Regarding backward playback, the code uses an unsanitary mix of internal
("transformed") and user timestamps. So the play_dir variable appears
more than usual.

To mention one unfixed issue: if you set an AB-loop that is completely
past the end of the file, it will get stuck in an infinite seeking loop
once playback reaches the end of the file. Fixing this reliably seemed
annoying, so the fix is "just don't do this". It's not a hard freeze
anyway.

1 files changed, 45 insertions, 47 deletions

diff --git a/player/misc.c b/player/misc.c
index a01eeef479..a39c8df82b 100644
--- a/player/misc.c
+++ b/player/misc.c
@@ -45,7 +45,7 @@
 #include "core.h"
 #include "command.h"
 
-static double rel_time_to_abs(struct MPContext *mpctx, struct m_rel_time t)
+double rel_time_to_abs(struct MPContext *mpctx, struct m_rel_time t)
 {
     double length = get_time_length(mpctx);
     // Relative times are an offset to the start of the file.
@@ -75,69 +75,67 @@ static double rel_time_to_abs(struct MPContext *mpctx, struct m_rel_time t)
     return MP_NOPTS_VALUE;
 }
 
-double get_play_end_pts(struct MPContext *mpctx)
+static double get_play_end_pts_setting(struct MPContext *mpctx)
 {
     struct MPOpts *opts = mpctx->opts;
     double end = rel_time_to_abs(mpctx, opts->play_end);
-    if (opts->play_length.type) {
+    double length = rel_time_to_abs(mpctx, opts->play_length);
+    if (length != MP_NOPTS_VALUE) {
         double start = get_play_start_pts(mpctx);
-        if (start == MP_NOPTS_VALUE)
-            start = 0;
-        double length = rel_time_to_abs(mpctx, opts->play_length);
-        if (length != MP_NOPTS_VALUE && (end == MP_NOPTS_VALUE || start + length < end))
+        if (end == MP_NOPTS_VALUE || start + length < end)
             end = start + length;
     }
-    // even though MP_NOPTS_VALUE is currently negative
-    // it doesn't necessarily have to remain that way
-    double ab_loop_start_time = get_ab_loop_start_time(mpctx);
-    if (mpctx->ab_loop_clip && opts->ab_loop[1] != MP_NOPTS_VALUE &&
-        (ab_loop_start_time == MP_NOPTS_VALUE || opts->ab_loop[1] > ab_loop_start_time))
-    {
-        if (end == MP_NOPTS_VALUE || end > opts->ab_loop[1])
-            end = opts->ab_loop[1];
+    return end;
+}
+
+// Return absolute timestamp against which currently playing media should be
+// clipped. Returns MP_NOPTS_VALUE if no clipping should happen.
+double get_play_end_pts(struct MPContext *mpctx)
+{
+    double end = get_play_end_pts_setting(mpctx);
+    double ab[2];
+    if (mpctx->ab_loop_clip && get_ab_loop_times(mpctx, ab)) {
+        if (end == MP_NOPTS_VALUE || end > ab[1])
+            end = ab[1];
     }
     return end;
 }
 
-/**
- * Get the rebased PTS for which playback should start.
- * The order of priority is as follows:
- *   1. --start, if set.
- *   2. The start chapter, if set.
- *   3. MP_NOPTS_VALUE.
- * If unspecified, return MP_NOPTS_VALUE.
- * Does not return zero unless the start time is explicitly set to zero.
- */
+// Get the absolute PTS at which playback should start.
+// Never returns MP_NOPTS_VALUE.
 double get_play_start_pts(struct MPContext *mpctx)
 {
     struct MPOpts *opts = mpctx->opts;
-    return rel_time_to_abs(mpctx, opts->play_start);
+    double res = rel_time_to_abs(mpctx, opts->play_start);
+    if (res == MP_NOPTS_VALUE) {
+        res = 0;
+        if (!opts->rebase_start_time && mpctx->demuxer)
+            res = mpctx->demuxer->start_time;
+        // Backward playback -> start from end by default.
+        if (mpctx->play_dir < 0 && mpctx->demuxer)
+            res = MPMAX(mpctx->demuxer->duration, 0);
+    }
+    return res;
 }
 
-/**
- * Get the time that an ab-loop seek should seek to.
- * The order of priority is as follows:
- *   1. --ab-loop-a, if set.
- *   2. The Playback Start PTS, if set.
- *   3. MP_NOPTS_VALUE.
- * If unspecified, return MP_NOPTS_VALUE.
- * Does not return zero unless the start time is explicitly set to zero.
- */
-double get_ab_loop_start_time(struct MPContext *mpctx)
+// Get timestamps to use for AB-loop. Returns false iff any of the timestamps
+// are invalid and/or AB-loops are currently disabled, and set t[] to either
+// the user options or NOPTS on best effort basis.
+bool get_ab_loop_times(struct MPContext *mpctx, double t[2])
 {
     struct MPOpts *opts = mpctx->opts;
-    double ab_loop_start_time;
-    if (opts->ab_loop[0] != MP_NOPTS_VALUE) {
-        ab_loop_start_time = opts->ab_loop[0];
-    } else {
-        /*
-         * There is no check for MP_NOPTS_VALUE here
-         * because that's exactly what we want to return
-         * if get_play_start_pts comes up empty here.
-         */
-        ab_loop_start_time = get_play_start_pts(mpctx);
-    }
-    return ab_loop_start_time;
+    int dir = mpctx->play_dir;
+
+    t[0] = opts->ab_loop[0];
+    t[1] = opts->ab_loop[1];
+
+    if (t[0] == MP_NOPTS_VALUE || t[1] == MP_NOPTS_VALUE || t[0] == t[1])
+        return false;
+
+    if (t[0] * dir > t[1] * dir)
+        MPSWAP(double, t[0], t[1]);
+
+    return true;
 }
 
 double get_track_seek_offset(struct MPContext *mpctx, struct track *track)