options: allow using % for width and height in --geometry

Now all numbers in the --geometry specification can take percentages.

Rewrite the parsing of --geometry, because adjusting the sscanf() mess
would require adding all the combinations of using and not using %. As
a side effect, using % and pixel values can be freely mixed.

Keep the aspect if only one of width or height is set. This is more
useful in general.

Note: there is one semantic change: --geometry=num used to mean setting
the window X position, but now it means setting the window width.
Apparently this was a mplayer-specific feature (not part of standard X
geometry specifications), and it doesn't look like an overly useful
feature, so we are fine with breaking it.

In general, the new parsing should still adhere to standard X geometry
specification (as used by XParseGeometry()).

3 files changed, 106 insertions, 76 deletions

diff --git a/DOCS/man/en/options.rst b/DOCS/man/en/options.rst
index f2787cdf78..fb3a96307d 100644
--- a/DOCS/man/en/options.rst
+++ b/DOCS/man/en/options.rst
@@ -676,22 +676,24 @@
     consider using options such as ``--srate`` and ``--format`` to explicitly
     select what the shared output format will be.
 
---geometry=<x[%][:y[%]]>, --geometry=<[WxH][+-x+-y]>
-    Adjust where the output is on the screen initially. The x and y
-    specifications are in pixels measured from the top-left of the screen to
-    the top-left of the image being displayed, however if a percentage sign is
-    given after the argument it turns the value into a percentage of the
-    screen size in that direction. It also supports the standard X11
-    ``--geometry`` option format, in which e.g. +10-50 means "place 10 pixels
-    from the left border and 50 pixels from the lower border" and "--20+-10"
-    means "place 20 pixels beyond the right and 10 pixels beyond the top
-    border". If an external window is specified using the ``--wid`` option,
+--geometry=<[W[xH]][+-x+-y]>, --geometry=<x:y>
+    Adjust the initial window position or size. W and H set the window size in
+    pixels. x and y set the window position, measured in pixels from the
+    top-left of the screen to the top-left of the image being displayed. If a
+    percentage sign (``%``) is given after the argument it turns the value into
+    a percentage of the screen size in that direction. Positions are specified
+    similar to the standard X11 ``--geometry`` option format, in which e.g.
+    +10-50 means "place 10 pixels from the left border and 50 pixels from the
+    lower border" and "--20+-10" means "place 20 pixels beyond the right and
+    10 pixels beyond the top border".
+
+    If an external window is specified using the ``--wid`` option,
     then the x and y coordinates are relative to the top-left corner of the
     window rather than the screen. The coordinates are relative to the screen
     given with ``--screen`` for the video output drivers that fully
     support ``--screen``.
 
-    *NOTE*: May not be supported by some of the older VO drivers.
+    *NOTE*: Generally only supported by GUI VOs. Ignored for encoding.
 
     *NOTE (OSX)*: On Mac OSX the origin of the screen coordinate system is
     located on the the bottom-left corner. For instance, ``0:0`` will place the
@@ -703,10 +705,18 @@
         Places the window at x=50, y=40.
     ``50%:50%``
         Places the window in the middle of the screen.
-    ``100%``
-        Places the window at the middle of the right edge of the screen.
     ``100%:100%``
         Places the window at the bottom right corner of the screen.
+    ``50%``
+        Sets the window width to half the screen width. Window height is set so
+        that the window has the video aspect ratio.
+    ``50%x50%``
+        Forces the window width and height to half the screen width and height.
+        Will show black borders to compensate for the video aspect ration (with
+        most VOs and without ``--no-keepaspect``).
+    ``50%+10+10``
+        Sets the window to half the screen widths, and positions it 10 pixels
+        below/left of the top left corner of the screen.
 
 --grabpointer, --no-grabpointer
     ``--no-grabpointer`` tells the player to not grab the mouse pointer after a
diff --git a/core/m_option.c b/core/m_option.c
index 0811e99a79..a55e87a5d3 100644
--- a/core/m_option.c
+++ b/core/m_option.c
@@ -1166,69 +1166,83 @@ const m_option_type_t m_option_type_color = {
 };
 
 
