Skip to content

refactor: python implementation of audioop.mul#2176

Merged
plun1331 merged 21 commits intoPycord-Development:masterfrom
davidhozic:feat/audioop
Oct 17, 2024
Merged

refactor: python implementation of audioop.mul#2176
plun1331 merged 21 commits intoPycord-Development:masterfrom
davidhozic:feat/audioop

Conversation

@davidhozic
Copy link
Contributor

@davidhozic davidhozic commented Jul 23, 2023 •
edited
Loading

Summary

This PR replaces the PCMVolumeTransformer.read() method with a Python equivalent, that does not use the audioop module, which is deprecated since Python 3.11 and will be removed in Python 3.13.

This implements #2177.

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.

@davidhozic davidhozic changed the title feat: Replace deprecated audioop feat: Replace deprecated audioop implementation of PCMVolumeTransformer.read Jul 23, 2023
@davidhozic davidhozic marked this pull request as ready for review July 23, 2023 10:54
@davidhozic davidhozic requested a review from a team as a code owner July 23, 2023 10:54
@pullapprove4 pullapprove4 bot requested a review from BobDotCom July 23, 2023 10:54
@pullapprove4 pullapprove4 bot requested a review from FrostByte266 July 23, 2023 10:54
@davidhozic davidhozic mentioned this pull request Jul 23, 2023
Closed
@Lulalaby Lulalaby linked an issue Jul 23, 2023 that may be closed by this pull request
Replace deprecated audioop implementation of PCMVolumeTransformer.read #2177
Closed
VincentRPS
VincentRPS previously approved these changes Jul 26, 2023
View reviewed changes
Copy link
Contributor

@VincentRPS VincentRPS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OmLanke
Copy link
Contributor

OmLanke commented Jul 27, 2023

A python equivalent will not be nearly as close as the C one in terms of performance. Live audio is a heavy load and has to be low latency.

@VincentRPS
Copy link
Contributor

VincentRPS commented Jul 27, 2023

A python equivalent will not be nearly as close as the C one in terms of performance. Live audio is a heavy load and has to be low latency.

I mean we can always rewrite it in Rust.

@VincentRPS
Copy link
Contributor

VincentRPS commented Jul 27, 2023

and also this largely uses python stdlib modules which from the seems of things are written in C in many places.

@OmLanke
Copy link
Contributor

OmLanke commented Jul 27, 2023

A python equivalent will not be nearly as close as the C one in terms of performance. Live audio is a heavy load and has to be low latency.

I mean we can always rewrite it in Rust.

The whole voice stuff could be rewritten to Rust 😬

@OmLanke
Copy link
Contributor

OmLanke commented Jul 27, 2023

and also this largely uses python stdlib modules which from the seems of things are written in C in many places.

Possible to do actual performance tests?

@VincentRPS
Copy link
Contributor

VincentRPS commented Jul 27, 2023

A python equivalent will not be nearly as close as the C one in terms of performance. Live audio is a heavy load and has to be low latency.

I mean we can always rewrite it in Rust.

The whole voice stuff could be rewritten to Rust 😬

only if :)

@davidhozic
Copy link
Contributor Author

davidhozic commented Jul 27, 2023

audioop:
image

Without audioop:
image

Hardware:
OS Name Microsoft Windows 10 Pro
Processor AMD Ryzen 5 5600X 6-Core Processor, 3701 Mhz, 6 Core(s), 12 Logical Processor(s)

@OmLanke
Copy link
Contributor

OmLanke commented Jul 27, 2023

Hmm that sure is a difference.

Use time.perf_counter_ns() instead of time.time(). The result will be more accurate and will return ns instead of s

@OmLanke
Copy link
Contributor

OmLanke commented Jul 27, 2023

Can we maintain and ship our own audioop C file? Dpy plans to do something like that

Or we could try Cython or maybe Rust?

@Lulalaby Lulalaby modified the milestones: v2.6, v2.7 Jul 9, 2024
Lulalaby and others added 2 commits August 16, 2024 14:53
@davidhozic
Copy link
Contributor Author

davidhozic commented Oct 7, 2024

bump

@davidhozic
Copy link
Contributor Author

davidhozic commented Oct 8, 2024 •
edited
Loading

Btw, that requested change is already resolved, not sure why it doesn't automatically detect it.

@davidhozic davidhozic requested a review from Lulalaby October 12, 2024 23:49
@Paillat-dev Paillat-dev mentioned this pull request Oct 15, 2024
Merged
Lulalaby
Lulalaby requested changes Oct 15, 2024
View reviewed changes
Copy link
Member

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this come from

