Skip to content

Fixes #2932 - Allow a toneFrequency prop to be passed to <Audio> for changing the pitch.#2979

Merged
JonnyBurger merged 31 commits intoremotion-dev:mainfrom
evoxf1:main
Oct 7, 2023
Merged

Fixes #2932 - Allow a toneFrequency prop to be passed to <Audio> for changing the pitch.#2979
JonnyBurger merged 31 commits intoremotion-dev:mainfrom
evoxf1:main

Conversation

@evoxf1
Copy link
Copy Markdown
Contributor

@evoxf1 evoxf1 commented Oct 6, 2023

Issue

Fixes #2932

Description

This PR addresses issue #2932 by introducing the toneFrequency prop to the <Audio> component. The toneFrequency prop allows users to specify the pitch adjustment for audio playback, with a valid range from 0.01 to 2(0 being not assignable). This feature enables precise control over audio pitch during rendering.

Changes Made

  • Added toneFrequency prop to the <Audio> component, allowing users to control pitch adjustment during audio playback.
  • Updated documentation to include information about the toneFrequency prop, specifying its usage and valid range.
  • Ensured that the test suite passes with the new changes.
  • Documented potential tradeoffs, if any, related to the implementation of the toneFrequency prop.

Tradeoffs Considered

Not at the moment.

How to Test

Please refer to audio.md Docs.

/claim #2932

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Oct 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
bugs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 7, 2023 0:43am
remotion ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 7, 2023 0:43am

@evoxf1 evoxf1 changed the title Fixed #2932 - Allow a toneFrequency prop to be passed to <Audio> for changing the pitch. Fixes #2932 - Allow a toneFrequency prop to be passed to <Audio> for changing the pitch. Oct 6, 2023
Copy link
Copy Markdown
Member

@JonnyBurger JonnyBurger left a comment

Choose a reason for hiding this comment

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

This goes into the right direction.

However, the PR did not actually work, it was not actually possible to pass a prop and then the filter was not actually applied to the audio. I helped a bit here!

Now it works, but I noticed regarding documentation and validation there is one major flaw:
It is currently implemented to take values between 0 and 1, however 1 is the normal pitch, and a value like 1.2 will make the pitch 20% higher, while a value like 0.8 will make the pitch 20% lower.

It is not a range between 0 and 1.
Can you fix this in the validation logic and documentation?

@evoxf1
Copy link
Copy Markdown
Contributor Author

evoxf1 commented Oct 7, 2023

@JonnyBurger

Ya sure it can be fixed maybe altering the function, so according to you the pitch should be between 0-2, meaning value less that one will reduce the pitch and more than 1 will increase?

Copy link
Copy Markdown
Member

@JonnyBurger JonnyBurger left a comment

Choose a reason for hiding this comment

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

Nice! Thanks a lot! 🙌

@JonnyBurger
Copy link
Copy Markdown
Member

/approve

@algora-pbc
Copy link
Copy Markdown

algora-pbc bot commented Oct 7, 2023

Could not find a claim that corresponds to the PR. Please add the /claim snippet to the PR body first.

@JonnyBurger
Copy link
Copy Markdown
Member

/approve

@algora-pbc
Copy link
Copy Markdown

algora-pbc bot commented Oct 7, 2023

@JonnyBurger: The claim has been successfully added to reward-all. You can visit your dashboard to complete the payment.

JonnyBurger added a commit that referenced this pull request Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow a toneFrequency prop to be passed to <Audio> (Change pitch of Audio)

2 participants