+// Parse a >=0 number starting at s. Set s to the string following the number.
+// If the number ends with '%', eat that and set *out_per to true, but only
+// if the number is between 0-100; if not, don't eat anything, even the number.
+static bool eat_num_per(bstr *s, int *out_num, bool *out_per)
+{
+    bstr rest;
+    long long v = bstrtoll(*s, &rest, 10);
+    if (s->len == rest.len || v < INT_MIN || v > INT_MAX)
+        return false;
+    *out_num = v;
+    *out_per = false;
+    *s = rest;
+    if (bstr_eatstart0(&rest, "%") && v >= 0 && v <= 100) {
+        *out_per = true;
+        *s = rest;
+    }
+    return true;
+}
 
-static bool parse_geometry_str(struct m_geometry *gm, char *s)
+static bool parse_geometry_str(struct m_geometry *gm, bstr s)
 {
-    if (s == NULL)
+    *gm = (struct m_geometry) { .x = INT_MIN, .y = INT_MIN };
+    if (s.len == 0)
         return true;
-    char xsign[2], ysign[2], dummy[2];
-    int width, height, xoff, yoff, xper, yper;
-    int ok = 0;
-    for (int i = 0; !ok && i < 9; i++) {
-        width = height = xoff = yoff = xper = yper = INT_MIN;
-        strcpy(xsign, "+");
-        strcpy(ysign, "+");
-        switch (i) {
-        case 0:
-            ok = sscanf(s, "%ix%i%1[+-]%i%1[+-]%i%c",
-                        &width, &height, xsign, &xoff, ysign,
-                        &yoff, dummy) == 6;
-            break;
-        case 1:
-            ok = sscanf(s, "%ix%i%c", &width, &height, dummy) == 2;
-            break;
-        case 2:
-            ok = sscanf(s, "%1[+-]%i%1[+-]%i%c",
-                        xsign, &xoff, ysign, &yoff, dummy) == 4;
-            break;
-        case 3:
-            ok = sscanf(s, "%i%%:%i%1[%]%c", &xper, &yper, dummy, dummy) == 3;
-            break;
-        case 4:
-            ok = sscanf(s, "%i:%i%1[%]%c", &xoff, &yper, dummy, dummy) == 3;
-            break;
-        case 5:
-            ok = sscanf(s, "%i%%:%i%c", &xper, &yoff, dummy) == 2;
-            break;
-        case 6:
-            ok = sscanf(s, "%i:%i%c", &xoff, &yoff, dummy) == 2;
-            break;
-        case 7:
-            ok = sscanf(s, "%i%1[%]%c", &xper, dummy, dummy) == 2;
-            break;
-        case 8:
-            ok = sscanf(s, "%i%c", &xoff, dummy) == 1;
-            break;
+    // Approximate grammar:
+    // [W[xH]][{+-}X{+-}Y] | [X:Y]
+    // (meaning: [optional] {one character of} one|alternative)
+    // Every number can be followed by '%'
+    int num;
+    bool per;
+
+#define READ_NUM(F, F_PER) do {         \
+    if (!eat_num_per(&s, &num, &per))   \
+        goto error;                     \
+    gm->F = num;                        \
+    gm->F_PER = per;                    \
+} while(0)
+
+#define READ_SIGN(F) do {               \
+    if (bstr_eatstart0(&s, "+")) {      \
+        gm->F = false;                  \
+    } else if (bstr_eatstart0(&s, "-")) {\
+        gm->F = true;                   \
+    } else goto error;                  \
+} while(0)
+
+    if (bstrchr(s, ':') < 0) {
+        gm->wh_valid = true;
+        if (!bstr_startswith0(s, "+") && !bstr_startswith0(s, "-")) {
+            READ_NUM(w, w_per);
+            if (bstr_eatstart0(&s, "x"))
+                READ_NUM(h, h_per);
+        }
+        if (s.len > 0) {
+            gm->xy_valid = true;
+            READ_SIGN(x_sign);
+            READ_NUM(x, x_per);
+            READ_SIGN(y_sign);
+            READ_NUM(y, y_per);
         }
+    } else {
+        gm->xy_valid = true;
+        READ_NUM(x, x_per);
+        if (!bstr_eatstart0(&s, ":"))
+            goto error;
+        READ_NUM(y, y_per);
     }
-    if (!ok)
-        return false;
-
-    gm->x_per = xper >= 0 && xper <= 100;
-    gm->x = gm->x_per ? xper : xoff;
-    gm->x_sign = xsign[0] == '-';
-    gm->y_per = yper >= 0 && yper <= 100;
-    gm->y = gm->y_per ? yper : yoff;
-    gm->y_sign = ysign[0] == '-';
-    gm->xy_valid = gm->x != INT_MIN || gm->y != INT_MIN;
 
-    gm->w = width;
-    gm->h = height;
-    gm->wh_valid = gm->w > 0 || gm->h > 0;
+    return s.len == 0;
 
-    return true;
+error:
+    return false;
 }
 
