Skip to content

Commit

Permalink
fix: make FFmpegOpusAudio codec behave as docs suggest (Pycord-Develo…
Browse files Browse the repository at this point in the history
…pment#2581)

* make ffmpegopusaudio codec behave according to docs

* style(pre-commit): auto fixes from pre-commit.com hooks

* Update CHANGELOG.md

* Apply suggestions from code review

Co-authored-by: JustaSqu1d <[email protected]>
Signed-off-by: felix920506 <[email protected]>

* style(pre-commit): auto fixes from pre-commit.com hooks

---------

Signed-off-by: felix920506 <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: JustaSqu1d <[email protected]>
  • Loading branch information
3 people authored Sep 20, 2024
1 parent e09c607 commit bef59ac
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ These changes are available on the `master` branch, but have not yet been releas

- Fixed `Enum` options not setting the correct type when only one choice is available.
([#2577](https://github.com/Pycord-Development/pycord/pull/2577))
- Fixed `codec` option for `FFmpegOpusAudio` class to make it in line with
documentation. ([#2581](https://github.com/Pycord-Development/pycord/pull/2581))

### Changed

Expand Down
30 changes: 21 additions & 9 deletions discord/player.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,9 @@ class FFmpegOpusAudio(FFmpegAudio):
The codec to use to encode the audio data. Normally this would be
just ``libopus``, but is used by :meth:`FFmpegOpusAudio.from_probe` to
opportunistically skip pointlessly re-encoding Opus audio data by passing
``copy`` as the codec value. Any values other than ``copy``, ``opus``, or
``libopus`` will be considered ``libopus``. Defaults to ``libopus``.
``copy`` as the codec value. Any values other than ``copy``, or
``libopus`` will be considered ``libopus``. ``opus`` will also be considered
``libopus`` since the ``opus`` encoder is still in development. Defaults to ``libopus``.
.. warning::
Expand Down Expand Up @@ -407,7 +408,9 @@ def __init__(
args.append("-i")
args.append("-" if pipe else source)

codec = "copy" if codec in ("opus", "libopus") else "libopus"
# use "libopus" when "opus" is specified since the "opus" encoder is incomplete
# link to ffmpeg docs: https://www.ffmpeg.org/ffmpeg-codecs.html#opus
codec = "copy" if codec == "copy" else "libopus"

args.extend(
(
Expand All @@ -417,17 +420,24 @@ def __init__(
"opus",
"-c:a",
codec,
"-ar",
"48000",
"-ac",
"2",
"-b:a",
f"{bitrate}k",
"-loglevel",
"warning",
)
)

# only pass in bitrate, sample rate, channels arguments when actually encoding to avoid ffmpeg warnings
if codec != "copy":
args.extend(
(
"-ar",
"48000",
"-ac",
"2",
"-b:a",
f"{bitrate}k",
)
)

if isinstance(options, str):
args.extend(shlex.split(options))

Expand Down Expand Up @@ -501,6 +511,8 @@ def custom_probe(source, executable):

executable = kwargs.get("executable")
codec, bitrate = await cls.probe(source, method=method, executable=executable)
# only re-encode if the source isn't already opus, else directly copy the source audio stream
codec = "copy" if codec in ("opus", "libopus") else "libopus"
return cls(source, bitrate=bitrate, codec=codec, **kwargs) # type: ignore

@classmethod
Expand Down

0 comments on commit bef59ac

Please sign in to comment.