Lulalaby
Lulalaby requested changes Oct 15, 2024
View reviewed changes
@davidhozic @Lulalaby
Update CHANGELOG.md
7e91341 
Co-authored-by: Lala Sabathil <aiko@aitsys.dev>
Signed-off-by: David Hozic <davidhozic@gmail.com>
@davidhozic
Copy link
Contributor Author

davidhozic commented Oct 16, 2024

Where does this come from

The changelog thing? It was probably added through some weird auto-merge.

@plun1331 plun1331 merged commit 33adf22 into Pycord-Development:master Oct 17, 2024
baribarton pushed a commit to baribarton/pycord-no-potential-reconnect that referenced this pull request Oct 24, 2024
@davidhozic @pre-commit-ci
@Dorukyum @Lulalaby @plun1331 @baribarton
refactor: python implementation of audioop.mul (Pycord-Development#…
8eec8da 
…2176)

* Replace audioop

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

* versionchanged

* changelog

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

* speed

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

* changelog

* Optimization (1 ms)

* Update discord/player.py

Co-authored-by: Dorukyum <53639936+Dorukyum@users.noreply.github.com>
Signed-off-by: David Hozic <davidhozic@gmail.com>

* Update CHANGELOG.md

Co-authored-by: Lala Sabathil <aiko@aitsys.dev>
Signed-off-by: David Hozic <davidhozic@gmail.com>

---------

Signed-off-by: David Hozic <davidhozic@gmail.com>
Signed-off-by: Lala Sabathil <lala@pycord.dev>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Dorukyum <53639936+Dorukyum@users.noreply.github.com>
Co-authored-by: Lala Sabathil <lala@pycord.dev>
Co-authored-by: plun1331 <plun1331@gmail.com>
Co-authored-by: Lala Sabathil <aiko@aitsys.dev>
OmLanke pushed a commit to OmLanke/pycord that referenced this pull request Dec 16, 2024
@davidhozic @pre-commit-ci
@Dorukyum @Lulalaby @plun1331 @OmLanke
refactor: python implementation of audioop.mul (Pycord-Development#…
461acfa 
…2176)

* Replace audioop

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

* versionchanged

* changelog

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

* speed

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

* changelog

* Optimization (1 ms)

* Update discord/player.py

Co-authored-by: Dorukyum <53639936+Dorukyum@users.noreply.github.com>
Signed-off-by: David Hozic <davidhozic@gmail.com>

* Update CHANGELOG.md

Co-authored-by: Lala Sabathil <aiko@aitsys.dev>
Signed-off-by: David Hozic <davidhozic@gmail.com>

---------

Signed-off-by: David Hozic <davidhozic@gmail.com>
Signed-off-by: Lala Sabathil <lala@pycord.dev>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Dorukyum <53639936+Dorukyum@users.noreply.github.com>
Co-authored-by: Lala Sabathil <lala@pycord.dev>
Co-authored-by: plun1331 <plun1331@gmail.com>
Co-authored-by: Lala Sabathil <aiko@aitsys.dev>
@davidhozic
Copy link
Contributor Author

davidhozic commented May 4, 2025

Are there any plans or are the main maintainters open for rewriting some of the audio functionalities in Rust (for performance reasons)?

@davidhozic davidhozic deleted the feat/audioop branch May 4, 2025 18:14
@DA-344
Copy link
Contributor

DA-344 commented May 4, 2025

Are there any plans or are the main maintainters open for rewriting some of the audio functionalities in Rust (for performance reasons)?

Py-cord is meant to be used in (almost) all machines, and writing Rust implementations will prevent some machines to use it anymore because they cannot compile such code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Lulalaby Lulalaby Lulalaby approved these changes

@plun1331 plun1331 plun1331 approved these changes

@VincentRPS VincentRPS VincentRPS left review comments

@BobDotCom BobDotCom Awaiting requested review from BobDotCom

@FrostByte266 FrostByte266 Awaiting requested review from FrostByte266

@Dorukyum Dorukyum Awaiting requested review from Dorukyum

Assignees

No one assigned

Labels

priority: low Low Priority

Projects

Pycord Releases
Status: Done

Milestone

v2.7

Development

Successfully merging this pull request may close these issues.

Replace deprecated audioop implementation of PCMVolumeTransformer.read

7 participants

@davidhozic @OmLanke @VincentRPS @DA-344 @Lulalaby @plun1331 @Dorukyum