+#undef READ_NUM
+#undef READ_SIGN
+
 // xpos,ypos: position of the left upper corner
 // widw,widh: width and height of the window
 // scrw,scrh: width and height of the current screen
@@ -1237,10 +1251,18 @@ void m_geometry_apply(int *xpos, int *ypos, int *widw, int *widh,
                       int scrw, int scrh, struct m_geometry *gm)
 {
     if (gm->wh_valid) {
+        int prew = *widw, preh = *widh;
         if (gm->w > 0)
-            *widw = gm->w;
+            *widw = gm->w_per ? scrw * (gm->w / 100.0) : gm->w;
         if (gm->h > 0)
-            *widh = gm->h;
+            *widh = gm->h_per ? scrh * (gm->h / 100.0) : gm->h;
+        // keep aspect if the other value is not set
+        double asp = (double)prew / preh;
+        if (gm->w > 0 && !(gm->h > 0)) {
+            *widh = *widw / asp;
+        } else if (!(gm->w > 0) && gm->h > 0) {
+            *widw = *widh * asp;
+        }
     }
 
     if (gm->xy_valid) {
@@ -1255,7 +1277,7 @@ void m_geometry_apply(int *xpos, int *ypos, int *widw, int *widh,
             *ypos = gm->y;
             if (gm->y_per)
                 *ypos = (scrh - *widh) * (*ypos / 100.0);
-            if (gm->x_sign)
+            if (gm->y_sign)
                 *ypos = scrh - *widh - *ypos;
         }
     }
@@ -1264,11 +1286,8 @@ void m_geometry_apply(int *xpos, int *ypos, int *widw, int *widh,
 static int parse_geometry(const m_option_t *opt, struct bstr name,
                           struct bstr param, void *dst)
 {
-    char *s = bstrdup0(NULL, param);
-    struct m_geometry gm = {0};
-    bool res = parse_geometry_str(&gm, s);
-    talloc_free(s);
-    if (!res)
+    struct m_geometry gm;
+    if (!parse_geometry_str(&gm, param))
         goto error;
 
     if (dst)
@@ -1280,7 +1299,7 @@ error:
     mp_msg(MSGT_CFGPARSER, MSGL_ERR, "Option %.*s: invalid geometry: '%.*s'\n",
            BSTR_P(name), BSTR_P(param));
     mp_msg(MSGT_CFGPARSER, MSGL_ERR,
-           "Valid format: [WxH][[+-]X[+-]Y] | [X[%%]:[Y[%%]]]\n");
+           "Valid format: [W[%%][xH[%%]]][{+-}X[%%]{+-}Y[%%]] | [X[%%]:Y[%%]]\n");
     return M_OPT_INVALID;
 }
 
diff --git a/core/m_option.h b/core/m_option.h
index ca75d494bb..87fa959992 100644
--- a/core/m_option.h
+++ b/core/m_option.h
@@ -81,6 +81,7 @@ struct m_color {
 struct m_geometry {
     int x, y, w, h;
     bool xy_valid : 1, wh_valid : 1;
+    bool w_per : 1, h_per : 1;
     bool x_sign : 1, y_sign : 1, x_per : 1, y_per : 1;
 };