Skip to content

Unify use of 'output' and 'desired' in audio code-base #697

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented May 23, 2025

This PR renames a few stuff in our C++ code. (This isn't about Python code or Python conventions!)

I've been a bit slopy with the use of the desired and output prefixes in the audio C++ code, and this is becoming more evident as I'm working now on sample rate conversions (PR to come). I want to establish a clearer convention for the C++ audio code:

  • "desired" is for stuff that are desired by the user, e.g. desiredNumChannels. In C++ we should just represent this as fields in AudioStreamOptions (will create follow-up PR). It usually is an std::optional. When these fields are in AudioStreamOptions (as they should and will), we can ommit the desired prefix.
  • "output", as in "outNumChannels" are for properties of the actual output. It can e.g. refer to the number of channels of the decoded tensor (when decoding), or to the number of channels of the encoded data (when encoding). outputStuff is never an std::optional and it is typically set to desiredStuff.value_or(value_of_the_source).

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label May 23, 2025
Copy link
Contributor

@scotts scotts 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 making these even more clear!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants