Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make FFmpegOpusAudio codec behave as docs suggest #2581

Merged

Conversation

felix920506
Copy link
Contributor

@felix920506 felix920506 commented Sep 18, 2024

Summary

make FFmpegOpusAudio "codec" parameter behave as what the inline docs suggest

Original behavior:
passing in "libopus" or "opus" will result in "copy" being passed into ffmpeg
passing in anything else (including "copy") will result in "libopus" being passed into ffmpeg

New behavior:
passing in "copy" will result in "copy" being passed into ffmpeg
passing in anything else (including "libopus", "opus") will result in "libopus" being passed into ffmpeg

Information

  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed).
  • This PR is not a code change (e.g. documentation, README, typehinting,
    examples, ...).

Checklist

  • I have searched the open pull requests for duplicates.
  • If code changes were made then they have been tested.
  • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why.
  • I have updated the changelog to include these changes.

@felix920506 felix920506 changed the title make FFmpegOpusAudio codec behave as docs suggest fix: make FFmpegOpusAudio codec behave as docs suggest Sep 18, 2024
@plun1331
Copy link
Member

should we maybe change the docs instead because like, breaking changes and stuff

@Lulalaby
Copy link
Member

It's more an internal change. So I wouldn't consider it breaking

@felix920506
Copy link
Contributor Author

This is my first contribution here so I don't really know if this would count as breaking or not. I checked the box only because it technically changes the behavior of this function.

@felix920506
Copy link
Contributor Author

I don't think this is going to be much an issue since the only scenario it would break functionality on existing bots would be someone passing in "copy" for a source that isn't opus, which is something that was never supposed to work according to the docs.
The fix this will require under that scenario would be a few lines at most (Likely single line changes for most cases.)

@felix920506
Copy link
Contributor Author

I see this tagged with "changelog needed", is there anything that I need to do? If so are there instructions?

@JustaSqu1d
Copy link
Member

I see this tagged with "changelog needed", is there anything that I need to do? If so are there instructions?

Here's an example of a changelog entry being added: 7fb1a4b

@felix920506
Copy link
Contributor Author

I see this tagged with "changelog needed", is there anything that I need to do? If so are there instructions?

Here's an example of a changelog entry being added: 7fb1a4b

would this be "Fixed" or "Changed" ?

@Lulalaby
Copy link
Member

Fixed

@felix920506
Copy link
Contributor Author

I have added an entry to the changelog.

CHANGELOG.md Outdated Show resolved Hide resolved
discord/player.py Outdated Show resolved Hide resolved
felix920506 and others added 2 commits September 20, 2024 14:01
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: felix920506 <felix920506@gmail.com>
@JustaSqu1d JustaSqu1d added status: awaiting review Awaiting review from a maintainer and removed changelog needed labels Sep 20, 2024
@plun1331 plun1331 enabled auto-merge (squash) September 20, 2024 18:07
@plun1331 plun1331 added this to the v2.7 milestone Sep 20, 2024
@plun1331 plun1331 added the bug Something isn't working label Sep 20, 2024
@plun1331 plun1331 merged commit bef59ac into Pycord-Development:master Sep 20, 2024
25 checks passed
baribarton pushed a commit to baribarton/pycord-no-potential-reconnect that referenced this pull request Oct 24, 2024
…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 <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: felix920506 <felix920506@gmail.com>

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

---------

Signed-off-by: felix920506 <felix920506@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
OmLanke pushed a commit to OmLanke/pycord that referenced this pull request Dec 16, 2024
…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 <89910983+JustaSqu1d@users.noreply.github.com>
Signed-off-by: felix920506 <felix920506@gmail.com>

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

---------

Signed-off-by: felix920506 <felix920506@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: JustaSqu1d <89910983+JustaSqu1d@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: medium Medium Priority status: awaiting review Awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants