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

Sounds setters #395

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

sockheadrps
Copy link

@sockheadrps sockheadrps commented Apr 23, 2023

edit:
Added TinyTag as a dependency to detect audio file meta data for channels and rate to auto set those values.
TinyTag is MIT and has no external dependencies of its own

Pull request summary

This could be a solution to #358

The sample rate of audio for Sound is hard coded (it might have something to do with YTDL, Im not sure), however if you create a sound using a local file

Sound(source="my_audio.mp3")

it's possible and likely to have files with different sample rates. Ive personally run into this issue and have seen others run into it as well, and the only solution right now is to hack at the library code to change the sample rate manually.

This PR just gives Sound.rate and Sound.channels a setter on each class property. Maybe in the future I can look at pulling sample meta data from the suppled audio file, but sometimes that can even be wrong so those class properties kind of need to be able to be set.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
    • I have updated the changelog with a quick recap of my changes.
  • 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, ...)

@IAmTomahawkx
Copy link
Member

@sockheadrps sorry for the late response, I kind of forgot to check prs.
Currently this pr is failing lints, please fix that by running black twitchio -l120.
Additionally, due to the new dependency the docs fail to build. Please add the dependency to both setup.py under the sounds extra and the docs requirements.txt. Beyond that it looks fine to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants