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

feat: make it possible to play audio through the earpiece speaker #2285

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

Conversation

Egbert-Jan
Copy link

@Egbert-Jan Egbert-Jan commented Apr 2, 2024

This new functionality allows you to set the audioUsageType and audioContentType for Android, and the iosCategroy and iosCategoryMode for iOS with the updateOptions function. This makes it possible to force the audio to come through the earpiece speaker. This can for example be useful to play an audio message in a chat app. For this to work on Android I also made a pull request in KotlinAudio (doublesymmetry/KotlinAudio#105). I tried but could not get the RNTP example app to use the updated KotlinAudio repo while executing the publishToMavenLocal steps as shown in https://github.com/doublesymmetry/react-native-track-player/tree/main/example.

Example:
TrackPlayer.updateOptions({
iosCategory: IOSCategory.PlayAndRecord,
iosCategoryMode: IOSCategoryMode.VoiceChat,
androidAudioContentType: AndroidAudioContentType.Music,
androidAudioUsageType: AndroidAudioUsageType.VoiceCommunication
})

@Egbert-Jan
Copy link
Author

Hello @jspizziri @dcvz, could you maybe take a look at this pull request? I think it would be a useful feature. If something is wrong with it I could fix it.

@dcvz
Copy link
Contributor

dcvz commented Apr 17, 2024

Hey @Egbert-Jan we've been a bit busy, but I'll get to it as soon as I can.

Copy link
Collaborator

@jspizziri jspizziri left a comment

Choose a reason for hiding this comment

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

@Egbert-Jan

I left some low-level feedback. All in all, it looks like it probably works, but I'd need to test it. However, before we go any further there's a high level design question that needs to be answered (and @dcvz will need to weigh in on it).

Should things like androidAudioUsageType and androidAudioContentType be pushed into the AndroidOptions object that we introduced in recent versions. So rather than doing:

TrackPlayer.updateOptions({
   androidAudioUsageType: ...,
   androidAudioContentType: ...,
   iosCategory: ...,
   iosCategoryMode: ...,
   iosCategoryOptions: ...,
   ...
})

You'd do:

TrackPlayer.updateOptions({
  android: {
    usageType: ...,
    contentType: ...,
  },
  ios: {
    category: ...,
    categoryMode: ...,
    categoryOptions: ...,
  },
   ...
})

Since we're introducing this into updateOptions for the first time, I think that is the right approach personally. However, it does cause an api consistency problem, where setupPlayer would be different than updateOptions (since these options already exist on setupPlayer and we don't want to introduce a breaking change).

What I propose is that we add android and ios config to setupPlayer in an identical manner and only allow the new properties to be set in that way. Then we can add a simple shim in the TS side, that maps legacy config of androidAudioContentType, etc. into the new android: AndroidOptions property. We then mark the top level properties as deprecated.

This would IMO, allow for a more organized, and consistent API between both updateOptions and setupPlayer.

Let me know your thoughts.

docs/docs/api/objects/player-options.md Outdated Show resolved Hide resolved
docs/docs/api/objects/player-options.md Outdated Show resolved Hide resolved
docs/docs/api/objects/update-options.md Outdated Show resolved Hide resolved
src/interfaces/PlayerOptions.ts Outdated Show resolved Hide resolved
@Egbert-Jan
Copy link
Author

Thanks I resolved the documentation and import issues. Have you also noticed I made a pull request here: doublesymmetry/KotlinAudio#105. Those changed are needed for these to work.

@jspizziri
Copy link
Collaborator

@watadarkstar & @Egbert-Jan , it seems that there is some overlap between your two PRs (#2259). Can the two of you evaluate and and if I'm not mistaken, collaborate?

@Egbert-Jan
Copy link
Author

My pull request introduces the same functionality as #2259 for the iOS side. However it also adds the functionality to the Android side.

@tfkennedy777
Copy link

Just chiming in to say that I really appreciate the contributions on this issue, and I'm rooting for this to get merged! Would be super helpful to update these options on the fly after initialization.

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.

None yet

4 participants