8000 [WIP] Add selectAudioOutput method. by cloudwebrtc · Pull Request #1030 · flutter-webrtc/flutter-webrtc · GitHub
[go: up one dir, main page]
Skip to content

[WIP] Add selectAudioOutput method. #1030

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 31 commits into from
Aug 22, 2022
Merged

[WIP] Add selectAudioOutput method. #1030

merged 31 commits into from
Aug 22, 2022

Conversation

cloudwebrtc
Copy link
Member
@cloudwebrtc cloudwebrtc commented Jul 29, 2022 •
edited
Loading

UPDATE:
Upstream dependencies have been upgraded, Need to be implemented in this PR

   Function(dynamic event)? ondevicechange;

   /// Prompts the user to select a specific audio output device.
   Future<MediaDeviceInfo> selectAudioOutput([AudioOutputOptions? option
8000
s]);

Future<MediaDeviceInfo> selectAudioOutput([AudioOutputOptions? options]);

Function(dynamic event)? ondevicechange

  • Windows
  • Android
  • macOS
  • iOS

MediaDevices.enumerateDevices

  • Windows
  • Android
  • macOS
  • iOS

@cloudwebrtc cloudwebrtc force-pushed the feat/select-audio-output branch from 5e25c2e to e941e12 Compare July 29, 2022 03:59
@davidzhao davidzhao requested a review from hiroshihorie July 29, 2022 04:56
ycherniavskyi
ycherniavskyi requested changes Aug 2, 2022
View reviewed changes
Copy link
Member
@ycherniavskyi ycherniavskyi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

What do you think about choose some common formatter rules (with configs for popular IDEs)?

@cloudwebrtc
Copy link
Member Author
cloudwebrtc commented Aug 2, 2022 •
edited
Loading

What do you think about choose some common formatter rules (with configs for popular IDEs)?

I used to add a clang-format styles file https://github.com/flutter-webrtc/flutter-webrtc/blob/main/.clang-format, but also need to add relevant formatting validation in CI.
Let me see how flutter/engine does it

@ycherniavskyi
Copy link
Member

It seems that flutter engine rather specific on this point - CONTRIBUTING.

@cloudwebrtc cloudwebrtc mentioned this pull request Aug 3, 2022
Open
@nikohpng nikohpng mentioned this pull request Aug 5, 2022
Open
@ollyde
Copy link
ollyde commented Aug 5, 2022 •
edited
Loading

@cloudwebrtc Can I add to this as well; on iOS there's no external microphone to select, only the internal one. Makes the whole library useless since a lot of people use airpods for video calls etc. (phone in the pocket, meeting booths, on-the-go, etc)

Calling

List<MediaDeviceInfo> devices = await navigator.mediaDevices.enumerateDevices();

Only the built-in microphone is returned audioinput

Tested on Mac/Web and it works just fine, on iPad and iPhone it breaks.

@ollyde
Copy link
ollyde commented Aug 5, 2022

More another story for the audio input issue #1041

ycherniavskyi
ycherniavskyi reviewed Aug 9, 2022
View reviewed changes
@cloudwebrtc cloudwebrtc mentioned this pull request Aug 15, 2022
Merged
@cloudwebrtc
Copy link
Member Author
cloudwebrtc commented Aug 15, 2022 •
edited
Loading

@ycherniavskyi It seems that iOS can't enumerate all audio outputs devices and can't select output devices, do you have any ideas?

@OllyDixon Perhaps we have to hand over control of output to the system on iOS, and the only thing we can do is select the speaker or the currently preferred output.

@ycherniavskyi
Copy link
Member

@cloudwebrtc I will check this PR today and write my thoughts.

@cloudwebrtc cloudwebrtc force-pushed the feat/select-audio-output branch from e4cb77a to 1dac603 Compare August 19, 2022 13:53
@cloudwebrtc cloudwebrtc merged commit 1dcb822 into main Aug 22, 2022
@cloudwebrtc cloudwebrtc deleted the feat/select-audio-output branch August 22, 2022 02:11
@ollyde
Copy link
ollyde commented Aug 22, 2022

@cloudwebrtc I see you merged it, is it now ready for all devices? 🚀

@cloudwebrtc
Copy link
Member Author

@OllyDixon Yes, all platforms can choose the output device,
iOS can switch the output device between the preferred device and the speaker if a wired or Bluetooth headset is connected.
Other platforms can select all enumerated output devices

@ollyde
Copy link
ollyde commented Aug 22, 2022

@cloudwebrtc great work, thanks! 🚀

wer-mathurin
wer-mathurin reviewed Aug 23, 2022
View reviewed changes
android/src/main/java/com/cloudwebrtc/webrtc/MethodCallHandlerImpl.java
@@ -980,14 +988,14 @@ public String peerConnectionInit(ConstraintsMap configuration, ConstraintsMap co
if (mPeerConnectionObservers.size() == 0) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.S
|| context.getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.S) {
audioManager.onAudioManagerRequested(true);
//audioSwitchManager.start();
Copy link
Contributor

Choose a reason for hiding this comment

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

Look strange that this if execute nothing. I didn't know if we need to start the manager or not. But having a if that does nothing seems useless

Copy link
Member Author

Choose a reason for hiding this comment

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

I rechecked the code, this piece should be able to be removed,
audioSwitchManager has already called start when it is created, and start is only used to monitor device changes, so it will not take over the audio route

Copy link
@fcancela fcancela Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Is this Bluetooth permission necessary?

...
        ArrayList<String> permissions = new ArrayList<>();
        permissions.add(Manifest.permission.BLUETOOTH_CONNECT);
        requestPermissions(
                permissions,
                (args) -> {
                  //audioSwitchManager.start();
                },
                (args) -> {
                });
      }
    }
...

I'm getting requested access for nearby devices but from my understanding, this shouldn't be required. My app does not require bluetooth.

Can it be removed? If not, is there a way to prevent this line from being called?

Thanks in advance!

wer-mathurin
wer-mathurin reviewed Aug 23, 2022
View reviewed changes
android/src/main/java/com/cloudwebrtc/webrtc/MethodCallHandlerImpl.java
@@ -1545,7 +1567,7 @@ public void peerConnectionDispose(final String id) {
mPeerConnectionObservers.remove(id);
}
if (mPeerConnectionObservers.size() == 0) {
audioManager.onAudioManagerRequested(false);
//audioSwitchManager.stop();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as my previous comments @cloudwebrtc

@amantoux amantoux mentioned this pull request Sep 29, 2022
Closed
@vmwsree
Copy link
vmwsree commented Oct 29, 2022

Is there an example code that we can refer regarding routing audio between Bluetooth and speakerphone

@michaeljajou
Copy link

How can we achieve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ycherniavskyi ycherniavskyi ycherniavskyi requested changes

@wer-mathurin wer-mathurin wer-mathurin left review comments

@fcancela fcancela fcancela left review comments

@hiroshihorie hiroshihorie hiroshihorie approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@cloudwebrtc @ycherniavskyi @ollyde @vmwsree @michaeljajou @hiroshihorie @wer-mathurin @fcancela @davidliu @zjzhang-cn
0