Repository: streamlink/streamlink
Pull request #3313 (2020-10-31 19:00:53)

cli: optional player-args input variable
- Make the player input variable in the `--player-args` parameter
  optional. If it's missing, append it to the parameter value.
- Rename it from `{filename}` to `{playerinput}`, to reduce confusion
  and add `{filename}` back as a secondary fallback variable.
- Update parameter docs accordingly and add+fix tests

See: #2947

gravyboat: Looks good, thanks!

Patch:
diff --git a/src/streamlink_cli/argparser.py b/src/streamlink_cli/argparser.py
index 8e9c22e4..d71968f4 100644
--- a/src/streamlink_cli/argparser.py
+++ b/src/streamlink_cli/argparser.py
@@ -9,7 +9,9 @@ from streamlink.utils.args import (
     boolean, comma_list, comma_list_filter, filesize, keyvalue, num
 )
 from streamlink.utils.times import hours_minutes_seconds
-from streamlink_cli.constants import (DEFAULT_PLAYER_ARGUMENTS, DEFAULT_STREAM_METADATA, STREAM_PASSTHROUGH, SUPPORTED_PLAYERS)
+from streamlink_cli.constants import (
+    DEFAULT_STREAM_METADATA, PLAYER_ARGS_INPUT_DEFAULT, PLAYER_ARGS_INPUT_FALLBACK, STREAM_PASSTHROUGH, SUPPORTED_PLAYERS
+)
 from streamlink_cli.utils import find_default_player
 
 _printable_re = re.compile(r"[{0}]".format(printable))
