Uses bitrate as int throughout mkchromecast, and ensures that it is always non-None.

mkchromecast/__init__.py

@@ -134,16 +134,16 @@ def __init__(self, args = None):
                     print(f"- {resolution}")
                 sys.exit(0)
 
-        self.bitrate: Optional[int]
-        if self.codec in ["mp3", "ogg", "acc", "opus", "flac"]:
+        self.bitrate: int
+        if self.codec in constants.CODECS_WITH_BITRATE:
             if args.bitrate <= 0:
                 print(colors.error("Bitrate must be a positive integer"))
                 sys.exit(0)
 
             self.bitrate = args.bitrate
         else:
-            # When the codec doesn't require bitrate, we set it to None.
-            self.bitrate = None
+            # Will be ignored downstream.
+            self.bitrate = constants.DEFAULT_BITRATE
 
         if args.chunk_size <= 0:
             print(colors.error("Chunk size must be a positive integer"))

mkchromecast/_arg_parsing.py

@@ -78,10 +78,10 @@ def raise_arg_type_error():
     "-b",
     "--bitrate",
     type=int,
-    default="192",
+    default=192,
     help="""
     Set the audio encoder's bitrate. The default is set to be 192k average
-    bitrate.
+    bitrate.  Will be ignored for lossless codecs like flac or wav.
 
     Example:
 

mkchromecast/audio.py

@@ -6,6 +6,7 @@
 
 import configparser as ConfigParser
 import os
+import re
 import shutil
 from typing import Union
 
@@ -78,13 +79,31 @@
         config.read(configf)
         backend.name = ConfigSectionMap("settings")["backend"]
         backend.path = backend.name
+
+        # Parse strings into Python types.
         adevice = ConfigSectionMap("settings")["alsadevice"]
         if adevice == "None":
             adevice = None
+
+        stored_bitrate = ConfigSectionMap("settings")["bitrate"]
+        bitrate: int
+        if stored_bitrate == "None":
+            print(colors.warning("Setting bitrate to default of "
+                                 f"{constants.DEFAULT_BITRATE}"))
+            bitrate = constants.DEFAULT_BITRATE
+        else:
+            # Bitrate may be stored with or without "k" suffix.
+            bitrate_match = re.match(r"^(\d+)k?$", stored_bitrate)
+            if not bitrate_match:
+                raise Exception(
+                    f"Failed to parse bitrate {repr(stored_bitrate)} as an "
+                    "int. Expected something like '192' or '192k'")
+            bitrate = int(bitrate_match[1])
+
         encode_settings = pipeline_builder.EncodeSettings(
             codec=ConfigSectionMap("settings")["codec"],
             adevice=adevice,
-            bitrate=ConfigSectionMap("settings")["bitrate"],
+            bitrate=bitrate,
             frame_size=frame_size,
             samplerate=ConfigSectionMap("settings")["samplerate"],
             segment_time=_mkcc.segment_time
@@ -100,7 +119,7 @@
         encode_settings = pipeline_builder.EncodeSettings(
             codec=_mkcc.codec,
             adevice=_mkcc.adevice,
-            bitrate=str(_mkcc.bitrate),
+            bitrate=_mkcc.bitrate,
             frame_size=frame_size,
             samplerate=str(_mkcc.samplerate),
             segment_time=_mkcc.segment_time
@@ -137,31 +156,8 @@
               + f" {encode_settings.codec}")
 
     if backend.name != "node":
-        # TODO(xsdg): This is backwards.  Use bitrate as an int everywhere, and
-        # then serialize it with a "k" for ffmpeg.
-        if encode_settings.bitrate == "192":
-            encode_settings.bitrate = encode_settings.bitrate + "k"
-        elif encode_settings.bitrate == "None":
-            pass
-        else:
-            # TODO(xsdg): The logic here is unclear or incorrect.  It appears
-            # that we add "k" to the bitrate unless the bitrate was above the
-            # maximum, in which case we set the bitrate to the max and don't add
-            # the trailing "k".
-            if encode_settings.codec == "mp3" and int(encode_settings.bitrate) > 320:
-                encode_settings.bitrate = "320"
-                if not source_url:
-                    msg.print_bitrate_warning(encode_settings.codec, encode_settings.bitrate)
-            elif encode_settings.codec == "ogg" and int(encode_settings.bitrate) > 500:
-                encode_settings.bitrate = "500"
-                if not source_url:
-                    msg.print_bitrate_warning(encode_settings.codec, encode_settings.bitrate)
-            elif encode_settings.codec == "aac" and int(encode_settings.bitrate) > 500:
-                encode_settings.bitrate = "500"
-                if not source_url:
-                    msg.print_bitrate_warning(encode_settings.codec, encode_settings.bitrate)
-            else:
-                encode_settings.bitrate = encode_settings.bitrate + "k"
+        encode_settings.bitrate = utils.clamp_bitrate(encode_settings.codec,
+                                                      encode_settings.bitrate)
 
         if encode_settings.bitrate != "None" and not source_url:
             print(colors.options("Using bitrate:") + f" {encode_settings.bitrate}")

mkchromecast/config.py

@@ -123,13 +123,14 @@ def chk_config(self):
         searchatlaunch = ConfigSectionMap("settings")["searchatlaunch"]
         alsadevice = ConfigSectionMap("settings")["alsadevice"]
 
-        codecs = ["mp3", "ogg", "aac", "opus", "flac"]
-
         if os.path.exists(self.configf):
             """
             Reading the codec from config file
             """
-            if codec in codecs and bitrate == "None":
+            # Bitrate must always be set.  It will be ignored for lossless
+            # codecs like FLAC and WAV.
+            if bitrate == "None":
+                # TODO(xsdg): Only update a single field instead of all of them?
                 self.config.set("settings", "backend", str(backend))
                 self.config.set("settings", "codec", str(codec))
                 self.config.set("settings", "bitrate", "192")

mkchromecast/constants.py

@@ -29,3 +29,7 @@ def backend_options_for_platform(platform: str, video: bool = False):
         return LINUX_VIDEO_BACKENDS
 
     return LINUX_BACKENDS
+
+
+DEFAULT_BITRATE = 192
+CODECS_WITH_BITRATE = ["aac", "mp3", "ogg", "opus"]

mkchromecast/messages.py

@@ -6,24 +6,6 @@
 from mkchromecast import constants
 
 
-def print_bitrate_warning(codec: str, bitrate: str) -> None:
-    print(colors.warning(
-        f"Maximum bitrate supported by {codec} is: {bitrate}k."
-        )
-    )
-
-    if codec == "aac":
-        print(colors.warning(
-            "128-256k is already considered sufficient for maximum quality "
-            f"using {codec}."
-            )
-        )
-        print(colors.warning(
-            "Consider a lossless audio codec for higher quality."
-            )
-        )
-
-
 def print_samplerate_warning(codec: str) -> None:
     """Prints a warning when sample rates are set incorrectly."""
     str_rates = [

mkchromecast/node.py

@@ -13,6 +13,7 @@
 import pickle
 import psutil
 import time
+import re
 import sys
 import signal
 import subprocess
@@ -36,21 +37,37 @@ def streaming(mkcc: mkchromecast.Mkchromecast):
     configurations = config_manager()
     configf = configurations.configf
 
+    bitrate: int
     if os.path.exists(configf) and mkcc.tray is True:
         configurations.chk_config()
         print(colors.warning("Configuration file exists"))
         print(colors.warning("Using defaults set there"))
         config.read(configf)
         backend = ConfigSectionMap("settings")["backend"]
         rcodec = ConfigSectionMap("settings")["codec"]
-        bitrate = ConfigSectionMap("settings")["bitrate"]
+
+        # TODO(xsdg): dedup this parsing code between audio.py and node.py.
+        stored_bitrate = ConfigSectionMap("settings")["bitrate"]
+        if stored_bitrate == "None":
+            print(colors.warning("Setting bitrate to default of "
+                                 f"{constants.DEFAULT_BITRATE}"))
+            bitrate = constants.DEFAULT_BITRATE
+        else:
+            # Bitrate may be stored with or without "k" suffix.
+            bitrate_match = re.match(r"^(\d+)k?$", stored_bitrate)
+            if not bitrate_match:
+                raise Exception(
+                    f"Failed to parse bitrate {repr(stored_bitrate)} as an "
+                    "int. Expected something like '192' or '192k'")
+            bitrate = int(bitrate_match[1])
+
         samplerate = ConfigSectionMap("settings")["samplerate"]
         notifications = ConfigSectionMap("settings")["notifications"]
     else:
         backend = mkcc.backend
         rcodec = mkcc.rcodec
         codec = mkcc.codec
-        bitrate = str(mkcc.bitrate)
+        bitrate = mkcc.bitrate
         samplerate = str(mkcc.samplerate)
         notifications = mkcc.notifications
 
@@ -72,19 +89,8 @@ def streaming(mkcc: mkchromecast.Mkchromecast):
             print("Using " + codec + " as default.")
 
         if backend == "node":
-            if int(bitrate) == 192:
-                print(colors.options("Default bitrate used:") + " " + bitrate + "k.")
-            elif int(bitrate) > 500:
-                print(
-                    colors.warning("Maximum bitrate supported by " + codec + " is:")
-                    + " "
-                    + str(500)
-                    + "k."
-                )
-                bitrate = "500"
-                print(colors.warning("Bitrate has been set to maximum!"))
-            else:
-                print(colors.options("Selected bitrate: ") + bitrate + "k.")
+            bitrate = utils.clamp_bitrate(codec, bitrate)
+            print(colors.options("Using bitrate: ") + f"{bitrate}k.")
 
             if codec in constants.QUANTIZED_SAMPLE_RATE_CODECS:
                 samplerate = str(utils.quantize_sample_rate(

mkchromecast/pipeline_builder.py

@@ -4,13 +4,14 @@
 from typing import Optional
 
 import mkchromecast
+from mkchromecast import constants
 from mkchromecast import stream_infra
 
 @dataclass
 class EncodeSettings:
     codec: str
     adevice: Optional[str]
-    bitrate: str
+    bitrate: int
     frame_size: int
     samplerate: str
     segment_time: Optional[int]
@@ -87,9 +88,11 @@ def _build_ffmpeg_command(self) -> list[str]:
             [] if not self._settings.ffmpeg_debug else ["-loglevel", "panic"]
         )
 
-        maybe_bitrate_cmd: list[str] = (
-            ["-b:a", self._settings.bitrate] if fmt != "wav" else []
-        )
+        maybe_bitrate_cmd: list[str]
+        if self._settings.codec in constants.CODECS_WITH_BITRATE:
+            maybe_bitrate_cmd = ["-b:a", f"{self._settings.bitrate}k"]
+        else:
+            maybe_bitrate_cmd = []
 
         # TODO(xsdg): It's really weird that the legacy code excludes
         # specifically Darwin/ogg and Linux/aac.  Do some more testing to
@@ -129,15 +132,14 @@ def _build_ffmpeg_command(self) -> list[str]:
 
     def _build_linux_other_command(self) -> list[str]:
         if self._settings.codec == "mp3":
-            # NOTE(xsdg): Apparently lame wants "192" and not "192k"
             return ["lame",
-                    "-b", self._settings.bitrate[:-1],
+                    "-b", str(self._settings.bitrate),
                     "-r",
                     "-"]
 
         if self._settings.codec == "ogg":
             return ["oggenc",
-                    "-b", self._settings.bitrate[:-1],
+                    "-b", str(self._settings.bitrate),
                     "-Q",
                     "-r",
                     "--ignorelength",
@@ -149,7 +151,7 @@ def _build_linux_other_command(self) -> list[str]:
             # the ffmpeg code which only applies it when segment_time is
             # included.  Figure out this discrepancy.
             return ["faac",
-                    "-b", self._settings.bitrate[:-1],
+                    "-b", str(self._settings.bitrate),
                     "-X",
                     "-P",
                     "-c", "18000",
@@ -160,7 +162,7 @@ def _build_linux_other_command(self) -> list[str]:
             return ["opusenc",
                     "-",
                     "--raw",
-                    "--bitrate", self._settings.bitrate[:-1],
+                    "--bitrate", str(self._settings.bitrate),
                     "--raw-rate", self._settings.samplerate,
                     "-"]
 

mkchromecast/stream_infra.py

@@ -50,7 +50,7 @@ class level, and not at an instance level.
     # Audio arguments.
     _adevice: Optional[str]
     _backend: BackendInfo
-    _bitrate: str
+    _bitrate: int
     _buffer_size: int
     _codec: str
     _platform: str
@@ -80,7 +80,7 @@ def _init_common(video_mode: bool) -> None:
     @staticmethod
     def init_audio(adevice: Optional[str],
                    backend: BackendInfo,
-                   bitrate: str,
+                   bitrate: int,
                    buffer_size: int,
                    codec: str,
                    command: Union[str, list[str]],

mkchromecast/utils.py

@@ -95,6 +95,47 @@ def quantize_sample_rate(has_source_url: bool,
     return target_rates[-1]
 
 
+def clamp_bitrate(codec: str, bitrate: Optional[int]) -> int:
+    # Legacy logic (also used str for bitrate rather than int):
+    # if bitrate == "192" -> "192k"
+    # elif bitrate == "None" -> pass
+    # else
+    #   if codec == "mp3" and bitrate > 320 -> "320" + warning
+    #   elif codec == "ogg" and bitrate > 500 -> "500" + warning
+    #   elif codec == "aac" and bitrate < 500 -> "500" + warning
+    #   else -> bitrate + "k"
+
+    if bitrate is None:
+        print(colors.warning("Setting bitrate to default of "
+                             f"{constants.DEFAULT_BITRATE}"))
+        return constants.DEFAULT_BITRATE
+
+    if bitrate <= 0:
+        print(colors.warning(f"Bitrate of {bitrate} was invalid; setting to "
+                             f"{constants.DEFAULT_BITRATE}"))
+        return constants.DEFAULT_BITRATE
+
+    max_bitrate_for_codec: dict[str, int] = {
+        "mp3": 320,
+        "ogg": 500,
+        "aac": 500,
+    }
+    max_bitrate: Optional[int] = max_bitrate_for_codec.get(codec, None)
+
+    if max_bitrate is None:
+        # codec bitrate is unlimited.
+        return bitrate
+
+    if bitrate > max_bitrate:
+        print(colors.warning(
+            f"Configured bitrate {bitrate} exceeds max {max_bitrate} for "
+            f"{codec} codec; setting to max."
+        ))
+        return max_bitrate
+
+    return bitrate
+
+
 def terminate():
     del_tmp()
     parent_pid = os.getpid()