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

Fix the "AudioEffectRecord" descriptions. #52441

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

follower
Copy link
Contributor

@follower follower commented Sep 6, 2021

The AudioEffectRecord effect has no microphone capture-specific functionality--it can be used with any audio bus.

This patch attempts to clarify this fact (so people like me who want to capture audio output know they're in the right place) while still providing a pointer to use of the effect with AudioStreamMicrophone for microphone capture.

Additional context

Edited to add:
Forgot to mention that I discovered the Audio Buses tutorial does have a mostly (i.e. probably doesn't have to be a file) correct description for this effect:

The Record effect allows audio passing through the bus to be written to a file.

The `AudioEffectRecord` effect has no microphone capture-specific functionality--it can be used with any audio bus.

This patch attempts to clarify this fact (so people like me who want to capture audio output know they're in the right place) while still providing a pointer use of the effect with `AudioStreamMicrophone` for microphone capture.
@follower follower requested a review from a team as a code owner September 6, 2021 13:57
@YeldhamDev YeldhamDev added this to the 4.0 milestone Sep 6, 2021
@YeldhamDev YeldhamDev added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 6, 2021
@follower
Copy link
Contributor Author

follower commented Sep 6, 2021

Related links

Dumping these related links (which I used to figure out what was going on) here so I can close some tabs. :D

While there's further followup actions that could be taken I'm not able to do so right now, so hopefully this additional context is useful for anyone (including future me) wanting to followup.

@fire
Copy link
Member

fire commented Sep 6, 2021

I and @lyuma will find time to review since we proposed the idea of AudioEffectRecord.

Edited: We proposed AudioEffectCapture which is mostly a replacement for our usecases for AudioEffectRecord because Record wasn't able to be toggled every 20ms.

@lyuma
Copy link
Contributor

lyuma commented Sep 6, 2021

@fire You are mistaken. It is AudioEffectCapture which we implemented, not AudioEffectRecord. But regardless we are in a position to understand both and their usages.

@YeldhamDev reading your tweets and such, I believe it is may actually be AudioEffectCapture that you want to use for your project... but that's your choice.

Anyway it's good to improve docs, so here are my comments.

I would like to add cross-linking between AudioEffectCapture and AudioEffectRecord so that it is easier to find them.

On AudioEffectRecord, I would write "This is for generating recordings. For real time processing, use [AudioEffectCapture]."

On AudioEffectCapture we can make a comment that "this is used for realtime processing. To record into an AudioStreamSample for later playback, see [AudioEffectRecord]"

And the same thing about "AudioStreamMicrophone" should also be written on AudioEffectCapture, since it is also expected to be a common use case.

One more thing, since you had difficulty finding this from the microphone capture, perhaps we can link to both of these AudioEffectCapture/AudioEffectRecord classes from AudioStreamMicrophone, too.

Haven't ever looked at those demos. They are probably out of date.

@follower
Copy link
Contributor Author

Oh! I don't think I knew AudioEffectCapture even existed! :) Or maybe I didn't pay attention because I wasn't at that stage of one of my side projects... :)

I'm fine with any changes you want to make to the text so please feel free to amend as desired.

@fire
Copy link
Member

fire commented Sep 13, 2021

Can you amend the changes so one is not waiting for each other?

@GeorgeS2019
Copy link

GeorgeS2019 commented Jan 1, 2022

@follower For a tutorial on AudioEffectCapture

for real-time processing and monitoring if the microphone is capturing audio

Godot Voip

@GeorgeS2019
Copy link

Feedback to AudioEffectCapture

Why doesn't this support signals, and instead require every user to poll?

Signals from Godot-Voip

  • received_voice_data(data: PoolRealArray, from_id: int) Emitted when voice data is received.
  • sent_voice_data(data: PoolRealArray) Emitted when recording and data is sent.

@GeorgeS2019
Copy link

@follower

One use case that supports the use of AudioEffectCapture instead of AudioEffectRecord:

Sound activated voice recording from microphone

@mhilbrunner mhilbrunner merged commit a1ea897 into godotengine:master Jan 7, 2022
@mhilbrunner
Copy link
Member

Thank you!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jan 12, 2022
@akien-mga
Copy link
Member

Cherry-picked for 3.4.3.

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.

7 participants