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

resample audio to 16kHz: #80 #81

Merged
merged 2 commits into from
Mar 13, 2024
Merged

resample audio to 16kHz: #80 #81

merged 2 commits into from
Mar 13, 2024

Conversation

ndrean
Copy link
Collaborator

@ndrean ndrean commented Mar 11, 2024

Small JS optimisation that resamples the audio from (usually) 48kHz to 16kHz.

JS code only. You can test it.

Next (2/2 for #80) is Readme.

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (d532505) to head (9a65d5d).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #81   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          199       199           
=========================================
  Hits           199       199           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@LuchoTurtle LuchoTurtle left a comment

Choose a reason for hiding this comment

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

Looks good and it seems to be working @ndrean .

Instead of opening a new PR, can you add the changes to the README on this one? Just to keep the feature contained to this PR and making it easier for people to go over it in the future = D

@ndrean ndrean changed the title 1/2 resample audio to 16kHz: JS code only. #80 resample audio to 16kHz: #80 Mar 12, 2024
@ndrean
Copy link
Collaborator Author

ndrean commented Mar 12, 2024

@LuchoTurtle
Check readme.

Copy link
Member

@LuchoTurtle LuchoTurtle left a comment

Choose a reason for hiding this comment

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

Thank you for the changes :)

Just small feedback:

  • please refrain from formatting the whole README in these PRs. It makes it difficult to review the changes related to the feature the PR is addressing :P
  • I can't really make changes to the PR because you are merging from your main instead of creating a branch. So I can't push commits upstream to your fork.

Regardless, awesome PR as always and I really thank you for effort you put in, it's greatly appreciated as always! :D

@LuchoTurtle LuchoTurtle merged commit f19c7c2 into dwyl:main Mar 13, 2024
3 checks passed
@ndrean
Copy link
Collaborator Author

ndrean commented Mar 13, 2024

ah, sorry, yes, how stupid, I was on main, I did not pay attention. Git is not my friend.

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.

2 participants