Skip to content

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.)

@JustaSqu1d JustaSqu1d added hold: changelog This pull request is missing a changelog entry priority: medium Medium Priority labels Sep 20, 2024
@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.

@JustaSqu1d JustaSqu1d added status: awaiting review Awaiting review from a maintainer and removed hold: changelog This pull request is missing a changelog entry 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 <[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]>
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 <[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]>
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