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

AudioManager adjustments #486

Merged
merged 8 commits into from
Sep 18, 2024

Conversation

hiroshihorie
Copy link
Member

No description provided.

@bcherry
Copy link
Contributor

bcherry commented Sep 18, 2024

I think @davidzhao 's idea to offer a pinned category is a good alternative. basically instead of alwaysUsePlayAndRecord we'd offer pinnedCategory and that way you can pin it to anything you want. The default (nil) would be automatic and would maybe set it to soloAmbient -> playback or record -> playAndRecord based on capabilities enabled, and would never go back down the chain as they get unenabled within a session. or maybe just playAndRecord is the default and you can't set it to nil at all (so it's just sessionCategory and you're expected to set it).

worth considering and maybe more future-proof?

@davidzhao
Copy link
Member

I think @davidzhao 's idea to offer a pinned category is a good alternative. basically instead of alwaysUsePlayAndRecord we'd offer pinnedCategory and that way you can pin it to anything you want. The default (nil) would be automatic and would maybe set it to soloAmbient -> playback or record -> playAndRecord based on capabilities enabled, and would never go back down the chain as they get unenabled within a session.

worth considering and maybe more future-proof?

maybe we can allow pinning a LKRTCAudioSessionConfiguration ? that can include all of the customizations with audio level.. and our job is to sync it with webrtc stack

@bcherry
Copy link
Contributor

bcherry commented Sep 18, 2024

Yeah that's also a better way to expose a lot of detailed configuration capability, without the customConfigureFunc which is pretty messy to actually use.

@hiroshihorie
Copy link
Member Author

So a pinned configuration for all states ? or per state ?

@davidzhao
Copy link
Member

So a pinned configuration for all states ? or per state ?

IMO just a single state.. users could change it however they want.. but most likely my guess is they would stay in a single configuration.

for those that want dynamic configuration, they could already implement the function override.

@hiroshihorie hiroshihorie changed the title Option to always use .playAndRecord AudioManager adjustments Sep 18, 2024
@@ -121,6 +123,12 @@ public class AudioManager: Loggable {
set { _state.mutate { $0.isSpeakerOutputPreferred = newValue } }
}

/// If this is set, this will be used instead of dynamic configuration.
public var sessionConfiguration: AudioSessionConfiguration? {
Copy link
Contributor

Choose a reason for hiding this comment

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

you should add a comment to the customConfigureFunc as well noting that if you set it then the sessionConfiguration and isSpeakerOutputPreferred properties will be ignored

Copy link
Contributor

@bcherry bcherry left a comment

Choose a reason for hiding this comment

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

makes sense to me - great change

@hiroshihorie hiroshihorie merged commit 21d2c4e into main Sep 18, 2024
12 checks passed
@hiroshihorie hiroshihorie deleted the hiroshi/audiomanager-prefer-playandrecord branch September 18, 2024 21:16
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.

3 participants