@@ -310,33 +312,36 @@ def build_parser():
     player.add_argument(
         "-a", "--player-args",
         metavar="ARGUMENTS",
-        default=DEFAULT_PLAYER_ARGUMENTS,
-        help="""
+        default="",
+        help=f"""
         This option allows you to customize the default arguments which are put
         together with the value of --player to create a command to execute.
-        Unlike the --player parameter, custom player arguments will not be logged.
 
-        This value can contain formatting variables surrounded by curly braces,
+        It's usually enough to only use --player instead of this unless you need
+        to add arguments after the player's input argument or if you don't want
+        any of the player arguments to be logged.
+
+        The value can contain formatting variables surrounded by curly braces,
         {{ and }}. If you need to include a brace character, it can be escaped
         by doubling, e.g. {{{{ and }}}}.
 
         Formatting variables available:
 
-        {{filename}}
-            This is the filename that the player will use. It's usually "-"
-            (stdin), but can also be a URL or a file depending on the options
-            used.
-
-        It's usually enough to use --player instead of this unless you need to
-        add arguments after the filename.
+        {{{PLAYER_ARGS_INPUT_DEFAULT}}}
+            This is the input that the player will use. For standard input (stdin),
+            it is ``-``, but it can also be a URL, depending on the options used.
 
-        Default is "{0}".
+        {{{PLAYER_ARGS_INPUT_FALLBACK}}}
+            The old fallback variable name with the same functionality.
 
         Example:
 
-          %(prog)s -p vlc -a "--play-and-exit {{filename}}" <url> [stream]
+          %(prog)s -p vlc -a "--play-and-exit {{{PLAYER_ARGS_INPUT_DEFAULT}}}" <url> [stream]
 
-        """.format(DEFAULT_PLAYER_ARGUMENTS)
+        Note: When neither of the variables are found, ``{{{PLAYER_ARGS_INPUT_DEFAULT}}}``
+        will be appended to the whole parameter value, to ensure that the player
+        always receives an input argument.
+        """
     )
     player.add_argument(
         "-v", "--verbose-player",
diff --git a/src/streamlink_cli/constants.py b/src/streamlink_cli/constants.py
index 75d138ec..0d4dc809 100644
--- a/src/streamlink_cli/constants.py
+++ b/src/streamlink_cli/constants.py
@@ -2,7 +2,9 @@ import os
 
 from streamlink_cli.compat import is_win32
 
-DEFAULT_PLAYER_ARGUMENTS = "{filename}"
+PLAYER_ARGS_INPUT_DEFAULT = "playerinput"
+PLAYER_ARGS_INPUT_FALLBACK = "filename"
+
 DEFAULT_STREAM_METADATA = {
     "title": "Unknown Title",
     "author": "Unknown Author",
@@ -34,6 +36,7 @@ STREAM_SYNONYMS = ["best", "worst", "best-unfiltered", "worst-unfiltered"]
 STREAM_PASSTHROUGH = ["hls", "http", "rtmp"]
 
 __all__ = [
-    "CONFIG_FILES", "DEFAULT_PLAYER_ARGUMENTS",
-    "PLUGINS_DIR", "STREAM_SYNONYMS", "STREAM_PASSTHROUGH"
+    "PLAYER_ARGS_INPUT_DEFAULT", "PLAYER_ARGS_INPUT_FALLBACK",
+    "DEFAULT_STREAM_METADATA", "SUPPORTED_PLAYERS",
+    "CONFIG_FILES", "PLUGINS_DIR", "STREAM_SYNONYMS", "STREAM_PASSTHROUGH"
 ]
diff --git a/src/streamlink_cli/output.py b/src/streamlink_cli/output.py
index 2267e9c4..60d1977b 100644
--- a/src/streamlink_cli/output.py
+++ b/src/streamlink_cli/output.py
@@ -1,12 +1,13 @@
 import logging
 import os
+import re
 import shlex
 import subprocess
 import sys
 from time import sleep
 
 from streamlink_cli.compat import is_win32, stdout
-from streamlink_cli.constants import DEFAULT_PLAYER_ARGUMENTS, SUPPORTED_PLAYERS
+from streamlink_cli.constants import PLAYER_ARGS_INPUT_DEFAULT, PLAYER_ARGS_INPUT_FALLBACK, SUPPORTED_PLAYERS
 from streamlink_cli.utils import ignored
 
 if is_win32:
@@ -77,7 +78,12 @@ class FileOutput(Output):
 class PlayerOutput(Output):
     PLAYER_TERMINATE_TIMEOUT = 10.0
 
-    def __init__(self, cmd, args=DEFAULT_PLAYER_ARGUMENTS, filename=None, quiet=True, kill=True,
+    _re_player_args_input = re.compile("|".join(map(
+        lambda const: re.escape(f"{{{const}}}"),
+        [PLAYER_ARGS_INPUT_DEFAULT, PLAYER_ARGS_INPUT_FALLBACK]
+    )))
+
+    def __init__(self, cmd, args="", filename=None, quiet=True, kill=True,
                  call=False, http=None, namedpipe=None, record=None, title=None):
         super().__init__()
         self.cmd = cmd
@@ -106,6 +112,9 @@ class PlayerOutput(Output):
             self.stdout = sys.stdout
             self.stderr = sys.stderr
 
+        if not self._re_player_args_input.search(self.args):
+            self.args += f"{' ' if self.args else ''}{{{PLAYER_ARGS_INPUT_DEFAULT}}}"
+
     @property
     def running(self):
         sleep(0.5)
@@ -201,7 +210,7 @@ class PlayerOutput(Output):
                     self.title = self.title.replace('"', '')
                     filename = filename[:-1] + '\\' + self.title + filename[-1]
 
-        args = self.args.format(filename=filename)
+        args = self.args.format(**{PLAYER_ARGS_INPUT_DEFAULT: filename, PLAYER_ARGS_INPUT_FALLBACK: filename})
         cmd = self.cmd
 
         # player command


Test patch:
diff --git a/tests/test_cli_main.py b/tests/test_cli_main.py
index 20177921..d01abe1a 100644
--- a/tests/test_cli_main.py
+++ b/tests/test_cli_main.py
@@ -139,6 +139,7 @@ class TestCLIMain(unittest.TestCase):
         args.record_and_pipe = None
         args.title = None
         args.player = "mpv"
+        args.player_args = ""
         self.assertIsInstance(create_output(FakePlugin), PlayerOutput)
 
     def test_create_output_file_output(self):
@@ -182,6 +183,7 @@ class TestCLIMain(unittest.TestCase):
             args.record_and_pipe = None
             args.title = None
             args.player = "mpv"
+            args.player_args = ""
             args.player_fifo = None
             self.assertIsInstance(create_output(FakePlugin), PlayerOutput)
         finally:
diff --git a/tests/test_cli_playerout.py b/tests/test_cli_playerout.py
index e3a80bf3..15a692a1 100644
--- a/tests/test_cli_playerout.py
+++ b/tests/test_cli_playerout.py
@@ -1,4 +1,4 @@
-from unittest.mock import ANY, patch
+from unittest.mock import ANY, Mock, patch
 
 from streamlink_cli.output import PlayerOutput
 from tests import posix_only, windows_only
@@ -7,27 +7,30 @@ UNICODE_TITLE = "기타치는소율 with UL섬 "
 
 
 @posix_only
-@patch('subprocess.Popen')
+@patch("streamlink_cli.output.sleep", Mock())
+@patch("subprocess.Popen")
 def test_output_mpv_unicode_title_posix(popen):
     po = PlayerOutput("mpv", title=UNICODE_TITLE)
     popen().poll.side_effect = lambda: None
     po.open()
-    popen.assert_called_with(['mpv', f'--title={UNICODE_TITLE}', '-'],
+    popen.assert_called_with(["mpv", f"--title={UNICODE_TITLE}", "-"],
                              bufsize=ANY, stderr=ANY, stdout=ANY, stdin=ANY)
 
 
 @posix_only
-@patch('subprocess.Popen')
+@patch("streamlink_cli.output.sleep", Mock())
+@patch("subprocess.Popen")
 def test_output_vlc_unicode_title_posix(popen):
     po = PlayerOutput("vlc", title=UNICODE_TITLE)
     popen().poll.side_effect = lambda: None
     po.open()
-    popen.assert_called_with(['vlc', '--input-title-format', UNICODE_TITLE, '-'],
+    popen.assert_called_with(["vlc", "--input-title-format", UNICODE_TITLE, "-"],
                              bufsize=ANY, stderr=ANY, stdout=ANY, stdin=ANY)
 
 
 @windows_only
-@patch('subprocess.Popen')
+@patch("streamlink_cli.output.sleep", Mock())
+@patch("subprocess.Popen")
 def test_output_mpv_unicode_title_windows_py3(popen):
     po = PlayerOutput("mpv.exe", title=UNICODE_TITLE)
     popen().poll.side_effect = lambda: None
@@ -37,10 +40,41 @@ def test_output_mpv_unicode_title_windows_py3(popen):
 
 
 @windows_only
-@patch('subprocess.Popen')
+@patch("streamlink_cli.output.sleep", Mock())
+@patch("subprocess.Popen")
 def test_output_vlc_unicode_title_windows_py3(popen):
     po = PlayerOutput("vlc.exe", title=UNICODE_TITLE)
     popen().poll.side_effect = lambda: None
     po.open()
     popen.assert_called_with(f"vlc.exe --input-title-format \"{UNICODE_TITLE}\" -",
                              bufsize=ANY, stderr=ANY, stdout=ANY, stdin=ANY)
+
+
+@posix_only
+def test_output_args_posix():
+    po_none = PlayerOutput("foo")
+    assert po_none._create_arguments() == ["foo", "-"]
+
+    po_implicit = PlayerOutput("foo", args="--bar")
+    assert po_implicit._create_arguments() == ["foo", "--bar", "-"]
+
+    po_explicit = PlayerOutput("foo", args="--bar {playerinput}")
+    assert po_explicit._create_arguments() == ["foo", "--bar", "-"]
+
+    po_fallback = PlayerOutput("foo", args="--bar {filename}")
+    assert po_fallback._create_arguments() == ["foo", "--bar", "-"]
+
+
+@windows_only
+def test_output_args_windows():
+    po_none = PlayerOutput("foo")
+    assert po_none._create_arguments() == "foo -"
+
+    po_implicit = PlayerOutput("foo", args="--bar")
+    assert po_implicit._create_arguments() == "foo --bar -"
+
+    po_explicit = PlayerOutput("foo", args="--bar {playerinput}")
+    assert po_explicit._create_arguments() == "foo --bar -"
+
+    po_fallback = PlayerOutput("foo", args="--bar {filename}")
+    assert po_fallback._create_arguments() == "foo --bar -"