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

Change audiosessioncategory to AVAudioSesstionCategoryPlayAndRecord #2

Closed
wants to merge 1 commit into from

Conversation

jferas
Copy link

@jferas jferas commented Feb 1, 2020

This allows it to better coexist with flutter_tts plugin .. without this change, flutter_tts would no longer speak after the speech recognition plugin did its first "listen".

@jferas jferas requested a review from jaumard February 1, 2020 16:41
@jferas
Copy link
Author

jferas commented Feb 2, 2020

Forgot to mention.. this fixes a problem observed only on iOS.... no problem on Android.

@jaumard
Copy link
Owner

jaumard commented Feb 3, 2020

Hello,

Thanks for the PR, looks a bit weird to add "Play" when this plugin doesn't play anything...
Might be better to let this one by default and have an optional parameter to "speech.listen" method that you can set to have PlayAndRecord instead of just record.

What do you think ?

@jferas
Copy link
Author

jferas commented Feb 3, 2020

Good point... I'll push a new commit to the PR that does that... thanks for the feedback.

Hello,

Thanks for the PR, looks a bit weird to add "Play" when this plugin doesn't play anything...
Might be better to let this one by default and have an optional parameter to "speech.listen" method that you can set to have PlayAndRecord instead of just record.

What do you think ?

@jferas
Copy link
Author

jferas commented Feb 3, 2020

I was in the middle of modifying the .m code to take the optional parameter "allowPlay" and gave some more thought about it... this optional parameter is not needed for Android, where TTS and Speech Recognition inter-operate well together... so as strange as it seems to have "Play" in the objective C code where category is setup, it seems as strange (if not more so) to pass "allowPlay" in as a parameter at the Flutter/Dart level just to deal with an issue on iOS that makes it work differently than Android.

The more I think about it, I think the approach that doesn't employ the optional parameter makes more sense, as there is no need for the user of the library to think differently about iOS vs Android.

@jaumard
Copy link
Owner

jaumard commented Feb 3, 2020

Having "Play" on iOS make only sens if you want to do TTS too, which is not the case for everyone. I prefer having a useless optional parameter that only people who want TTS will set that putting it by default for everyone. Also if the optional is well documented on the dart side it can say that it's iOS only flag.

I afraid that putting "Play" by default someone will make another PR to remove it because it conflicts with another third library that will not need "Play" or whatever. It's not needed by this plugin so I really prefer not having it by default.

There is few plugin (even official one) that have flag that do stuff only in one specific platform, so I'm not afraid about that, it just have to be documented.

@jaumard
Copy link
Owner

jaumard commented Apr 25, 2021

no move since a year, closing this :) be free to reopen if needed

@jaumard jaumard closed this Apr 25, 2021
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.

2 participants